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

Define exports entrypoints using default #169

Merged
merged 2 commits into from
Dec 14, 2024

Conversation

aduth
Copy link
Contributor

@aduth aduth commented Dec 6, 2024

Updates the package.json exports to set default entrypoints using default instead of import.

Reference documentation: https://nodejs.org/api/packages.html#conditional-exports

"import" limits use to ESM environments. As of Node.js v22 (current LTS), there's now improved support to allow CommonJS to use ESM-only projects like this one.

Without these changes, a CommonJS project trying to import this library will encounter the following error:

 Exception during run: Error: No "exports" main defined in /Code/my-project/node_modules/clipboard-polyfill/package.json

(This is actually a long-delayed follow-up to #148 (comment), where our project is still CommonJS, but we'd love to be able to upgrade with this new capability in Node.js v22)

@lgarron
Copy link
Owner

lgarron commented Dec 7, 2024

Hmm, how widely is this supported in the ecosystem? Is it possible / safe to have both an "import" and "default" field?

(I actually really like this change, thanks for teaching me about it. But I've faced a lot of very unfortunate bundler assumptions over the years that can make it a lot of work to adopt these kind of changes. Also, since this is mainly a browser library bundler support is a bigger concern than runtime support.)

@lgarron
Copy link
Owner

lgarron commented Dec 7, 2024

Aha!

  • "default" - the generic fallback that always matches. Can be a CommonJS or ES module file. This condition should always come last.
    Within the "exports" object, key order is significant. During condition matching, earlier entries have higher priority and take precedence over later entries. The general rule is that conditions should be from most specific to least specific in object order.

@aduth
Copy link
Contributor Author

aduth commented Dec 9, 2024

Yeah, my understanding is that it ought to work same as it does today, but with CommonJS compatibility.

If you're concerned, a more conservative option could be to leave things as they are, and add an additional require exports condition pointing to the same file as imports.

I couldn't find much about the types (presumably TypeScript) field and how that works to avoid regressions, aside from some text about it in the 4.7 release notes, which does at least say it ought to come first in the list.

@lgarron
Copy link
Owner

lgarron commented Dec 9, 2024

Hmm, I'm not really sure what to do here. On the one hand, if this makes life easier for CommonJS folks, I don't mind it.

On the other hand, I have completely removed CommonJS code from my life. The package is ESM, and any compatibility with loading from CommonJS is incidental.

I also didn't know the ordering was semantic, which is something supported by all JavaScript runtime, but as far as I know is not a feature of JSON in general. So I don't know if I might be making more work with myself down the line by adding a "default" field due to inconsistent preservation of field ordering. (For example, Rust tools will lose the order if they use a regular HashMap instead of an ordered one) I've already spent countless hours working around all sorts of bundler issues, and this sets off my spidey senses.

I guess what I can do is merge and publish this with "import" falling back to "default", but I may revert it at any time with no notice if it causes any new issues that would take debugging. Does that sound reasonable?

@aduth
Copy link
Contributor Author

aduth commented Dec 12, 2024

I guess what I can do is merge and publish this with "import" falling back to "default", but I may revert it at any time with no notice if it causes any new issues that would take debugging. Does that sound reasonable?

Works for me 👍

@lgarron lgarron merged commit 864a60f into lgarron:main Dec 14, 2024
1 check passed
@lgarron
Copy link
Owner

lgarron commented Dec 14, 2024

Published in v4.1.1, thanks!

@aduth aduth deleted the aduth-package-exports-default branch December 16, 2024 18:36
@aduth
Copy link
Contributor Author

aduth commented Dec 16, 2024

Thanks @lgarron, seems to work great in our CommonJS project now!

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