embroider-build / embroider

Compiling Ember apps into spec-compliant, modern Javascript.
MIT License
333 stars 136 forks source link

Webpack crashes with Babel transpiled ES6 modules #1004

Open andsmedeiros opened 2 years ago

andsmedeiros commented 2 years ago

Webpack started enforcing the presence of file extensions on import/export statements inside ES6 modules (https://github.com/webpack/webpack/issues/11467).

It seems Babel is currently injecting statements that violate this rule while transpiling. The following log is from a fresh install of Ember 3.28.1 with Embroider blueprint, after installing a dependency with ES6 modules and requiring one of them (I've redacted the sensible information):

ERROR in ../../../PROJECT_PATH/node_modules/@my-scope/my-package/src/auth-client.js 1:0-166 Module not found: Error: Can't resolve '/PROJECT_PATH/node_modules/@embroider/core/node_modules/@babel/runtime/helpers/esm/defineProperty' in '/PROJECT_PATH/node_modules/@my-scope/my-package/src' Did you mean 'defineProperty.js'? BREAKING CHANGE: The request '/PROJECT_PATH/node_modules/@embroider/core/node_modules/@babel/runtime/helpers/esm/defineProperty' failed to resolve only because it was resolved as fully specified (probably because the origin is strict EcmaScript Module, e. g. a module with javascript mimetype, a '.mjs' file, or a '.js' file where the package.json contains '"type": "module"'). The extension in the request is mandatory for it to be fully specified. Add the extension to the request. @ ../../../PROJECT_PATH/node_modules/@my-scope/my-package/src/mixins/token/unauthenticated.js 1:0-50 11:13-23 @ ../../../PROJECT_PATH/node_modules/@my-scope/my-package/src/client.js 3:0-66 32:15-27 @ ../../../PROJECT_PATH/node_modules/@my-scope/my-package/index.js 1:0-41 1:0-41 @ ./services/my-service.js 2:0-37 @ ./assets/ember-client.js 49:9-41

There are some issues on Babel that revolve around this, but they claim it was fixed by 7.12.0 (https://github.com/babel/babel/issues/12003, https://github.com/babel/babel/issues/12158).

There is a workaround so Webpack doesn't freak out on incorrect Babel transpiled code. Maybe it'd be good to add it to the default blueprint: https://github.com/webpack/webpack/issues/11467#issuecomment-808618999

Ps.: I can't disclosure the package code, but it's just some classes with methods and only two dependencies: buffer and cross-fetch. It works as expected on node.js.

ef4 commented 2 years ago

While I think it's fine for an app author to use fullySpecified: false as a workaround, adding it to the defaults is the kind of loose-by-default behavior that's hard to undo later across the whole ecosystem.

It looks like this is solved and/or getting solved upstream by changes to babel and/or core-js.

If I recall correctly, Ember apps using ember-cli-babel's polyfill option currently get an old core-js, which is problematic for other reasons too (like it lacks Object.fromEntries).

andsmedeiros commented 2 years ago

Maybe we should better document this then, at least until it is corrected. This error happened in a new 3.28 app.

MrChocolatine commented 2 years ago

I am facing the almost-same error (https://github.com/peopledoc/ember-reading-time/actions/runs/1409500930):

ERROR Summary:

  - broccoliBuilderErrorStack: ModuleNotFoundError: Module not found: Error: Can't resolve 'stream' in '/home/runner/work/ember-reading-time/ember-reading-time/node_modules/reading-time/lib/stream.js'
Did you mean './stream'?
Requests that should resolve in the current directory need to start with './'.
Requests that start with a name are treated as module requests and resolve within module directories (node_modules).
If changing the source code is not an option there is also a resolve options called 'preferRelative' which tries to resolve these kind of requests in the current directory too.

BREAKING CHANGE: webpack < 5 used to include polyfills for node.js core modules by default.
This is no longer the case. Verify if you need this module and configure a polyfill for it.

If you want to include a polyfill, you need to:
    - add a fallback 'resolve.fallback: { "stream": require.resolve("stream-browserify") }'
    - install 'stream-browserify'
If you don't want to include a polyfill, you can use an empty module like this:
    resolve.fallback: { "stream": false }
    at /home/runner/work/ember-reading-time/ember-reading-time/node_modules/webpack/lib/Compilation.js:2004:28
    at /home/runner/work/ember-reading-time/ember-reading-time/node_modules/webpack/lib/NormalModuleFactory.js:795:13
    at eval (eval at create (/home/runner/work/ember-reading-time/ember-reading-time/node_modules/webpack/node_modules/tapable/lib/HookCodeFactory.js:33:10), <anonymous>:8:1)
    at /home/runner/work/ember-reading-time/ember-reading-time/node_modules/webpack/lib/NormalModuleFactory.js:275:22
    at eval (eval at create (/home/runner/work/ember-reading-time/ember-reading-time/node_modules/webpack/node_modules/tapable/lib/HookCodeFactory.js:33:10), <anonymous>:7:1)
    at /home/runner/work/ember-reading-time/ember-reading-time/node_modules/webpack/lib/NormalModuleFactory.js:431:22
    at /home/runner/work/ember-reading-time/ember-reading-time/node_modules/webpack/lib/NormalModuleFactory.js:124:11
    at /home/runner/work/ember-reading-time/ember-reading-time/node_modules/webpack/lib/NormalModuleFactory.js:667:25
    at /home/runner/work/ember-reading-time/ember-reading-time/node_modules/webpack/lib/NormalModuleFactory.js:852:8
    at /home/runner/work/ember-reading-time/ember-reading-time/node_modules/webpack/lib/NormalModuleFactory.js:972:5
  - code: [undefined]
  - codeFrame: Module not found: Error: Can't resolve 'stream' in '/home/runner/work/ember-reading-time/ember-reading-time/node_modules/reading-time/lib/stream.js'

...

Out of curiosity I tried to set the Webpack option resolve.fullySpecified to false (https://github.com/peopledoc/ember-reading-time/commit/6dd34eb521bf8286d8727fb641eb9662600c4a4d) but it does not change anything, the error is still there.

ef4 commented 2 years ago

@MrChocolatine I think your problem is different than this issue. ember-reading-time is using a feature that only works on Webpack 4:

https://github.com/peopledoc/ember-reading-time/blob/be75bb98cbd1f88f42799c4534bc1795bc7b5a9e/index.js#L9

MrChocolatine commented 2 years ago

How unfortunate, thanks you took a look.

andsmedeiros commented 2 years ago

@MrChocolatine note however that you may probably get this working by following the instructions in the error message:

If you want to include a polyfill, you need to:

  • add a fallback 'resolve.fallback: { "stream": require.resolve("stream-browserify") }'
  • install 'stream-browserify'

This happens because Webpack used to provide automatic polyfills for core Node.js APIs before v5.x, such as the stream module. As @ef4 said, this feature is now deprecated.

MrChocolatine commented 2 years ago

@MrChocolatine note however that you may probably get this working by following the instructions in the error message:

If you want to include a polyfill, you need to:

  • add a fallback 'resolve.fallback: { "stream": require.resolve("stream-browserify") }'
  • install 'stream-browserify'

This happens because Webpack used to provide automatic polyfills for core Node.js APIs before v5.x, such as the stream module. As @ef4 said, this feature is now deprecated.

@andsmedeiros

Indeed, with a colleague we were actually investigating this.

However, because we still use ember-auto-import v1 we have to stick with Webpack v4, therefore it does not seem possible to bring this fix that works for Webpack v5?

andsmedeiros commented 2 years ago

I don't get it, the logs you posted were produced by Webpack v5.x. If you were to stick with v4.x this wouldn't even be an issue as polyfills are included. On a side note, IIRC, ember-auto-import v2.x is backwards compatible, so you shouldn't have trouble updating it.

MrChocolatine commented 2 years ago

I don't get it, the logs you posted were produced by Webpack v5.x. If you were to stick with v4.x this wouldn't even be an issue as polyfills are included. On a side note, IIRC, ember-auto-import v2.x is backwards compatible, so you shouldn't have trouble updating it.

Huh, true true... Embroider scenarios, Webpack 5 implied... I am not quite not sure why I said that about Webpack, I have been juggling with Embroider and tests all day so my brain was kind of melting after a while.

Anyway I found a way to fix my initial issue:

https://github.com/peopledoc/ember-reading-time/pull/13/commits/c55b4070223849bf60ab504aed6c5f2e377b96cc

I realise I am squatting your issue sorry, let me know if you want me to create another one so that I stop interfering with yours :v: .