Prevent redundant toTree wrapping for macros

[raycohen]

In an app with many addons, we are seeing this wrapping get applied a significant number of times, eventually leading to "Maximum call stack size exceeded" errors when toTree is finally executed.

When I copy the stack trace out of visual code just before the "Maximum call stack size exceeded" error, there are 7364 lines in the stack that match node_modules/@embroider/macros/src/ember-addon-main.js:56

I think this wrapper only needs to be applied once per app instance. Is there something else I should be doing to prevent @embroider/macros included method from getting hit so much?

[simonihmig]

LGTM!

Is there something else I should be doing to prevent @embroider/macros included method from getting hit so much?

No, I don't think so. You are getting called (not literally you of course 😉), by EmberCLI, so there is nothing to change that on this side. However I am quite puzzled that include() is called that much! (>7000x 😱)

[ef4]

I've seen some pretty crazy things like that in big apps. ember-cli creates separate addon instances per consumer.

If you have an addon that uses embroider/macros, and that addon is used by 10 other addons, and those addons are each used by 10 other addons, and those addons are used by 10 other addons, embroider/macros will get instantiated 1000 times.

[raycohen]

ember-cli creates separate addon instances per consumer.

Yeah that seems to be the situation I'm seeing. I've got ~175 addons recognized by the project, but the recursive included calls from

https://github.com/ember-cli/ember-cli/blob/1519391eb562c446123e6b63fa400ed7b9e6b77a/lib/models/addon.js#L769

blow things up. We've got 37 addons that declare a dependency on ember-classic-decorator which depends on @embroider/macros

[rwjblue]

This will still happen once per @embroider/macros version, right (because the module scoped boolean or set would be per-version)? Unless there is some sort of highlander logic going on (which I don't recall having here),

[ef4]

I think that's correct, but that's probably not enough to blow up the stack limit.