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 to change core behavior Developer Guides	0	1559	April 18, 2023
modifyClass failed with a warning Dev	6	701	April 18, 2023
A versioned API for client side plugins Dev rest-api	34	19425	June 21, 2024
TypeError: Discourse._registerPluginCode is not a function Support	7	2410	January 23, 2019
Add Category Column Theme component official , category-column	13	6807	May 28, 2024

api.modifyClass sometimes(!) not working

#1 Working

#2 Failing

#3 Failing with warning

#4 Working

Related topics