Error using ember-moment since 2.6.2

[dwickern]

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

if (macroCondition(dependencySatisfies('moment-timezone', '*'))) {
  return importSync('moment-timezone').default;
} else if (macroCondition(dependencySatisfies('moment', '*'))) {
  return importSync('moment').default;
}

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: https://github.com/adopted-ember-addons/ember-moment/issues/396

[NullVoxPopuli]

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?

[mkszepp]

@NullVoxPopuli i was running today in the same error like @dwickern .

We use npm v9.5.0 with node v18.14.2.

We have installed moment

npm ls moment

app@1.3.2 /home/app
├─┬ ember-moment@10.0.0
│ └── moment@2.29.4 deduped
├─┬ ember-power-calendar-moment@0.2.0
│ └── moment@2.29.4 deduped
├─┬ moment-locales-webpack-plugin@1.2.0
│ └── moment@2.29.4 deduped
└── moment@2.29.4

[NullVoxPopuli]

Does auto import 2.6.3 fix anything for you? It reverts the change to defaults in 2.6.2

[mkszepp]

i don't think so... because i have manually set ^2.6.3 in my package.json, removed package.lock & node_modules folder

Update: tested again, the latest working version is atm 2.6.1

[dwickern]

My app uses yarn:

% yarn --version
3.2.2
% yarn why moment
└─ ui@workspace:.
   └─ moment@npm:2.29.4 (via npm:^2.29.4)
% yarn why moment-timezone

The reproduction was using npm:

% npm --version
8.18.0
% npm ls moment
ember-moment-396-repro@0.0.0 /Users/dwickern/code/test/ember-moment-396-repro
├─┬ ember-moment@10.0.0
│ └── moment@2.29.4 deduped
└── moment@2.29.4
% npm ls moment-timezone
ember-moment-396-repro@0.0.0 /Users/dwickern/code/test/ember-moment-396-repro
└── (empty)

I converted the reproduction to pnpm and it has the same issue:

% pnpm --version
8.3.1
% pnpm why moment
Legend: production dependency, optional only, dev only

ember-moment-396-repro@0.0.0 /Users/dwickern/code/test/ember-moment-396-repro

devDependencies:
ember-moment 10.0.0
└── moment 2.29.4 peer
moment 2.29.4
% pnpm why moment-timezone

The reproduction is using ember-auto-import 2.6.3. It works if I downgrade to 2.6.1.

[ef4]

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 moment branch and not the moment-timezone branch.

The problem is that we aren't even getting that far. The bug fix in https://github.com/embroider-build/ember-auto-import/pull/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 ember s --environment=production it works. That's because in prod we fully optimize away the dead branch, even before webpack has a chance to see it. We don't do that in development because in general some macros want to have runtime implementations in development (although dependencySatisfies is not really one of them, and if we had a more sophisticated implementation of macroCondition it could do this particular branch elimination even in dev).

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
   });

[mkszepp]

@ef4 thanks, confirm that the workaround has solved the problem

[mehran-naghizadeh]

This workaround breaks the ember serve command, in my case with ember-source 3.28. The issue is with module being null in this loop:

function gatherExternals(module, output = new Set()) {
  if (externals.has(module)) {
    ...
  }
  else {
    ...
    for (let dep of module.dependencies) {
      ...
    }
    ...
  }
  ...
}

_.../nodemodules/ember-auto-import/js/webpack.js

[kategengler]

The workaround solved this today in an app using ember-source 4.4

[mehran-naghizadeh]

Still struggling with this issue, I want to share my case. I have replaced moment.js with day.js, hoping that I will just escape the issue. Now I get the following error message, though:

Uncaught Error: Could not find module dayjs imported from ...

When I remove that import line (along with other stuff associated with it), I get yet another similar error:

Could not find module @formatjs/intl imported from ...

Apparently then, the issue is not specific to moment.

[NullVoxPopuli]

@mehran-naghizadeh are these dependencies that you're importing declared in your package.json? A reproduction would be most helpful ✨

[ef4]

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 https://github.com/embroider-build/embroider/pull/1468.