api.modifyClass sometimes(!) not working

RGJ · December 17, 2021, 7:54am

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 · December 17, 2021, 9:49am

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 · December 17, 2021, 11:06am

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 · December 17, 2021, 11:14am

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 · December 17, 2021, 11:21am

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 · December 17, 2021, 11:23am

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

RGJ · December 17, 2021, 11:30am

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

merefield · December 17, 2021, 11:34am

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 · December 17, 2021, 12:15pm

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 · December 18, 2021, 9:20am

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 #bug …

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

@Johani thank you for your help!

RGJ · December 20, 2021, 5:54pm

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 · June 23, 2022, 9:32am

Revisiting this:

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

merefield · January 15, 2023, 5:04pm

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

david · April 18, 2023, 10:39am

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

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

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.

david · April 22, 2023, 7:00am

This topic was automatically closed after 3 days. New replies are no longer allowed.

Topic		Replies	Views
Using modifyClass for objects which are initialized early in boot developers	14	1073	November 2, 2023
Some users get javascript errors when trying to load our Discourse support	2	838	January 16, 2023
modifyClass failed with a warning dev	6	624	April 18, 2023
A versioned API for client side plugins dev rest-api	32	18767	October 13, 2023
How do you force a script to refire on every page load in Discourse? dev	39	18463	November 26, 2020

api.modifyClass sometimes(!) not working

#1 Working

#2 Failing

#3 Failing with warning

#4 Working

Related Topics