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 conditional exports entries #137

Merged
merged 1 commit into from
Feb 4, 2023

Conversation

cometkim
Copy link
Contributor

No description provided.

@arcanis arcanis merged commit a9d9475 into arcanis:master Feb 4, 2023
@cometkim cometkim deleted the conditional-exports branch February 5, 2023 08:00
@arcanis
Copy link
Owner

arcanis commented Feb 6, 2023

@cometkim I had to revert this PR: #139

If we want to re-land it I think this time we'll need to add tests to make sure that the ESM build works fine (I took a quick look, and I think the idea would be to add an exports entry for ./platform which would point to either node.ts or browser.ts based on whether the node condition is set; Rollup might also need to be upgraded).

@cometkim
Copy link
Contributor Author

cometkim commented Feb 6, 2023

OK I will try again

@cometkim
Copy link
Contributor Author

cometkim commented Feb 6, 2023

@arcanis Can you push your reverted commit to master?

arcanis added a commit that referenced this pull request Feb 6, 2023
@arcanis
Copy link
Owner

arcanis commented Feb 6, 2023

Pushed 👍

@artechventure
Copy link

It seems that exports map in package.json inside subpath like ./platform is not well respected. What about using subpath imports? I think that will solve this platform problem better.

@cometkim
Copy link
Contributor Author

cometkim commented Feb 7, 2023

@arcanis New PR here #140

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.

3 participants