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

Add exports.types to package.json #3

Closed
wants to merge 1 commit into from

Conversation

prichey
Copy link

@prichey prichey commented Jan 24, 2023

Resolves codemirror/dev#891

Codemirror is not able to compile since TypeScript 4.7 with "moduleResolution": "node16" enabled, because it is unable to find types associated with the ESM export, ./dist/index.js. This PR adds the proper types export, which should be backwards compatible with both Node and TS runtimes.

More context on that here and here.

I'm happy to submit PRs to all the other repos if this approach works. Thanks for the consideration!

@prichey
Copy link
Author

prichey commented Jan 26, 2023

@marijnh Any objections to pulling this in?

@marijnh
Copy link
Member

marijnh commented Jan 27, 2023

I've been holding off on this because node16 resolution is (was?) unstable and it seemed like this kind of breakage might be a temporary bug. But indeed, they seem to have made it worse in 4.9. The syntax used in this PR is different from what the announcement shows (though definitely less verbose, so in principle more palatable)—do we know for sure that this is going to keep working as they stabilize it? See also the apparently even different new resolution method in TypeScript 5. What I'd really like to avoid is to have these kinds of changes to all the CodeMirror packages every three weeks as TypeScript breaks or fixes or changes its oddly picky new resolution algorithms.

@rix0rrr
Copy link

rix0rrr commented Jan 4, 2024

In May of 2022, a TypeScript dev commented that this behavior is on purpose, and they consider a missing export.types field a packaging mistake.

Please reconsider merging this? Not being able to use node16 is causing us pain trying to mix CJS and ESM modules together.

@marijnh
Copy link
Member

marijnh commented Jan 4, 2024

This has been fixed in another way (making the type declaration file names correspond to TS's expectations).

@marijnh marijnh closed this Jan 4, 2024
@rix0rrr
Copy link

rix0rrr commented Jan 4, 2024

So it is, looks like we were on an old version. Thanks!

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.

TS and node16 modules
3 participants