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

@babel/template placeholder build error resurrection #1077

Closed
Windvis opened this issue Jan 13, 2022 · 18 comments
Closed

@babel/template placeholder build error resurrection #1077

Windvis opened this issue Jan 13, 2022 · 18 comments

Comments

@Windvis
Copy link
Collaborator

Windvis commented Jan 13, 2022

Since the release of ember-get-config v1 (which uses @embroider/macros v0.50.0) several people in the Discord are seeing the following errors at build time:

broccoliBuilderErrorStack: Error: @babel/template placeholder "APP": Expected string substitution

This seems to be the issue that was fixed by this commit and was included in the 0.43.0 release of @embroider/macros.

In our app the issue is triggered by @embroider/macros v0.41.0 which is included by the ember-element-helper addon. What's strange is that that version of the macros didn't cause issues until a v0.50.0 release was part of our node_modules folder.

I'm unfamiliar with how this works so my debugging efforts didn't lead to something useful. Did anything change in the latest release that might explain this behavior? It probably worked by accident before and we should PR version bumps of the macro packages to all addons?

npm ls @embroider/macros of our project:

├─┬ [email protected]
│ └─┬ @embroider/[email protected]
│   └── @embroider/[email protected] 
├─┬ [email protected]
│ └─┬ [email protected]
│   └── @embroider/[email protected] 
└─┬ [email protected]
  └─┬ [email protected]
    ├─┬ @ember/[email protected]
    │ └── @embroider/[email protected] 
    ├── @embroider/[email protected] 
    ├─┬ @embroider/[email protected]
    │ └── @embroider/[email protected] 
    └─┬ [email protected]
      └─┬ @embroider/[email protected]
        └── @embroider/[email protected] 

Discord discussion: https://discord.com/channels/480462759797063690/930822209725948004

@obrunofontana
Copy link

I have the same problem, i corrected it by fixing the lib version ember-cli-moment-shim": "3.7.1", however, just a quick solution!

@jrjohnson
Copy link
Contributor

We're seeing this in an addon as @babel/template placeholder "LOG_VIEW_LOOKUPS": Expected string substitution

@navels
Copy link

navels commented Jan 13, 2022

Downgrading ember-cli-moment-shim isn't an option for us as it breaks our production build (terser/terser#684 (reference)).

@navels
Copy link

navels commented Jan 13, 2022

This seems to be fixed in 0.50.1 and adding this resolution to our package.json has fixed our build until ember-get-config can update:

  "resolutions": {
    "**/@embroider/macros": "0.50.1"
  }

@navels
Copy link

navels commented Jan 13, 2022

PR: mansona/ember-get-config#32

@RobbieTheWagner
Copy link

@navels I have been talking with @mansona and we did some digging. The real issue here is if you have any addons using < embroider 0.50 and then any addon using > 0.50 things go 💥. The best fix I have found for now, is to do:

    "**/@embroider/macros": "< 0.50.0",
    "**/@embroider/util": "< 0.50.0",

@jrjohnson
Copy link
Contributor

any addons using < embroider 0.50 and then any addon using > 0.50 things go 💥

That's gonnaa be tricky since the latest ember-cli LTS includes [email protected] which requires @embroider/[email protected]. So we have quite a few addons that fall into this version gap.

@RobbieTheWagner
Copy link

Yes, it is quite a bad problem.

@ef4 any plans on a fix so the versions can play nicely together?

@ef4
Copy link
Contributor

ef4 commented Jan 19, 2022

There were no significant changes to the macro system in 0.50.0. Can somebody share the contents of the value that's being passed to babelContext.template that makes it blow up?

It is true that all copies of the macros share the state that is being inlined into the source at this spot, so additional versions being around can influence the value here.

@jrjohnson
Copy link
Contributor

I'm not sure how to do that @ef4, I looked through the trace and I'm not sure where to put a debugger to get that info.

@ef4
Copy link
Contributor

ef4 commented Jan 19, 2022

Find this spot in the @embroider/macros package in your node_modules:

let statement = babelContext.template(`a(${JSON.stringify(value)})`)() as ExpressionStatement;

And console.log(JSON.stringify(value))

@jrjohnson
Copy link
Contributor

jrjohnson commented Jan 19, 2022

Looks a bit different, but I changed evaluate-json.js in our addon to

function buildLiterals(value, babelContext) {
    if (typeof value === 'undefined') {
        return babelContext.types.identifier('undefined');
    }
    console.log("\n\nOUTPUT:\n", JSON.stringify(value), "\n\n");
    let statement = babelContext.parse(`a(${JSON.stringify(value)})`);
    let expression = statement.program.body[0].expression;
    return expression.arguments[0];
}

and the output looks like:

OUTPUT:
 {"packages":{"APP_HOME/node_modules/ember-cli-moment-shim/node_modules/ember-get-config":{"config":{"modulePrefix":"dummy","environment":"development","rootURL":"/","locationType":"auto","intl":{"defaultLocale":"en"}},"EmberENV":{"FEATURES":{},"EXTEND_PROTOTYPES":{"Date":false},"DEBUG_TASKS":true,"_APPLICATION_TEMPLATE_WRAPPER":false,"_DEFAULT_ASYNC_OBSERVERS":true,"_JQUERY_INTEGRATION":false,"_TEMPLATE_ONLY_GLIMMER_COMPONENTS":true},"APP":{},"ember-cli-mirage":{"enabled":false,"usingProxy":false,"useDefaultPassthroughs":true},"moment":{"includeLocales":["es","fr"],"includeTimezone":"all"},"featureFlags":{"sessionLinkingAdminUi":true},"apiVersion":"v3.4","exportApplicationGlobal":true}}},"global":{"@embroider/macros":{"isTesting":false}}} 

OUTPUT:
 {"packages":{"APP_HOME/node_modules/ember-cli-moment-shim/node_modules/ember-get-config":{"config":{"modulePrefix":"dummy","environment":"development","rootURL":"/","locationType":"auto","intl":{"defaultLocale":"en"}},"EmberENV":{"FEATURES":{},"EXTEND_PROTOTYPES":{"Date":false},"DEBUG_TASKS":true,"_APPLICATION_TEMPLATE_WRAPPER":false,"_DEFAULT_ASYNC_OBSERVERS":true,"_JQUERY_INTEGRATION":false,"_TEMPLATE_ONLY_GLIMMER_COMPONENTS":true},"APP":{},"ember-cli-mirage":{"enabled":false,"usingProxy":false,"useDefaultPassthroughs":true},"moment":{"includeLocales":["es","fr"],"includeTimezone":"all"},"featureFlags":{"sessionLinkingAdminUi":true},"apiVersion":"v3.4","exportApplicationGlobal":true}}},"global":{"@embroider/macros":{"isTesting":false}}} 

OUTPUT:
 {"packages":{"APP_HOME/node_modules/ember-cli-moment-shim/node_modules/ember-get-config":{"config":{"modulePrefix":"dummy","environment":"development","rootURL":"/","locationType":"auto","intl":{"defaultLocale":"en"}},"EmberENV":{"FEATURES":{},"EXTEND_PROTOTYPES":{"Date":false},"DEBUG_TASKS":true,"_APPLICATION_TEMPLATE_WRAPPER":false,"_DEFAULT_ASYNC_OBSERVERS":true,"_JQUERY_INTEGRATION":false,"_TEMPLATE_ONLY_GLIMMER_COMPONENTS":true},"APP":{},"ember-cli-mirage":{"enabled":false,"usingProxy":false,"useDefaultPassthroughs":true},"moment":{"includeLocales":["es","fr"],"includeTimezone":"all"},"featureFlags":{"sessionLinkingAdminUi":true},"apiVersion":"v3.4","exportApplicationGlobal":true}}},"global":{"@embroider/macros":{"isTesting":false}}} 

⠹ building... [Babel: @embroider/macros > applyPatches]

OUTPUT:
 true 

OUTPUT:
 true 

OUTPUT:
 true 

OUTPUT:
 true 

OUTPUT:
 true 

OUTPUT:
 true 

OUTPUT:
 true 

Build Error (broccoli-persistent-filter:Babel > [Babel: @embroider/macros]) in @embroider/macros/runtime.js

APP_HOME/@embroider/macros/runtime.js: @babel/template placeholder "APP": Expected string substitution

@navels
Copy link

navels commented Jan 19, 2022

Basically it is the package's config/environment.js that is getting parsed incorrectly.

@Windvis
Copy link
Collaborator Author

Windvis commented Jan 19, 2022

It seems it's very easy to reproduce:

  1. Create a new Ember app (I used 3.28)
  2. Install ember-element-helper (it uses @embroider/util 0.41.0). The build will still work fine after this.
  3. Install ember-get-config v1. The build will fail after installing this package.

Reproduction app: https://github.com/Windvis/embroider-macros-babel-template-error-reproduction (Removed since the issue is resolved.)

I also tried installing @embroider/macros v0.50.0 as a direct dependency, but that doesn't make the build fail. So it seems to be related to ember-get-config's usage of the macros.

@ef4
Copy link
Contributor

ef4 commented Jan 19, 2022

So it seems to be related to ember-get-config's usage of the macros.

Ah, this is the actual issue. The format of the data that ember-get-config is storing in the macros is what triggers the bug. It's not really about version skew at all.

We could backport the fix and do patch releases, but we'd potentially need to manage 17 different patch releases, because the oldest embroider/macros mentioned here is 17 major releases behind.

I think the biggest thing we can do to help here is to cut a 1.0 release, so that the pain of asking addons to upgrade can happen once and then they'll be on a series that we can actually support with patch releases going forward.

I had been waiting to release a 1.0 until after solving some known limitations, but we're past the point where it's practical to consider this stuff prerelease.

@ef4
Copy link
Contributor

ef4 commented Jan 19, 2022

Ok, so 1.0 is released. Please spread the word, addons should depend on "@embroider/macros": "^1.0.0". There are no breaking changes in 1.0.

In the meantime, both yarn and npm now support overriding transitive dependencies. In the case of @embroider/macros it is safe to replace all of 0.x with ^1.0.0.

NPM added overrides in 8.3. Yarn has resolutions.

@Windvis
Copy link
Collaborator Author

Windvis commented Jan 19, 2022

Since it wasn't an actual bug I think we can close this now. Thanks for your insights and the new release!

@Windvis Windvis closed this as completed Jan 19, 2022
@navels
Copy link

navels commented Jan 19, 2022

thanks!

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

No branches or pull requests

6 participants