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

fix: export package.json required by react-native and bundlers #449

Merged
merged 1 commit into from May 20, 2020

Conversation

ctavan
Copy link
Member

@ctavan ctavan commented May 12, 2020

It appears that react-native and some bundlers rely on being able to
introspect the package.json file of a npm module. After adding the
exports field to package.json, only the paths defined in that field
are accessible to Node.js. In order to support introspection of the
package.json file itself it has to be listed as an export as well (see:
ai/nanoevents#44 (comment)).

It is currently being discussed to change Node.js' behavior again, see:
nodejs/node#33460

Fixes #444

Copy link
Member

@broofa broofa left a comment

Choose a reason for hiding this comment

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

Something about this smells a bit off. I don't really consider package.json to be part of our public API. Thus, adding it to exports seems wrong. But I guess there's no alternative. 'Really wish RN had figured out a different approach that didn't require 100's or 1000's of projects to all make the same change. :-(

@ctavan
Copy link
Member Author

ctavan commented May 12, 2020

Something about this smells a bit off. I don't really consider package.json to be part of our public API. Thus, adding it to exports seems wrong. But I guess there's no alternative. 'Really wish RN had figured out a different approach that didn't require 100's or 1000's of projects to all make the same change. :-(

I think this is only half-true: We have been using the browser field of package.json for module bundlers for ages, I believe the spec for it (by @defunctzombie) even emerged out of this very use case that we had to provide different rng and hashing functions for the browser! So in reality parts of the package.json have been part of the public API of this package for a long time.

While I also believe that RN should degrade gracefully if it cannot read the package.json (apparently webpack and rollup didn't have any problems) I still think that the fact that now it's no longer only webpack and rollup but also RN who need to package.json to work well with this library is not a big game changer.

Speaking of: @TrySound do you have any clue why rollup and webpack worked just fine (continuing to respect the browser field from package.json) but RN requires this change?

@TrySound
Copy link
Member

TrySound commented May 12, 2020

rollup and webpack use only fs calls for resolving which is not harmed by exports isolation. Metro may use require or require.resolve for some reason but I'm not confident about it as never used metro before.

It appears that react-native and some bundlers rely on being able to
introspect the package.json file of a npm module. After adding the
`exports` field to package.json, only the paths defined in that field
are accessible to Node.js. In order to support introspection of the
package.json file itself it has to be listed as an export as well.

See also:
ai/nanoevents#44 (comment)

Fixes #444
@ctavan
Copy link
Member Author

ctavan commented May 20, 2020

Given the discussion in nodejs/node#33460 I believe the correct thing to do right now is to treat our package.json as part of the public API, at least this section should be considered as such since it defines how module bundlers and Node.js environments should import this module:

uuid/package.json

Lines 19 to 31 in a21e4d8

"sideEffects": false,
"main": "./dist/index.js",
"exports": {
"require": "./dist/index.js",
"import": "./wrapper.mjs"
},
"module": "./dist/esm-node/index.js",
"browser": {
"./dist/md5.js": "./dist/md5-browser.js",
"./dist/rng.js": "./dist/rng-browser.js",
"./dist/sha1.js": "./dist/sha1-browser.js",
"./dist/esm-node/index.js": "./dist/esm-browser/index.js"
},

If node changes behavior of package.json exports in the future we can still adjust. For the time being exporting package.json should lead to minimum breakage in other environments.

@ctavan ctavan merged commit be1c8fe into master May 20, 2020
@ctavan ctavan deleted the export-package-json branch May 20, 2020 07:08
nloehlein-godaddy added a commit to nloehlein-godaddy/image-blob-reduce that referenced this pull request Dec 17, 2020
nloehlein-godaddy added a commit to nloehlein-godaddy/image-blob-reduce that referenced this pull request Dec 17, 2020
Frans-L added a commit to Frans-L/p-queue that referenced this pull request Apr 27, 2022
Fixes the issue: sindresorhus#145

A similar issue and fix can be found from instance here: uuidjs/uuid#449
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.

package.json is not defined by "exports"
3 participants