-
Notifications
You must be signed in to change notification settings - Fork 109
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
Error using ember-moment since 2.6.2 #578
Comments
Does the issue still occur if you use pnpm in your reproduction? Both older npm and yarn are ceally bad at dealing with peers. Also, what's your npm version? |
@NullVoxPopuli i was running today in the same error like @dwickern . We use npm We have installed npm ls moment
|
Does auto import 2.6.3 fix anything for you? It reverts the change to defaults in 2.6.2 |
i don't think so... because i have manually set Update: tested again, the latest working version is atm |
My app uses yarn:
The reproduction was using npm:
I converted the reproduction to pnpm and it has the same issue:
The reproduction is using ember-auto-import 2.6.3. It works if I downgrade to 2.6.1. |
Thanks for the reproduction. The macroCondition is actually not the problem here, when I look at the build output of the reproduction I see that it would go down the The problem is that we aren't even getting that far. The bug fix in #574 treats dependencies that webpack can't find as things that should be looked up at runtime in the classic AMD loader, and declares them as static dependencies so that the module loader can evaluate them in the right order. But it's doing that accidentally for the optional dependency here, making it not-optional. You will also notice that this bug only strikes in development. If you run the reproduction with Until we have a fix for this, here is a workaround that also fixes the reproduction app: diff --git a/ember-cli-build.js b/ember-cli-build.js
index ed991bd..d5aa0c0 100644
--- a/ember-cli-build.js
+++ b/ember-cli-build.js
@@ -1,9 +1,18 @@
'use strict';
const EmberApp = require('ember-cli/lib/broccoli/ember-app');
+const webpack = require('webpack');
module.exports = function (defaults) {
const app = new EmberApp(defaults, {
+ autoImport: {
+ webpack: {
+ plugins: new webpack.IgnorePlugin({
+ // workaround for https://github.com/embroider-build/ember-auto-import/issues/578
+ resourceRegExp: /moment-timezone/,
+ }),
+ },
+ },
// Add options here
}); |
@ef4 thanks, confirm that the workaround has solved the problem |
This workaround breaks the function gatherExternals(module, output = new Set()) {
if (externals.has(module)) {
...
}
else {
...
for (let dep of module.dependencies) {
...
}
...
}
...
} .../node_modules/ember-auto-import/js/webpack.js |
The workaround solved this today in an app using ember-source 4.4 |
Still struggling with this issue, I want to share my case. I have replaced
When I remove that import line (along with other stuff associated with it), I get yet another similar error:
Apparently then, the issue is not specific to |
@mehran-naghizadeh are these dependencies that you're importing declared in your package.json? A reproduction would be most helpful ✨ |
I think that sounds like an unrelated issue. The underlying issue here about those optional peer deps becoming non-optional should have been fixed in @embroider/macros 1.11.1 due to embroider-build/embroider#1468. |
Ember-moment uses macro conditions to decide which moment implementation to import:
https://github.com/adopted-ember-addons/ember-moment/blob/71cc4a1a3e12a912a56cc657c97170e3a7be0483/src/index.js
The first condition evaluates true, even though there is no dependency on moment-timezone. There is only the optional peer dependency in ember-moment's own package.json.
Reproduction: https://github.com/dwickern/ember-moment-396-reproduction
Related ember-moment issue: adopted-ember-addons/ember-moment#396
The text was updated successfully, but these errors were encountered: