Skip to content
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

chore!: migrate package to ESM #675

Merged
merged 1 commit into from
Oct 11, 2023
Merged

chore!: migrate package to ESM #675

merged 1 commit into from
Oct 11, 2023

Conversation

oyvinmar
Copy link
Contributor

What

This change transitions the package to be distributed as an ESM (ECMAScript Module) instead of CommonJS. The main change is the output of ESM-compatible code by the TypeScript compiler and the modification of the module type in package.json.

Given that most bundlers now support ESM-distributed packages, I decided to forgo adding backward compatibility for CommonJS.

BREAKING CHANGE: The package is now distributed as an ESM module

Why

The combination of CommonJS and Vite caused problems when introducing rxbeach in surveys (see: https://github.com/ardoq/ardoq-packages/pull/6393). Changing to ESM seems to have fixed the problem.

Test

I've tested the changes in both ardoq-surveys and ardoq-front with the help of https://github.com/mweststrate/relative-deps. Will deploy and test in the test environment once released.

This change transitions the package to be distributed as an ESM (ECMAScript Module)
instead of CommonJS. The main change is the output of ESM-compatible code by the
TypeScript compiler and the modification of the module type in `package.json`.

Given that most bundlers now support ESM-distributed packages, I decided to forgo
adding backward compatibility for CommonJS.

BREAKING CHANGE: The package is now distributed as an ESM module
Copy link
Contributor

@sseppola sseppola left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As long as you've tested this manually it LGTM 👍
Make sure you run the test suite as well, I had to do some work to get rxbeach to work smoothly with jest at some point.

Copy link
Contributor

@OyvindSabo OyvindSabo left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Tested in ardoq-surveys and ardoq-front🤓

Copy link

@chriskr chriskr left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not tested anything, but LGTM.

@oyvinmar oyvinmar merged commit 87e42ad into master Oct 11, 2023
3 checks passed
@oyvinmar oyvinmar deleted the migrate-to-esm-module branch October 11, 2023 08:00
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants