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

externals: ["@glimmer/validator"] does not work in apps #1487

Closed
chancancode opened this issue Jun 29, 2023 · 3 comments
Closed

externals: ["@glimmer/validator"] does not work in apps #1487

chancancode opened this issue Jun 29, 2023 · 3 comments

Comments

@chancancode
Copy link
Contributor

To reproduce:

  1. ember new --embroider zomg
  2. In package.json, add "ember-addon": { "externals": ["@glimmer/validator"] }
    Note: per spec, the key should be "ember" Support ember key in package.json (instead of ember-addon) #1226
  3. In app.js or anywhere else, add an import for @glimmer/validator

Expected result: @glimmer/validator should only be included in once vendor.js, not in chunks.*.js

Actual result: @glimmer/validator is included in vendor.js, and then a second time in one of the chunks.

This is inefficient, but in the specific case of @glimmer/validator it actually causes an error when multiple copies are included. It seems like addons are able to use the same configuration successfully to avoid the problem, and per spec the externals key is one of the few configurations that are supposed to work in both apps and addons.

Using require instead works as a workaround.

@ef4
Copy link
Contributor

ef4 commented Jun 29, 2023

Another workaround is to tell webpack directly to treat it as commonjs (which means webpack itself will do the require):

// ember-cli-build.js
compatBuild(app, Webpack, {
  packagerOptions: {
    webpackConfig: {
      externals: {
        '@glimmer/validator': 'commonjs @glimmer/validator',
      },
     },
   },
}

You're correct that the spec says this option is available to apps, however it's only available to apps that also declare ember-addon.version === 2, meaning they're already in v2 format. Embroider produces this format internally for apps, but it's not the format you author and even if you tried to opt-in to being v2 that is going to require a lot of other strictness that an app is not likely to be able to easily meet just to solve this bug.

We just shipped embroider 3.1 and have been noticing that this class of problem is likely to be more common in that release. The reason is that we no longer do a complete rewrite of node_modules, which leaves us more vulnerable to accidental resolutions now. Up until now, the ember-provided virtual packages have always just been a fallback when normal resolving fails, but I think we're forced to aggressively externalize all of them.

@glimmer/validator may not be a super common one, but rsvp definitely is and essentially every ember app has an accidentally-resolvable copy of rsvp in node_modules, since it's used by some of the broccoli tooling.

So in summary: we plan to ship a bugfix that would externalize @glimmer/validator (and rsvp and every other virtual package provided by ember-source) automatically for every app and addon. A package can opt out of this behavior by explicitly declaring their dependency. So an app with @glimmer/validator listed in its dependencies would get the real @glimmer/validator, but one that does not would always get Ember's copy, even if node_modules/@glimmer/validator happens to exist.

@chancancode
Copy link
Contributor Author

Sounds like a good plan and thanks for the explanation 🙌🏼

@ef4
Copy link
Contributor

ef4 commented Jul 1, 2023

This should be fixed by #1495

@ef4 ef4 closed this as completed Jul 1, 2023
chancancode added a commit to chancancode/embroider that referenced this issue Jul 3, 2023
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

2 participants