search

amcharts / amcharts5

The newest, fastest, and most advanced amCharts charting library for JavaScript and TypeScript apps.
Other
353 stars 94 forks source link

Add support as ECMAScript module (e.g add `type: module` to `package.json`; `.js` file extension to relative imports) #1265

Open davidjb opened 10 months ago

davidjb commented 10 months ago

Question

"type": "module"

Could "type": "module" be added to amCharts' package.json? As I understand it, the package is entirely ES modules so adding this flag tells Node that this package's .js files are all ESM (ref: https://nodejs.org/api/packages.html#type).

This would assist with importing amCharts directly in supported environments, such as testing with Jest. I know there's a setup process (https://github.com/amcharts/amcharts5/issues/1223#issuecomment-1850069653) for configuring Jest/Babel transformations, but with the addition of "type": "module", transformations aren't required and amCharts5 can be used directly (I manually edited amCharts' package.json to test). This is provided Node is run with Jest/ESM support ala https://jestjs.io/docs/ecmascript-modules.

Example test running successfully with manual modification of package.json: https://gist.github.com/davidjb/6c6492848dc924a9551b1e2d97f8a847

File extensions in imports

Full direct support as an ES module in Node would require the addition of file extensions to relative imports in the codebase (as per https://nodejs.org/api/esm.html#mandatory-file-extensions) - to my knowledge, this shouldn't affect any build tools or be a breaking change - but is a large change regardless.

Fwiw, Jest's resolver doesn't have an issue with missing file extensions and only needs "type": "module" to work as it is today.

Environment (if applicable)

Additional context

Pauan commented 10 months ago

We had tried adding "type": "module" before, but it caused lots of problems, because various tools and build systems aren't designed to handle it properly. It's something we want to do, but we can't do it until it's fully supported everywhere.

davidjb commented 10 months ago

Thanks for the info @Pauan. Hopefully this is something that can be enabled soon.

Fwiw, patching in "type": "module" is working fine with Parcel and Jest, and almost with Node (imports fail without file extensions, but it would appear it'd work otherwise).

Noting in case anyone else wants to test amCharts with ESM & Jest:

  1. Enable ESM support: https://jestjs.io/docs/ecmascript-modules
  2. Patch amCharts' package.json to add "type": "module" (I used patch-package from NPM)
  3. Install jest-canvas-mock and jest-environment-jsdom as devDependencies
  4. Set Jest up with: 
    "testEnvironment": "jsdom",
"setupFiles": ["jest-canvas-mock"]

    and that's it - working on the latest versions of Jest and amCharts at the time of writing.

workingbuddy10 commented 10 months ago

Thanks for the info @Pauan. Hopefully this is something that can be enabled soon.

Fwiw, patching in "type": "module" is working fine with Parcel and Jest, and almost with Node (imports fail without file extensions, but it would appear it'd work otherwise).

Noting in case anyone else wants to test amCharts with ESM & Jest:

  1. Enable ESM support: https://jestjs.io/docs/ecmascript-modules
  2. Patch amCharts' package.json to add "type": "module" (I used patch-package from NPM)
  3. Install jest-canvas-mock and jest-environment-jsdom as devDependencies
  4. Set Jest up with: 
    "testEnvironment": "jsdom",
"setupFiles": ["jest-canvas-mock"]

and that's it - working on the latest versions of Jest and amCharts at the time of writing.

Thank you for this flow ! @davidjb 💯