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

Fix @glimmer/core exports for node.js ESM and bundlers #354

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

izelnakri
Copy link
Contributor

@izelnakri izelnakri commented Jun 25, 2021

When I tried to run @emberx/component for its qunitx node.js mode tests, I received this following error:

ImportError: File: /home/izelnakri/Github/emberx/tmp/@emberx/component/create-template-test.js | SyntaxError: Named export 'renderComponent' not found. The requested module '@glimmer/core' is a CommonJS module, which may not support all module.exports as named exports.
# CommonJS modules can always be imported via the default export, for example using:
#
# import pkg from '@glimmer/core';
# const { renderComponent: glimmerRenderComponent, didRender, setComponentTemplate, getOwner, templateOnlyComponent } = pkg;

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:

import glimmerComponent from '@glimmer/component';
import glimmerCore from '@glimmer/core'; // NOTE: here I target a default import instead

// @ts-ignore
let Component = glimmerComponent.default ? glimmerComponent.default : glimmerComponent;
let { didRender, setComponentTemplate, getOwner, templateOnlyComponent } = glimmerCore;
let glimmerRenderComponent = glimmerCore.renderComponent;

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 and qunitx --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:

> node_modules/.bin/qunitx test/index.ts --browser --failFast

 > packages/@emberx/component/dist/index.js:7:7: error: No matching export in "node_modules/@glimmer/core/dist/modules/index.js" for import "default"
    7 │ import glimmerCore from '@glimmer/core'; // @ts-ignore
      ╵        ~~~~~~~~~~~

Error: Build failed with 1 error:
packages/@emberx/component/dist/index.js:7:7: error: No matching export in "node_modules/@glimmer/core/dist/modules/index.js" for import "default"

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 on d.ts, so we still need to build with tsc and then do an additional babel 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 with jsdom or simple-dom if the libraries needs the dom or window.

@izelnakri izelnakri changed the title Fix @glimmer/core exports for ESM/bundlers Fix @glimmer/core exports for node.js ESM and bundlers Jun 25, 2021
@izelnakri izelnakri force-pushed the fix-glimmer-core-exports branch 3 times, most recently from db4fe7c to c57e6ef Compare June 25, 2021 15:15
Previously @glimmer/core build outputs on commonjs
and modules folders were not universally runnable
interchangeably on bundlers are node.js runtimes.
This commit fixes by introducing default export
in addition to named exports.
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.

None yet

1 participant