api.modifyClass sometimes(!) not working

[RGJ (Richard - Communiteq)]

Edit: reclassified as bug.
Full repro below.

I’m using api.modifyClass in a theme component.

On most pages this is working fine, but in some topics the property is not added to the user model when the page is reloaded. I cannot find why it is working on most topics, but not working on some, or what the topics where it is not working have in common.

Does anyone understand what I am doing wrong here?

TL;DR Adding api.container.lookup for the model, below the modifyClass, works around this issue.

[pfaffman (Jay Pfaffman)]

Oh my word. I might know!

I think that you need to add a pluginId

     <script type="text/discourse-plugin" version="0.1">
    api.modifyClass('model:user', {
      pluginId: 'my-plugin', 
      testProperty: function() {
        return 1;
      } 
    });
</script>

It’s required on later versions, I actually submitted PRs for a couple themes yesterday to add this. I might be turning into a plugin/theme developer at long last

[RGJ (Richard - Communiteq)]

Thank you Jay! That actually made a difference on my test instance.
But unfortunately it is not a fix-all golden bullet, the problem is still there

[merefield (Robert)]

Yeah the pluginId is there to prevent modifications being applied twice, I believe. Lack of it should not explain why it hasn’t run once?

[pfaffman (Jay Pfaffman)]

Darn. Just when I thought I knew something.

I thought perhaps if something else was modifying the class then it might be a race condition to see which one did it. But javascript is still mostly a mystery to me.

[merefield (Robert)]

It’s still an issue with the code, so reasonable to bring up.

[RGJ (Richard - Communiteq)]

@pfaffman

race condition

Yes, that crossed my mind as well. But this still fails on a test site with this as the only theme component.

[merefield (Robert)]

Out of interest, if you add this into a Theme Component in say:

• /javascripts/discourse/initializers/test-init.js.es6

instead, does the behaviour change/improve?

I’m not sure if these things are semantically identical …

… theoretically both should be evaluated when the browser refreshes, after which point a race condition should be moot on route transition as the modification should already have been applied …

[RGJ (Richard - Communiteq)]

@merefield

does the behaviour change/improve

Nope, same behavior.

I made a theme component with javascripts/discourse-test/initializers/initialize-discourse-test.js.es6:

import { withPluginApi } from "discourse/lib/plugin-api";

export default {
  name: 'discourse-test-initializer',
  initialize(){
    console.log('here1');
    withPluginApi("0.1", api => {
      console.log('here2');

      api.modifyClass('model:user', {
        pluginId: 'test',
        testFunction: function() {
          console.log('here3');
          return 2;
        }
      });

    });
  }
}

Prints

here1
here2

on every page.

On some topics entering Discourse.currentUser.testFunction() in the console works and yields here3 and 2 as output.

On some topics it does not (Uncaught TypeError: Discourse.currentUser.testFunction is not a function)

[RGJ (Richard - Communiteq)]

Ok, I saw a notification from a helpful post that apparently has been removed in the meanwhile.
The suggestion did not work but it did contain a clue that helped me resolve this.

Adding the following snippet below the non-working code consistently resolved the issues I was having. I have tested this on multiple independent forums, both on stable and on tests-passed. I think this should be reclassified as Contribute > Bug …

<script type="text/discourse-plugin" version="0.1">
const userModel = api.container.lookup("model:user");
</script>

@Johani thank you for your help!

[RGJ (Richard - Communiteq)]

Reclassifying this as bug.

I have added a console.log in the plugin API code app/assets/javascripts/discourse/app/lib/plugin-api.js so it logs whenever modifyClass is being called.

I have removed all external plugins to make sure there was not a conflict somewhere.

Repro:

• create an empty forum on stable (so no Ember CLI). This is not working on tests-passed (without Ember CLI) either. I did not test this with Ember CLI.

• add a theme component with this in Common - Head

#1 Working

<script type="text/discourse-plugin" version="0.1">
    api.modifyClass('model:user', {
      pluginId: 'test-tc',
      testFunction: function() {
        return 1;
      } 
    });
</script>

• load the home page

• console shows modifyClass called for model:user _application-08d9058ddd37ba80992f770509f4919ad0738a17f14fb85167b1dc1f32f8b56e.js:23490:16 Object { pluginId: "test-tc", testFunction: testFunction() }

• enter Discourse.currentUser.testFunction() in console

• “1” is printed

#2 Failing

• Go to a topic, for instance ‘Welcome to Discourse’ and reload the page
• console shows the same “modifyClass called” logs
• enter Discourse.currentUser.testFunction() in console
• Uncaught TypeError: Discourse.currentUser.testFunction is not a function is printed

#3 Failing with warning

• Append a single line to top of the theme component so it looks like this:

<script type="text/discourse-plugin" version="0.1">
    const userModel = api.container.lookup("model:user");

    api.modifyClass('model:user', {
      pluginId: 'test-tc',
      testFunction: function() {
        return 1;
      } 
    });

</script>

• Go to a topic, for instance ‘Welcome to Discourse’ and reload the page
• console shows the same “modifyClass called” logs
• console shows a warning "model:user" was already cached in the container. Changes won't be applied.
• enter Discourse.currentUser.testFunction() in console
• Uncaught TypeError: Discourse.currentUser.testFunction is not a function is printed

#4 Working

• Move the lookup line to bottom of the theme component so it looks like this:

<script type="text/discourse-plugin" version="0.1">
    api.modifyClass('model:user', {
      pluginId: 'test-tc',
      testFunction: function() {
        return 1;
      } 
    });

    const userModel = api.container.lookup("model:user");
</script>

• Go to a topic, for instance ‘Welcome to Discourse’ and reload the page
• console shows the same “modifyClass called” logs
• enter Discourse.currentUser.testFunction() in console
• “1” is printed

[RGJ (Richard - Communiteq)]

Revisiting this:

On Discourse 2.8 and up, using Discourse.User.current().testFunction() , case #2 is still failing.

[merefield (Robert)]

I’m having an issue with modifyClass too, not sure if it is related?: Issues overriding getters in a controller (3.0.0)

[david (David Taylor)]

I’ve just improved our warning for this kind of situation:

https://github.com/discourse/discourse/pull/21140

and also written up this documentation which explains the cause and how to resolve it:

Using modifyClass to change core behavior

For advanced themes and plugins, Discourse offers the modifyClass system. This allows you to extend and override functionality in many of core’s javascript classes. When to use modifyClass modifyClass should be a last resort, when your customization cannot be made via Discourse’s more stable customization APIs (e.g. plugin-api methods, plugin outlets, transformers). Core’s code can change at any time. And therefore, customizations made via modifyClass could break at any time. When using this A…

I think that should take care of all the concerns in this topic. If not, please share some example failing code and I’ll take a look.