Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Installing addon with latest @embroider/macros as dependency fails #509

Closed
simonihmig opened this issue Sep 1, 2020 · 1 comment · Fixed by embroider-build/ember-auto-import#308

Comments

@simonihmig
Copy link
Collaborator

So, be prepared, this is a really weird one. Pulled my hair for a few hours...

Tbh, it's probably not really the fault of this library, but was not sure if there is any better place to report, especially as others adopting @embroider/macros will likely run into the same issue. This was first reported for ember-bootstrap, please see this issue for context: ember-bootstrap/ember-bootstrap#1201

I was somehow able to debug this, which was a bit tricky as you could not put breakpoints into embroider code that does not exist yet as the dependency is installed during script execution (I ran node --inspect node_modules/ember-cli/bin/ember i ember-bootstrap and put a breakpoint in ember-cli code where the (ember-bootstrap) addon was instantiated). So here are my findings...

The line that throws is this one: https://github.com/embroider-build/embroider/blob/master/packages/macros/src/ember-addon-main.ts#L83. And indeed the debugger shows that there is no astPlugins function defined on the class (i.e. static method). Quite confusingly the shape of the MacrosConfig class was similar, but not quite what was to be expected:

Bildschirmfoto 2020-09-01 um 16 25 26

As you can see, there exists a astPlugins function, but on the prototype, like a non-static method! Also other class members were different than what the source code would suggest.

I first thought that it could be something-something related to caching, like an old version of that dependency was somehow reused or whatever. But it turned out much more trivial, as a freshly generated Ember app already has @embroider/macros in its node_modules, albeit a rather old one (0.4.3), through ember-auto-import:

Bildschirmfoto 2020-09-01 um 17 35 36

And the shape of the MacrosConfig class of that version matches exactly what I was seeing in the debugger above: https://github.com/embroider-build/embroider/blob/v0.4.3/packages/macros/src/macros-config.ts!

But that alone would not explain why this breaks, if the addon entrypoint (here ember-addon-main.js were also that os the old v0.4.3 version. However it turns out, it is actually that of the latest addon version v0.21.0! (rightly, as that's what ember-bootstrap depends upon):

This is the debugger inside ember-addon-main.js, and as you can see it has a installBabelPlugin method, which exists for v0.21.0, bit not for v0.4.3

Bildschirmfoto 2020-09-01 um 18 16 35

tl;dr

So to summarize, when ember-bootstrap is installed and instantiated, whose init() calls into setupRegistry() and thus @embroider/macros' setupPreprocessorRegistry(), the evaluated ember-addon-main node module is that of v0.21.0, while its required macros-config.js is that of the previously installed 0.4.3! 🤯

Given that node evaluates a module only once, this could maybe explain why node still sees the old version of macros-config.js, as it is required eagerly before the new version is installed. But why the same does not hold true for ember-addon-main.js is beyond me. 🤔

What can we do about it?

First, as I said, it seems this is not really embroiders fault here. Rather it seems problematic to me, at least in this kind of edge cases, that ember-cli both requires modules that it also installs, all in the same execution run, which makes things sensitive to timing / execution order. Right?

Workarounds? We could at least update ember-auto-import's dependency. @ef4 Any reason to still use that old version of @embroider/core?

Other than that, anything to reliably prevent this issue?

/cc @rwjblue - sorry for the mention, but I thought someone with deep knowledge of ember-cli internals might be helpful here as well :)

@ef4
Copy link
Contributor

ef4 commented Sep 1, 2020

We can update ember-auto-import, it only shares @embroider/core so we can ensure that some configuration management happens consistently between them.

That said, we should also make sure @embroider/macros is robust against version skew. It's intended to be.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants