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

Required changes to support nodenext #2

Merged
merged 2 commits into from
Jan 17, 2023

Conversation

bytenik
Copy link
Contributor

@bytenik bytenik commented Jan 11, 2023

nodenext style resolution ignores the top-level types reference when there's an exports. A types in the various exports is required.

Also, in a multi-type module like this, where you offer both import and require, don't set the type field. The type field forces TypeScript module resolution to see the whole package as a module before it even evaluates the nodenext. This isn't either module or commonjs; its both, depending on the consumer. Even without nodenext, setting main and module properties handles this correctly without the type.

`nodenext` style resolution ignores the top-level `types` reference when there's an `exports`. A `types` in the various exports is required.
In a multi-type module like this, where you offer both import and require, don't set the `type` field. The `type` field forces TypeScript module resolution to see the whole package as a `module` before it even evaluates the `nodenext`.
@bytenik
Copy link
Contributor Author

bytenik commented Jan 17, 2023

@petrzjunior Hi! What are the odds of getting this merged this week? We unfortunately have to currently run with a patched version of the module in order to get our project to build.

@petrzjunior
Copy link
Owner

@bytenik Thank you for your contribution, I'll merge it ASAP, sorry for the delay.

@petrzjunior petrzjunior merged commit d422483 into petrzjunior:master Jan 17, 2023
@petrzjunior
Copy link
Owner

Published in version 1.3.1.
@bytenik Let me know if everything works fine now. I see that there is still a running discussion over in TS repo.

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.

2 participants