-
Notifications
You must be signed in to change notification settings - Fork 95
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 to exports for React Native #74
Conversation
"import": "./dist/bundle/index.mjs", | ||
"require": "./dist/bundle/index.js" | ||
"./package.json": "./package.json", | ||
".": { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
are you sure on that one ? it seems awkward. Anyway I believe you should rather open an issue on Metro repo. This setup is pretty common, is not it?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pretty sure, as it fixes the issue. I believe Metro and other bundlers process package.json
files by importing them instead of reading the file directly. In such case, the package.json
's path must be specified in the exports
field if it exists. It seems that there has been a lot of discussion about this topic, but not much of a conclusion I think. Therefore, feel free to close this PR for the time being. I'll apply a patch on my end while the issue is not addressed on React Native's side.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ok 👍 . I was referring to the "."
Have you checked this path does not break nodejs cjs and es import and tools like rollup ?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No, I haven't. I've only checked in React Native.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
yeah it should all work correctly, this is just the unsugared version of the previous and the fallbacks for old stuff (webpack 4, old nodes) are also correct.
refs
https://nodejs.org/dist/latest-v14.x/docs/api/packages.html#packages_determining_module_system
https://nodejs.org/dist/latest-v14.x/docs/api/packages.html#packages_dual_commonjs_es_module_packages
https://webpack.js.org/guides/package-exports/#reference-syntax
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
With the new line for package.json we can't use the sugar syntax anymore, so it needs to be unpacked to the verbose version.
@lorenzofox3, can you release this? 🙏 |
This is required or Metro, React Native's bundler, complains and the build breaks.