eatyourgreens commented 1 year ago

WHAT?? The tests for app-project and lib-classifier are failing on:

Error [ERR_REQUIRE_ESM]: require() of ES Module /Users/shaun/projects/front-end-monorepo/node_modules/d3/src/index.js from /Users/shaun/projects/front-end-monorepo/packages/app-project/src/shared/components/ProjectStatistics/components/CompletionBar/CompletionBar.js not supported.
Instead change the require of index.js in /Users/shaun/projects/front-end-monorepo/packages/app-project/src/shared/components/ProjectStatistics/components/CompletionBar/CompletionBar.js to a dynamic import() which is available in all CommonJS modules.

and

Error [ERR_REQUIRE_ESM]: require() of ES Module /Users/shaun/projects/front-end-monorepo/node_modules/d3/src/index.js from /Users/shaun/projects/front-end-monorepo/packages/lib-classifier/src/components/Classifier/components/Feedback/components/Graph2dRangeFeedback.js not supported.
Instead change the require of index.js in /Users/shaun/projects/front-end-monorepo/packages/lib-classifier/src/components/Classifier/components/Feedback/components/Graph2dRangeFeedback.js to a dynamic import() which is available in all CommonJS modules.

and etc

OK, so upon review:

D3 now ships as pure ES modules and requires Node.js 12 or higher. - this IS a problem.

Additional reading:

The awkward valley to ESM: Node.js, Victory, and D3 - 15 Jun 2022, formidable.com - TL;DR: as more libraries (e.g. d3.js) drop CommonJS support and go 100% ESM (ECMAScript Modules), a lot of upstream/downstream dependencies that use CommonJS get borked. (ESM setups can read ESM + CommonJS modules, but CommonJS setups can't read ESModules.) Awkward transpiling workarounds are required.
Importing Issues - Error [ERR_REQUIRE_ESM]: Must use import to load ES Module - 29 Jun 2021, D3 repo - TL;DR: lol good luck
import conflict with 3.0.0 update - 25 Apr 2021, D3 repo - TL;DR: lol not a d3 problem, fix your NextJS
- Yeah, but lib-classifier has the same error, and it's not running Next.JS. Also, I think our tests aren't running Next.JS?

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.

Originally posted by @shaunanoordin in https://github.com/zooniverse/front-end-monorepo/issues/3361#issuecomment-1171274486

eatyourgreens commented 1 year ago

Why our Mocha tests are using CommonJS to require modules, I do not know. Some Babel config rewriting import as require?

eatyourgreens commented 1 year ago

We could try adding regenerator-runtime/runtime to the Mocha config. https://stackoverflow.com/a/60522428

eatyourgreens commented 1 year ago

We could also try publishing the libraries as ES Modules, but need to be mindful of not breaking Zoo Notes or Mapping Viz.

eatyourgreens commented 1 year ago

Mocha Babel examples might be useful.

eatyourgreens commented 1 year ago

3900 adds ES modules to `@zooniverse/classifier` and `@zooniverse/react-components`, alongside the UMD modules built by webpack. Builds and deploys use the ES modules. Mocha tests require the UMD modules. Everything works but we need webpack builds, which are pretty big and slow, in order to run the tests.

3920 converts `@zooniverse/async-states`, which is a single JS file, from CommonJS to ES6. Builds that use it run fine, but tests fail for both NextJS apps and the classifier, when they try to `require` the ES module.

eatyourgreens commented 1 year ago

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.

goplayoutside3 commented 1 year ago

Going to close https://github.com/zooniverse/front-end-monorepo/pull/3781 for now, but linking here because of relation to ESM

eatyourgreens commented 1 year ago

@zooniverse/panoptes-js also uses require and module.exports (CommonJS.)

eatyourgreens commented 1 year ago

Shaun's ESM notes here are very useful.

https://github.com/zooniverse/community-catalog/issues/165

eatyourgreens commented 1 year ago

next-i18next breaks if you convert the config file to ES6.

https://github.com/i18next/next-i18next/issues/2185

There may be a workaround. See the conversation on that issue.

eatyourgreens commented 1 year ago

next-i18next also breaks if you rename the config to next-i18next.config.cjs.

eatyourgreens commented 12 months ago

Bun has a good explanation of Node module resolution and the differences between CommonJS and ESM.

https://bun.sh/docs/runtime/modules

eatyourgreens commented 5 months ago

Node 21 now has experimental support for require(esm).

https://joyeecheung.github.io/blog/2024/03/18/require-esm-in-node-js/

zooniverse / front-end-monorepo

Pure ES Modules break the Mocha tests #3779

Status

3920 converts `@zooniverse/async-states`, which is a single JS file, from CommonJS to ES6. Builds that use it run fine, but tests fail for both NextJS apps and the classifier, when they try to `require` the ES module.

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.

zooniverse / front-end-monorepo

Pure ES Modules break the Mocha tests #3779

Status

3920 converts @zooniverse/async-states, which is a single JS file, from CommonJS to ES6. Builds that use it run fine, but tests fail for both NextJS apps and the classifier, when they try to require the ES module.

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.

3920 converts `@zooniverse/async-states`, which is a single JS file, from CommonJS to ES6. Builds that use it run fine, but tests fail for both NextJS apps and the classifier, when they try to `require` the ES module.