-
Notifications
You must be signed in to change notification settings - Fork 30
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
Pure ES Modules break the Mocha tests #3779
Comments
Why our Mocha tests are using CommonJS to require modules, I do not know. Some Babel config rewriting |
We could try adding |
We could also try publishing the libraries as ES Modules, but need to be mindful of not breaking Zoo Notes or Mapping Viz. |
Mocha Babel examples might be useful. |
#3900 adds ES modules to #3920 converts |
#4064 removed the webpack UMD builds. The classifier and Zooniverse React Components are published as ESM, for the NextJS builds, and CommonJS for the Mocha tests. |
Going to close #3781 for now, but linking here because of relation to ESM |
|
Shaun's ESM notes here are very useful. |
There may be a workaround. See the conversation on that issue. |
|
Bun has a good explanation of Node module resolution and the differences between CommonJS and ESM. |
Node 21 now has experimental support for https://joyeecheung.github.io/blog/2024/03/18/require-esm-in-node-js/ |
https://nodejs.org/en/blog/release/v23.0.0 I think you can enable it with a flag in Node 22, which will enter LTS next week. Top level await is still restricted to modules.
|
How to convert CommonJS to ESM, just published yesterday. |
If you switch to Node 22, then It also looks like module loading has changed in Node 22. This warning is from @zooniverse/content:test
$ mocha --config ./test/.mocharc.json ./.storybook/specConfig.js "./src/**/*.spec.js"
(node:69592) [MODULE_TYPELESS_PACKAGE_JSON] Warning: Module type of file:///Users/jimodonnell/zooniverse/front-end-monorepo/packages/lib-content/test/setup.js is not specified and it doesn't parse as CommonJS.
Reparsing as ES module because module syntax was detected. This incurs a performance overhead.
To eliminate this warning, add "type": "module" to /Users/jimodonnell/zooniverse/front-end-monorepo/packages/lib-content/package.json.
(Use `node --trace-warnings ...` to show where the warning was created) |
|
I was investigating this mocha config x ESM issue yesterday and want steer this Github discussion toward potential solutions for the monorepo, ideally without completely change our testing architecture. Please correct me if I'm wrong, but in summary:
@eatyourgreens you've left a few comments with potential solutions. Did you try any of these combos?
|
I just ran a very quick experiment with Node 22: nvm use 22
yarn install --ignore-scripts
yarn bootstrap The project app build fails with: unhandledRejection TypeError: Cannot set properties of undefined (setting 'eio')
at /Users/jimodonnell/zooniverse/front-end-monorepo/node_modules/panoptes-client/lib/SugarClient/primus.js:3805:9
at Array.<anonymous> (/Users/jimodonnell/zooniverse/front-end-monorepo/node_modules/panoptes-client/lib/SugarClient/primus.js:3806:3)
at UMDish (/Users/jimodonnell/zooniverse/front-end-monorepo/node_modules/panoptes-client/lib/SugarClient/primus.js:4:15)
at Object.<anonymous> (/Users/jimodonnell/zooniverse/front-end-monorepo/node_modules/panoptes-client/lib/SugarClient/primus.js:11:3)
at Module._compile (node:internal/modules/cjs/loader:1546:14)
at Object..js (node:internal/modules/cjs/loader:1689:10)
at Module.load (node:internal/modules/cjs/loader:1318:32)
at Function._load (node:internal/modules/cjs/loader:1128:12)
at TracingChannel.traceSync (node:diagnostics_channel:315:14)
at wrapModuleLoad (node:internal/modules/cjs/loader:218:24) {
type: 'TypeError'
} |
This looks similar to the |
One problem the monorepo has is that both JS and JSX files use the If you look at PFE, for example, I think JSX files use |
Oh, looks like I already tried Node 22 back in October: #3779 (comment) |
Right, that's why I was curious if you'd tried Node v22 while also adjusting |
Adding @zooniverse/content:test
$ mocha --config ./test/.mocharc.json ./.storybook/specConfig.js "./src/**/*.spec.js"
Exception during run: TypeError: Cannot set property navigator of #<Object> which has only a getter
at file:///Users/jimodonnell/zooniverse/front-end-monorepo/packages/lib-content/test/setup.js:43:18
at ModuleJob.run (node:internal/modules/esm/module_job:268:25)
at async onImport.tracePromise.__proto__ (node:internal/modules/esm/loader:543:26)
at async formattedImport (/Users/jimodonnell/zooniverse/front-end-monorepo/node_modules/mocha/lib/nodejs/esm-utils.js:9:14)
at async exports.requireOrImport (/Users/jimodonnell/zooniverse/front-end-monorepo/node_modules/mocha/lib/nodejs/esm-utils.js:42:28)
at async exports.loadFilesAsync (/Users/jimodonnell/zooniverse/front-end-monorepo/node_modules/mocha/lib/nodejs/esm-utils.js:100:20)
at async singleRun (/Users/jimodonnell/zooniverse/front-end-monorepo/node_modules/mocha/lib/cli/run-helpers.js:162:3)
at async exports.handler (/Users/jimodonnell/zooniverse/front-end-monorepo/node_modules/mocha/lib/cli/run.js:375:5)
error Command failed with exit code 1.
info Visit https://yarnpkg.com/en/docs/cli/run for documentation about this command. Modules run in strict mode, so that's probably the source of that error. |
Removing the offending line, then running the @zooniverse/content:test
$ mocha --config ./test/.mocharc.json ./.storybook/specConfig.js "./src/**/*.spec.js"
Exception during run: SyntaxError[ @/Users/jimodonnell/zooniverse/front-end-monorepo/packages/lib-content/.storybook/specConfig.js ]: Unexpected token '<'
at compileSourceTextModule (node:internal/modules/esm/utils:340:16)
at ModuleLoader.moduleStrategy (node:internal/modules/esm/translators:102:18)
at #translate (node:internal/modules/esm/loader:433:12)
at ModuleLoader.loadAndTranslate (node:internal/modules/esm/loader:480:27)
error Command failed with exit code 1.
info Visit https://yarnpkg.com/en/docs/cli/run for documentation about this command. |
Mocha tests containing JSX with ESM suggests running Mocha with a custom Node loader, but I don't know how old that is or which versions of Node it was written for. @node-loader/babel looks like it's still maintained, so could be worth trying. |
I have to confess that currently I use Storybook with Playwright to run tests for JSX components in a pure ESM project, and it just works out-of-the-box. |
This seems like an excellent summary to me. I'd add a couple of points:
|
The Sugar client is crashing here, when it adds The Sugar client, and in fact the Panoptes JS Client in general, only runs in browsers, but monorepo code runs in Node and in browsers. So that might be causing problems when Something to be aware of in #6375 too. That PR is adding browser-only code to a library, |
#3920 converted |
I've started to track the third-party libraries that cause tests to fail in the top level description of this Issue. |
I tagged Dependabot PRs with ESM if they break the tests. I don’t know if that still happens. Labels are useful if you want to track a dynamic list of PRs. https://github.com/zooniverse/front-end-monorepo/issues?q=label%3AESM |
Updated 2024 - List of third-party packages this applies to:
(Dependabot has been told to ignore the major version upgrades)
Originally posted by @shaunanoordin in #3361 (comment) The tests for
app-project
andlib-classifier
are failing on:and
and etc
OK, so upon review:
Additional reading:
Status
🤷 throw_hands_up_emoji
Good gravy, I need to check with the other devs to see if anybody else has encountered the ESM import issues. I might have missed something someone on the team is already aware of, since this doesn't seem to be a niche problem.
The text was updated successfully, but these errors were encountered: