This repository has been archived by the owner on Oct 29, 2024. It is now read-only.
Fix @glimmer/core exports for node.js ESM and bundlers #354
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
When I tried to run
@emberx/component
for itsqunitx
node.js mode tests, I received this following error:This is the default error node.js prints when one tries to import the file or a @glimmer/core dependent file on node.js ESM mode currently. QUnitx just does an
await import()
of the same file with querystring to bust the cache on each refresh on node.js mode. So I changed my library imports to these one default import and then destructure the named imports from these, then node.js mode tests passed:However when I run the same lib code & my test code in qunitx node.js
--browser
mode(which transpiles typescript to js with esbuild for browsers) I got another error. I try to run the@emberx
packages in node.js ESM mode andqunitx --browser
mode on CI to make sure the code I write is universally runnable. QUnitX run the same test files in node.js or puppeteer(which depends on esbuild bundler) depending on the --browser flag:Then I went ahead to their actual references and when I do the changes that I propose in this PR, everything worked. Node.js ESM mode can import commonjs modules today, however it prints the first error when everything in the dependent module is named commonjs exports and the ESM code tries to import the named exports as if its from a default exports object.
Today esbuild and many other bundlers pick up the
"modules"
reference of the library when bundling, node.js picks up the commonjs("main"
) reference in the package.json. So having an export that supports both formats on their transpiled output fixes this issue.Ideally in the future, we could/should build a node.js ESM compliant format for the libraries. It would mean importing relative files/modules with explicit file extensions, we could do this later on if everyone agrees. Currently only
tsc
builds output the respective types ond.ts
, so we still need to build withtsc
and then do an additionalbabel
transpilation to make relative imports with explicit file extensions.babel-plugin-module-extension-resolver
allows for this.Having libraries distributed with explicit relative file imports makes them more universal, as they could be run on node.js natively(when it has "type": "module") and on even deno if asset map to the named package import is provided. Recently I built qunitx to make even test file and frontend code runnable on both node.js and browser environments with no changes to the test or library files, maybe a
--before
flag is needed to setup up a dom withjsdom
orsimple-dom
if the libraries needs the dom orwindow
.