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.

Sign up for GitHub

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

Jump to bottom

use-sync-external-store: Add exports field to package.json #25231

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open

use-sync-external-store: Add exports field to package.json #25231

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 11 additions & 0 deletions packages/use-sync-external-store/package.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,17 @@
"name": "use-sync-external-store",
"description": "Backwards compatible shim for React's useSyncExternalStore. Works with any React that supports hooks.",
"version": "1.2.0",
"exports": {
".": "./index.js",
"./with-selector": "./with-selector.js",
"./shim": {
"react-native": "./shim/index.native.js",
"default": "./shim/index.js"
},
"./shim/index.native": "./shim/index.native.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.

For compat with bundlers that don't support the react-native condition. Though I don't know if we need it. I just copied it from #24440

Copy link
Contributor

Choose a reason for hiding this comment

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

https://github.com/facebook/react/blob/main/packages/use-sync-external-store/src/__tests__/useSyncExternalStoreNative-test.js#L45
It seems that this entry has been used in several places. I'm not sure if this is necessary.

"./shim/with-selector": "./shim/with-selector.js",
"./package.json": "./package.json"
},
"repository": {
"type": "git",
"url": "https://github.com/facebook/react.git",
Expand Down
0 ...xternal-store/shim/with-selector/index.js → ...sync-external-store/shim/with-selector.js
View file
Open in desktop
File renamed without changes.
2 changes: 1 addition & 1 deletion packages/use-sync-external-store/src/useSyncExternalStoreWithSelector.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@

import * as React from 'react';
import is from 'shared/objectIs';
import {useSyncExternalStore} from 'use-sync-external-store/src/useSyncExternalStore';
import {useSyncExternalStore} from './useSyncExternalStore';
Copy link
Contributor

Choose a reason for hiding this comment

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

Perhaps a similar change should be done in other places to at least match the style?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

What other places?

Copy link
Contributor

Choose a reason for hiding this comment

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

Like here:

react/packages/use-sync-external-store/with-selector.js

Line 12 in 62a5371

export {useSyncExternalStoreWithSelector} from 'use-sync-external-store/src/useSyncExternalStoreWithSelector';

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

These are entrypoints so I'm not sure they should change. This PR is green and tested and I personally don't want make stylistic changes that invalidates all of that unless it's required to get this merged.

Copy link
Contributor

Choose a reason for hiding this comment

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

This should fail IMHO since there are no "./src/*" exports in package.json.

Copy link
Contributor

Choose a reason for hiding this comment

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

#23262

Copy link
Contributor

Choose a reason for hiding this comment

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

Since the src exports will get removed at the publish stage, it might be a good idea to add it back for local development.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

This should fail IMHO since there are no "./src/*" exports in package.json.

react/packages/use-sync-external-store/with-selector.js is fed to rollup and will not be shipped with the src/ path. I would suggest checking out the repository that was used for testing to see how files look after bundling.


// Intentionally not using named imports because Rollup uses dynamic dispatch
// for CommonJS interop.
Expand Down