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

Support Node.js ESM #39

Draft
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

kettanaito
Copy link

@CLAassistant
Copy link

CLA assistant check
Thank you for your submission! We really appreciate it. Like many open source projects, we ask that you sign our Contributor License Agreement before we can accept your contribution.
You have signed the CLA already but the status is still pending? Let us recheck it.

@kettanaito kettanaito changed the title Add "node.esm" build target Support Node.js ESM Oct 17, 2024
@kettanaito
Copy link
Author

Hi, @pimterry 👋 Can you please help me with the module tests for this change? I'm not entire familiar with what you are using for that. Thanks!

@pimterry
Copy link
Member

Can you please help me with the module tests for this change? I'm not entire familiar with what you are using for that. Thanks!

There's no formal framework. There's a single file of Mocha tests in test/, written in TypeScript, and there's scripts and to launch that same test file in various different ways for each env in package.json.

We'd need a new script there, which runs those tests in Node via ESM.

I'm not sure what the best approach to do that is - the only constraint is sharing the same test code (I'd really like to avoid duplicating this) but if you want to make separate Node vs Node-ESM entrypoints then that would be fine. Changes to the cross-compat code at the start of the test file (or even splitting it from the test implementations) is fine too, up to you.

Since this isn't usable in old Node anyway, you could skip these tests in that case, and in that case you could also potentially use --experimental-strip-types to handle the TypeScript part.

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.

Incorrect "exports" in package.json
3 participants