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 package.json exports for ts-node/register #1026

Closed
wants to merge 1 commit into from

Conversation

addaleax
Copy link

@addaleax addaleax commented May 3, 2020

Commit f6cd5d4 broke require('ts-node/register') because
exports: provides an exclusive list of possible entry points,
and makes importing through other entry points impossible.

In particular, this breaks node-tap with TypeScript support.

Refs: https://medium.com/@forbeslindesay/is-promise-post-mortem-cab807f18dcc

@cspotcode Would be cool to see a release very very soon in order to un-break CI for lots of people :)

@addaleax
Copy link
Author

addaleax commented May 3, 2020

Also, if you’re interested nodejs/node#33074 contains proposed wording for Node’s ESM docs that would warn about this :)

package.json Outdated Show resolved Hide resolved
@addaleax addaleax force-pushed the fix-exports-register branch from 3883721 to 5806ede Compare May 3, 2020 06:42
Commit f6cd5d4 broke `require('ts-node/register')` because
`exports:` provides an *exclusive* list of possible entry points,
and makes importing through other entry points impossible.

In particular, this breaks node-tap with TypeScript support.

Refs: https://medium.com/@forbeslindesay/is-promise-post-mortem-cab807f18dcc
@cspotcode
Copy link
Collaborator

cspotcode commented May 3, 2020

8.10.1 is out, should be fixed via #1027.

I plan to revisit this PR when I have time to learn more about the potential pitfalls of package.json "exports"

@addaleax thanks again for the quick bug report and PR!

@cspotcode
Copy link
Collaborator

In the future, we might tweak our tests to be more E2E-style, so we npm pack and then npm install into a temp project directory. This allows testing the ts-node bin entrypoint and require('ts-node/*').

@addaleax
Copy link
Author

addaleax commented May 3, 2020

@cspotcode Just fyi, it looks like there is actually an Easy Way Around This™ that just needs to be documented: https://twitter.com/MylesBorins/status/1257058330457133058

@cspotcode
Copy link
Collaborator

@addaleax thanks, yeah, that seems to work. So I guess each key in the "exports" dictionary is more of a prefix, and anything starting with that prefix will match?

Looks like the proper exports declaration for us is:

  "exports": {
    "./": "./",
    "./esm": "./esm.mjs"
  }

@addaleax
Copy link
Author

addaleax commented May 4, 2020

@cspotcode I’m not an expert here either, but that’s how I would read https://nodejs.org/api/esm.html#esm_subpath_exports, yes. 👍

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.

3 participants