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 package.json exports for use-sync-external-store #26230

Open
wants to merge 3 commits into
base: main
Choose a base branch
from

Conversation

devongovett
Copy link
Contributor

@devongovett devongovett commented Feb 24, 2023

Fixes #24590
Closes #25231

Summary

We recently added support for Node ESM to React Aria, allowing all of our packages to be loaded via import. However, one problem remains – import 'use-sync-external-store/shim' throws an error:

Error: Directory import '/Users/govett/dev/rsp-next/node_modules/use-sync-external-store/shim' is not supported resolving ES modules imported from /Users/govett/dev/rsp-next/node_modules/@react-stately/toast/dist/import.mjs

This occurs both when using Node and when using webpack (e.g. in Next.js), which treats dependencies resolved from .mjs files as more strictly.

This PR adds the "exports" field to package.json for use-sync-external-store, mapping the various extensionless and directory specifiers to the correct locations. Building the package itself as ESM is not necessary, we only need package exports for it to work in Node. This matches what react-dom and other packages with modules under subpaths do as well.

For backward compatibility I have included all files in the package in the exports map, since non-exported files will throw an error.

How did you test this change?

Copied the build output into a directory under node_modules, and ran import('use-sync-external-store/shim') in a Node REPL. Also copied this into a Next.js project and it worked.

@devongovett devongovett force-pushed the use-sync-external-store-exports branch from 29d0a5f to 70e246d Compare February 24, 2023 18:07
@react-sizebot
Copy link

react-sizebot commented Feb 24, 2023

Comparing: ca2cf31...5e6fa47

Critical size changes

Includes critical production bundles, as well as any change greater than 2%:

Name +/- Base Current +/- gzip Base gzip Current gzip
oss-stable/react-dom/cjs/react-dom.production.min.js = 154.46 kB 154.46 kB = 48.75 kB 48.75 kB
oss-experimental/react-dom/cjs/react-dom.production.min.js = 156.45 kB 156.45 kB = 49.41 kB 49.42 kB
facebook-www/ReactDOM-prod.classic.js = 530.70 kB 530.70 kB = 94.57 kB 94.57 kB
facebook-www/ReactDOM-prod.modern.js = 514.63 kB 514.63 kB = 92.12 kB 92.12 kB

Significant size changes

Includes any change greater than 0.2%:

(No significant changes)

Generated by 🚫 dangerJS against 5e6fa47

@eps1lon
Copy link
Collaborator

eps1lon commented Feb 25, 2023

Could you check the same steps as the test plan in #25231?

I guess my PR had to patch some files because it's still on old Rollup?

@devongovett
Copy link
Contributor Author

Ah missed that there was already a PR for this. Any reason it hasn't been merged?

@devongovett
Copy link
Contributor Author

@eps1lon Fixed a couple small things and ran your test repo against the latest commit and all passes now.

@eps1lon
Copy link
Collaborator

eps1lon commented Feb 27, 2023

For backward compatibility I have included all files in the package in the exports map, since non-exported files will throw an error.

So this change could be released as SemVer Minor while #25231 would be SemVer Major?

@devongovett
Copy link
Contributor Author

I think so, yes.

vicary added a commit to gqty-dev/gqty that referenced this pull request Mar 10, 2023
vicary added a commit to gqty-dev/gqty that referenced this pull request Mar 10, 2023
vicary added a commit to gqty-dev/gqty that referenced this pull request Mar 10, 2023
vicary added a commit to gqty-dev/gqty that referenced this pull request Mar 10, 2023
vicary added a commit to gqty-dev/gqty that referenced this pull request Mar 10, 2023
vicary added a commit to gqty-dev/gqty that referenced this pull request Mar 10, 2023
vicary added a commit to gqty-dev/gqty that referenced this pull request Mar 10, 2023
vicary added a commit to gqty-dev/gqty that referenced this pull request Apr 6, 2023
@markerikson
Copy link
Contributor

