Add "exports" field to use-sync-external-store's package.json #24440
Conversation
|
doesn't this also need |
|
Oops, it appears I overlooked something. Updated. |
|
|
|
What's the motivation for this? |
|
This would help support React Native and be esm compliant. In tannerlinsley/react-query#3582 RN support was broken because we imported from |
|
@gaearon @acdlite I think this PR would fix our issues we have over in react-query with the uSES shim for react native. apollo-client is having the same issue, and they have opted to copy over the uSES code to fix it on their side. I would rather wait for an official fix on your end, which I think this PR provides :) |
| "exports": { | ||
| ".": "./index.js", | ||
| "./with-selector": "./with-selector.js", | ||
| "./shim": "./shim/index.js", |
There was a problem hiding this comment.
thoughts on
| "./shim": "./shim/index.js", | |
| "./shim": { | |
| "react-native": "./shim/index.native.js", | |
| "default": "./shim/index.js" | |
| }, |
?
That way all code can just be use-sync-external-store/shim.
note that this relies on the bundler actually supporting exports and supplying react-native condition (which I'm guessing metro at least doesn't do), but it seems more future-proof than these (seemingly) somewhat random exports
There was a problem hiding this comment.
facebook/metro#670
Metro doesn't respect exports field at all.
There was a problem hiding this comment.
Right, so no issue.
This stanza should still work for any modern bundler, and metro whenever it enters the future
|
I had to say this would be a breaking change, though. That's why I held this PR. I'm trying to find out a better approach but with no luck. |
Summary
How did you test this change?