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 package.json exports #98

Closed
wants to merge 1 commit into from

Conversation

sagargurtu
Copy link

Adding exports field in package.json to make it easier for modern bundlers to properly bundle this package.

Copy link
Member

@ljharb ljharb left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Adding exports is always a breaking change, and I'm not sure why a modern bundler would have any issue with a package that lacks exports (especially since that's most of them)

@sagargurtu
Copy link
Author

Apologies - I should have been clearer in the initial description. I'm currently developing a Vite plugin that relies on custom resolution logic and I noticed that this line disrupts that behavior. I thought this could be a good opportunity to introduce the exports field in package.json, as it's a more modern and explicit approach for defining entry points.

I do realize this would be a breaking change, but I'm happy to ensure all entry points are properly mapped in the structure to maintain compatibility. If the necessary non-breaking changes are made, would this pull request be good to go, or would you prefer that I withdraw this change entirely? I'm happy to follow whichever direction works best.

@ljharb
Copy link
Member

ljharb commented Jan 4, 2025

If your plugin can't handle arbitrary paths in an exports-less package, then it's broken. While I definitely would want to eventually add exports to this package, I don't think this is a sufficiently motivating rationale.

@sagargurtu
Copy link
Author

Sounds good. Closing this PR.

@sagargurtu sagargurtu closed this Jan 4, 2025
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