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
build: Switch from CJS to ESM #1160
Conversation
Codecov Report
@@ Coverage Diff @@
## main #1160 +/- ##
==========================================
- Coverage 64.56% 62.07% -2.50%
==========================================
Files 59 59
Lines 7677 7111 -566
Branches 1714 1708 -6
==========================================
- Hits 4957 4414 -543
+ Misses 2617 2614 -3
+ Partials 103 83 -20
Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here. |
Deploying with Cloudflare Pages
|
📊 PerformanceKeyNote that each bar component rounds up to the nearest 100ms, so each full bar is an overestimate by up to 400ms.
Data
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM! lodash should really just be esm 🤡
Description
While working on #1092, I realized that it would be very nice to have access to top-level await. However, that is only supported by ESM and not CJS, so I decided to first open this PR to migrate us to ESM.
Implementation strategy and design decisions
This was mostly just a lot of trial-and-error, digging through documentation, and playing with config options until it worked. I got a lot of help from the following threads:
require()
calls of Node built-ins to import statements when emitting ESM for Node evanw/esbuild#2067By volume, most of the code changes in this PR are just switching imports of CJS modules to only use the default import rather than trying to destructure it, because apparently the latter is not actually supported when importing a CJS module from an ESM module.
One other change: because we need the
global-jsdom
import to come first inautomator
andsynthesizer
, I upgraded us to TypeScript 4.9.4 and prettier-plugin-organize-imports 3.2.1, to give us grouped imports which were added in TypeScript 4.7. These upgrades caused Prettier to start trying to remove a bunch of React imports, so I added"organizeImportsSkipDestructiveCodeActions": true
to our.prettierrc
.Examples with steps to reproduce them
I manually verified that this works by running
yarn start
with a top-levelawait
inpackages/core/src/index.ts
, but removed that before submitting this PR because we don't actually use it yet. Note that to get that to actually work, you have to also set the build target to ESNext for all our Vite packages:Checklist
diagrams/
folderOpen questions
Although
yarn start:docs-site
works with this PR, it does not work after applying the top-level await diff given above. Rather, it crashes with this error message:I propose to merge this PR now and then figure out how to do top-level await with Docusaurus in a followup.