Issues overriding getters in a controller (3.0.0)

bug
#1

I’ve got this code in my initializer, it’s attempting to override the getter for allAdminRoutes:

    api.modifyClass('controller:admin-plugins', {
      pluginId: PLUGIN_ID,

      get allAdminRoutes() {
        return this.model
          .filter((p) => p?.enabled)
          .map((p) => {
            return p.admin_route;
          })
          .filter(Boolean);
      },
    });

It’s failing with:

TypeError: this.model is undefined

Now, I wouldn’t mind, but it’s just the original getter code (I’ve not actually changed it yet!)

what am I missing? Is overriding a getter in this way somehow not supported or does it introduce some kind of scoping issue?

2 Likes
api.modifyClass sometimes(!) not working
#2

Oddly this doesn’t seem to be an issue with Latest. (That was only a test on Production.)

I’ll see if I can reproduce on a clean 3.0.0 Stable.

1 Like
#4

The example should work, and I’m not aware of any relevant changes between the stable release and now. Let us know if you manage to reproduce the issue!

1 Like
#5

thanks David. Will do. Yes I noticed that. Ball is definitely in my court for now as the environment in which I experienced this was not fully clean.

1 Like
#7

OK David, it appears (and this is weird):

  • works in Production
  • fails in Development 3.0.0 onwards (possibly before)
  • I’ve tested on a vanilla no docker Dev environment but also on a very clean (but possibly config incomplete bespoke Docker dev environment).

Here’s the errors:

Error occurred:

- While rendering:
  -top-level
    application
      discourse-root
        sidebar-wrapper
          admin
            admin-wrapper
              nav-item
                link-to
                  -link-to

Uncaught (in promise) TypeError: While generating link to route "adminPlugins": this.model is undefined
    get allAdminRoutes admin-plugins.js:20

note that I can put a debugger statement before the return this.model and it never fires the debugger.

@RGJ has kindly also looked at this and can repro this result.

This is a bit of a problem as clearly we need to develop stuff before it goes to production … but I don’t have to tell you that! :sweat_smile: Also a bit annoying it’s a problem on Stable …

Moved to bug for now …

2 Likes
#8

OK we’re even more sure of there’s an issue here… as mergeMixins is mentioned in the stack trace

image

(thanks to @RGJ for finding this)

4 Likes
#10

Nice digging - thanks @merefield and @RGJ

I’m not sure how much we can do to fix this within Discourse - as you noted, the issue seems to be upstream in Ember. Perhaps we can patch the problematic method :thinking:

To get us started, I’ve added a minimal failing test case so we can track progress. There are some comments in the test which describe why the issue is being triggered by our modifyClass system:

4 Likes
#11

Yeah, thanks for your efforts too. It’s strange they’ve left that issue unaddressed? Do we know if (Ember) 4.x will resolve it?

#12

Unfortunately it looks like the issue still exists in Ember 4.x :cry:

1 Like
#13

Funnily enough that seems to be the only progress over at the Ember project, within the associated PR which is still in Draft. I wonder if all that would be required is a polite request to prioritise it when the time becomes available?:

Fully appreciate this was opened by a staff member at a third party.

Is it worth raising an issue over at GitHub - emberjs/ember.js: Ember.js - A JavaScript framework for creating ambitious web applications and referencing this PR, this Topic and your test?

#14

Yep, my test is based on theirs (but with the discourse-specific api.modifyClass api). I don’t think we should open a new issue - it’s 100% the same as the one that’s already open. We’ll see if we can do anything to get it prioritised.

2 Likes