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 ESM exports in package.json #32

Closed
wants to merge 1 commit into from
Closed

fix ESM exports in package.json #32

wants to merge 1 commit into from

Conversation

OmarTawfik
Copy link

@OmarTawfik OmarTawfik commented May 16, 2023

Initial checklist

  • I read the support docs
  • I read the contributing guide
  • I agree to follow the code of conduct
  • I searched issues and couldn’t find anything (or linked relevant results below)
  • [N/A] If applicable, I’ve added docs and tests

Description of changes

Following up on 309f2a4 .. cc @wooorm

Per the NodeJS Spec: https://nodejs.org/api/packages.html#package-entry-points

When using the "exports" field, custom subpaths can be defined along with the main entry point by treating the main entry point as the "." subpath

This PR updates package.json to use the new syntax (starting with a dot).

It also export the lib/index.js path, which is required to properly propagate the generated types from lib/index.d.ts to the root index.d.ts that re-exports its. This is already breaking builds in dependents that use "moduleResolution": "node16" in their tsconfig.json. Example I found today in remark-parse latest version:

node_modules/remark-parse/lib/index.d.ts:3:26 - error TS2307: Cannot find module 'mdast-util-from-markdown/lib' or its corresponding type declarations.

3   options: void | import('mdast-util-from-markdown/lib').Options | undefined

Found 1 error in node_modules/remark-parse/lib/index.d.ts:3

@github-actions github-actions bot added 👋 phase/new Post is being triaged automatically 🤞 phase/open Post is being triaged manually and removed 👋 phase/new Post is being triaged automatically labels May 16, 2023
@wooorm
Copy link
Member

wooorm commented May 16, 2023

Hey! I think this is something else!

The changes you propose could be used (although: they break every package using this).
What you specify is different paths. Some could use import {} from 'x/development' to get dev code and import {} from 'x' to get prod code, that’s what you propose.

The existing code exists to do different things. They use conditions: https://nodejs.org/api/packages.html#conditional-exports. Someone can do this to get development code, from the outside.

This is already breaking builds in dependents that use "moduleResolution": "node16" in their tsconfig.json. Example I found today in remark-parse latest version:

I believe that to be a different error.

The problem is that remark-parse was last published (2 years ago) before "moduleResolution": "node16" was a thing (it’s not even in remark yet: https://github.com/remarkjs/remark/blob/main/tsconfig.json).
2 years ago, the TS CLI generated code that doesn’t work in todays TS with moduleResolution: node16.

I believe that the solution is to use a current correct TS config (e.g., like this: https://github.com/syntax-tree/mdast-util-from-markdown/blob/cd10e598ae2492c8b61131d2ace46e0ea7cc37e5/tsconfig.json), then regenerate all types in remark, and cut a new release

@OmarTawfik
Copy link
Author

The existing code exists to do different things. They use conditions: https://nodejs.org/api/packages.html#conditional-exports. Someone can do this to get development code, from the outside.

I see. I didn't know about that. Thanks for the information!

Will close this PR and look into remark-parse. Thanks!

@OmarTawfik OmarTawfik closed this May 16, 2023
@OmarTawfik OmarTawfik deleted the fix-exports branch May 16, 2023 12:28
@github-actions

This comment was marked as resolved.

@wooorm wooorm added the 🤷 no/invalid This cannot be acted upon label May 16, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
🤷 no/invalid This cannot be acted upon 🤞 phase/open Post is being triaged manually
Development

Successfully merging this pull request may close these issues.

None yet

2 participants