markerikson commented Jun 21, 2023

Hiya, folks. I'm working on upgrading React-Redux to have full ESM/CJS compat for our upcoming v9.0 major, and just ran into this lovely problem myself.

In our case, we'll need to be importing "use-sync-external-store/with-selector" and are dropping the shim usage, but same issue overall.

I just did a quick check and confirmed that hand-editing a locally installed node_modules/use-sync-external-store/package.json file with these "exports" changes appears to allow a simple .mjs test script to run okay:

import { useSelector } from "react-redux";
console.log(useSelector)

Could we get this merged and released?

vicary added a commit to gqty-dev/gqty that referenced this pull request Jul 4, 2023
vicary added a commit to gqty-dev/gqty that referenced this pull request Aug 26, 2023
@EskiMojo14
Copy link

any update on this?

Copy link

github-actions bot commented Apr 9, 2024

This pull request has been automatically marked as stale. If this pull request is still relevant, please leave any comment (for example, "bump"), and we'll keep it open. We are sorry that we haven't been able to prioritize reviewing it yet. Your contribution is very much appreciated.

@github-actions github-actions bot added the Resolution: Stale Automatically closed due to inactivity label Apr 9, 2024
@markerikson
Copy link
Contributor

This is definitely not stale!

@github-actions github-actions bot removed the Resolution: Stale Automatically closed due to inactivity label Apr 10, 2024
vicary added a commit to gqty-dev/gqty that referenced this pull request Apr 21, 2024
* feat(package/core): scoped query
* feat(package/subscriptions): drop the package for graphql-ws
* feat(packages/utils): drop unused package
* fix(packages/logger): scoped query shim
* feat: refactor resolvers for react
* chore: error message
* feat(packages/react): new core shim
* fix: facebook/react#26230
* fix: generated subscription client
* feat(packages/gqty): compat with queryFetcher and subsscriptionsClient
* chore(compat): queryFetcher and subscriptionsClient
* fix(package/react): infinite render loop
* feat(package/gqty): separate Cache instantiation from client
* chore(package/gqty): compat and optimized normalization
* fix(package/react): react hooks
* chore(deps): upgrade multidict
* feat(package/gqty): rename fetchPolicy to cachePolicy
* feat(package/gqty): add reload in cachePolicy
* fix(package/react): consistent API
* feat(package/react): Added onComplete for useMutation
* feat(packages/cli): interactive mode
* feat(examples): added the gnt example
* fix(package/react): missing fetches in useQuery
* fix(package/react): incorrect suspense in usePrepared
* feat(package/cli): rename introspectionOptions to introspections
* fix(deps): upgrade faulty multidict
* fix(ci): use esm imports
* fix(packages/gqty): compat, cache option should be optional.
* fix(ci): dlx errors
* feat(package/gqty): batching with microtask
* chore(package/cli): refactor generated client
* chore: remove legacy website
* feat(package/gqty): passthru return types of selectors
* chore(deps): upgrade testing-library for React 18
* fix(examples/vite): compatibility with v3
* chore(package/gqty): warn about empty selections
* fix(packages/gqty): pure peer dep of graphql-ws
* feat(package/gqty): edge compatible microtask
* chore(examples/github): remove stale example
* chore(deps): upgrade and dedupe typescript
* feat(package/gqty): added extensions option
* fix(packages/react): defaults to soft  to match the docs
* fix(packages/react): better simulate useTransactionQuery
* chore(docs): self-contained images in README.md
* fix(packages/react): flaky waitFor
* chore(deps): make type-fest optional
* feat(package/cli): error handling in generated client
* chore(package/cli): code-splitting for watch mode
* chore(deps): remove ws dependency
* fix(package/react): infinite render loop
* fix(package/react): early cache hydration
* fix(package/react): default  mode
* feat(core): even more stickier fetches
* chore: verbatimModuleSyntax
* chore: test grommet fetch loop
* fix(package/react): properly dispose cache subscribers
* chore(deps): update esbuild
* feat(cli): prompt for destination
* fix(packages/react): infinite fetch via stale proxy #1588
* feat(examples/gnt): add bundle analyzer
* fix(cli): compat with config.introspection
* fix(cli): fetch all schemas before merging
* fix(cli): install peer dep 'graphql'
* fix(cli): allow disable subscription via arg
* fix(packages/cli): typo on options definition
* feat(cli): add negative options
* fix(gqty): safe auto-selection on nullable interfaces
* feat(gqty): remove default return from resolve()
* fix(package/react): clear selections post-fetch in useQuery (#1594)
* chore(compat): normalize import.meta.url to increase compatibility.
* fix(package/gqty, package/react): error handling
* feat(chore): prettier v3
* fix(cli): revert prettier to v2, deps are not ready for it
* fix(package/react): reduce over-fetching between renders #1594
* feat(cli): enable suspense in the generated client
* chore(examples/gnt): simulate noverby's infinite fetch bug
* fix(package/react): reduce infinite fetch when normalization is disabled
* fix(package/react): prevent non-query types from sticky fetching
* chore(ci): test theguild's snapshot action
* fix(cli): missing default import in watch mode
* feat(cli): do not exit on fetch errors during watch mode
* chore(deps): remove unused packages
* chore(lint): add prettier-plugin-jsdoc
* feat(package/react): add extensions option to useMutation
* chore(prettier): remove jsdoc plugin to prevent noisy changes
* feat(package/react): increase compatibility with non-web environments
* fix(package/react): prevent infinite fetches
* feat(package/react): experimental greedy fetch
* feat(package/gqty): expose aliasLength option
* feat(package/gqty): supress empty warnings when onEmptyResolve is specified
* chore(exampels/vite-react): upgrade deprecated deps
* feat(package/logger): fix fetch timer
* chore(packages/gqty): user communication on fallbacks
* chore(ci): fix release errors
* chore(packages/gqty): prevent fetching stale inputs
* chore(package/gqty): add mutation test for the new core
* chore(examples/vite-react): clean up for suspense
* fix(package/gqty): properly batch with microtask
* feat(package/cli): allow force disabling of react
* feat(package/react): add initialLoadingState option
* chore(package/gqty): tidy up file structure
* fix(packages/react): prevent fetch loops from cofetching resolvers
* fix(packages/gqty): disable initialLoadingState upon first fetch
* fix(package/gqty): rerender after first fetch to refresh isLoading
* chore(examples/gnt): upgrade nhost client
* chore: fix type resolutions in esm
* chore(packages/cli): TypeScript v5 friendly type imports
* chore(packages/react): reduce redundant iterations
* chore(deps): upgrade pnpm
* chore(examples/gnt): preparation work for Supabase example API
* chore(package/react): remove experimental feature
@alexander-riss
Copy link

@acdlite any chance of getting this merged? it seems this is blocking ESM adoption in client libraries like Redux

@markerikson
Copy link
Contributor

Pinging @rickhanlonii - I think we briefly discussed this at ReactConf.

@phryneas
Copy link
Contributor

This would also allow Apollo Client to use the shim - currently we ship an inlined copy of the package, as the missing exports field caused too many problems to our users.

Copy link

This pull request has been automatically marked as stale. If this pull request is still relevant, please leave any comment (for example, "bump"), and we'll keep it open. We are sorry that we haven't been able to prioritize reviewing it yet. Your contribution is very much appreciated.

@github-actions github-actions bot added the Resolution: Stale Automatically closed due to inactivity label Sep 18, 2024
@markerikson
Copy link
Contributor

Very bump

@github-actions github-actions bot removed the Resolution: Stale Automatically closed due to inactivity label Sep 18, 2024
@pacocoursey
Copy link

pacocoursey commented Nov 2, 2024

Bump @sebmarkbage @shuding This is causing errors in my cmdk package

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Bug: Support ESM for the use-sync-external-store shim
9 participants