Skip to content

fix!: don't delete "main" in package.json #7

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

Merged
merged 2 commits into from
Mar 6, 2021

Conversation

rvagg
Copy link
Collaborator

@rvagg rvagg commented Mar 5, 2021

esbuild still doesn't support exports evanw/esbuild#187
it's uncertain as yet what impact this might have on other bundlers and
loaders, hence this is a BREAKING CHANGE.

esbuild still doesn't support exports evanw/esbuild#187
it's uncertain as yet what impact this might have on other bundlers and
loaders, hence this is a BREAKING CHANGE.
@mikeal
Copy link
Owner

mikeal commented Mar 5, 2021

This won’t work on its own, the built package doesn’t have an index.js in the root, it has two directories for the esm and cjs builds, we’ll need to re-write this to ‘cjs/index.js’.

@rvagg
Copy link
Collaborator Author

rvagg commented Mar 6, 2021

doh! of course .. and I even did this for a pkg I manually fixed & published.
I've pushed a fixup commit that does this, it should be squashed in if/when merged.

@@ -53,8 +53,7 @@ class Package {

const exports = {}
if (!json.exports) {
if (!json.main) exports['.'] = { import: this.file(toURL('./index.js')) }
exports['.'] = { import: this.file(toURL(json.main)) }
exports['.'] = { import: this.file(toURL(json.main || './index.js')) }
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

noticed the above logic was a bit broken so I fixed it while in here, the if has no effect and the second line would be calling toURL(undefined) if there's no json.main.

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