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 module field for ESM distribution #2

Merged
merged 1 commit into from Jun 24, 2019
Merged

Add module field for ESM distribution #2

merged 1 commit into from Jun 24, 2019

Conversation

FND
Copy link
Contributor

@FND FND commented Jun 21, 2019

As far as I can tell, jsnext:main is deprecated in favor of module (see https://github.com/rollup/rollup/wiki/pkg.module and rollup/rollup-plugin-node-resolve#209).

Just to be safe, I decided to duplicate rather than replace the jsnext:main field - not sure whether that's actually necessary though.

@btd
Copy link
Member

btd commented Jun 24, 2019

Well, first of all thank you for PR. I can merge it if you need for example for your profile, but actually this PR is meaningless. I do not support shouldjs modules outside of should.js.

@FND
Copy link
Contributor Author

FND commented Jun 24, 2019

Anything's fine by me - just for context:

I do not support shouldjs modules outside of should.js

A user of @faucet-pipeline (which uses Rollup under the hood) wanted to include shouldjs in a single JavaScript bundle (not sure why or whether that's ever a good idea). import should from "should" resulted in Rollup complaining:

'hasOwnProperty' is not exported by node_modules/should-util/cjs/should-util.js"

So I guess you never know how modules are gonna be used in the wild. ¯\_(ツ)_/¯

Having said that, we've since fixed support for jsnext:main in faucet, so this isn't an immediate issue anymore. Proposing an upstream patch just seemed like the right thing to do, being a good citizen and all.

@btd
Copy link
Member

btd commented Jun 24, 2019

Ah, ok. Will merge then

@btd btd merged commit 8c8a3e6 into shouldjs:master Jun 24, 2019
@FND FND deleted the patch-1 branch June 24, 2019 10:54
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.

None yet

2 participants