-
-
Notifications
You must be signed in to change notification settings - Fork 3k
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
ESM support #4092
ESM support #4092
Conversation
* fix: restore missing `exports` declarations * fix: restore package.json exports[C * fix: reexport types used by vue-query * fix: react-native uSES usage * fix: emit mjs for esm * fix: uSES build * fix: devtools exports to allow devtools in prod * fix: cjs and esm build bundled to lib dir * fix: sideEffect in react-query, better files paths * fix: generate declarations to lib * fix: lint and tests * fix: use the same ts build method for tests * fix: change force prod import
* - Fix UMD build getting overwritten - Updating "browser" field for @tanstack/react-query-devtools * Updating the "browser" field to be the same as "main"
# Conflicts: # examples/react/auto-refetching/package.json # examples/react/basic-graphql-request/package.json # examples/react/basic-typescript/package.json # examples/react/basic/package.json # examples/react/custom-hooks/package.json # examples/react/default-query-function/package.json # examples/react/focus-refetching/package.json # examples/react/load-more-infinite-scroll/package.json # examples/react/nextjs/package.json # examples/react/offline/package.json # examples/react/optimistic-updates-typescript/package.json # examples/react/optimistic-updates/package.json # examples/react/pagination/package.json # examples/react/playground/package.json # examples/react/prefetching/package.json # examples/react/react-native/package.json # examples/react/rick-morty/package.json # examples/react/simple/package.json # examples/react/star-wars/package.json # examples/react/suspense/package.json # package-lock.json # packages/query-broadcast-client-experimental/package.json # packages/query-broadcast-client-experimental/yarn.lock # packages/query-core/package.json # packages/react-query/package.json
* fix: remove browser entry, fix umd size * fix: bundle query-core with react-query for umd * fix: remove missed browser entry
because it doesn't exist anymore
# Conflicts: # examples/react/auto-refetching/package.json # examples/react/basic-graphql-request/package.json # examples/react/basic-typescript/package.json # examples/react/basic/package.json # examples/react/default-query-function/package.json # examples/react/load-more-infinite-scroll/package.json # examples/react/nextjs/package.json # examples/react/offline/package.json # examples/react/optimistic-updates-typescript/package.json # examples/react/pagination/package.json # examples/react/playground/package.json # examples/react/prefetching/package.json # examples/react/react-native/package.json # examples/react/rick-morty/package.json # examples/react/simple/package.json # examples/react/star-wars/package.json # examples/react/suspense/package.json # package-lock.json # packages/query-async-storage-persister/package.json # packages/query-broadcast-client-experimental/package.json # packages/query-core/package.json # packages/query-sync-storage-persister/package.json # packages/react-query-devtools/package.json # packages/react-query-persist-client/package.json # packages/react-query/package.json
I don't think we can have single quotes in commit message when passing them to --notes
* fix: umd build size * fix: devtools force production
# Conflicts: # examples/react/auto-refetching/package.json # examples/react/basic-graphql-request/package.json # examples/react/basic-typescript/package.json # examples/react/basic/package.json # examples/react/default-query-function/package.json # examples/react/load-more-infinite-scroll/package.json # examples/react/nextjs/package.json # examples/react/offline/package.json # examples/react/optimistic-updates-typescript/package.json # examples/react/pagination/package.json # examples/react/playground/package.json # examples/react/prefetching/package.json # examples/react/react-native/package.json # examples/react/rick-morty/package.json # examples/react/simple/package.json # examples/react/star-wars/package.json # examples/react/suspense/package.json # package-lock.json # packages/query-async-storage-persister/package.json # packages/query-broadcast-client-experimental/package.json # packages/query-core/package.json # packages/query-sync-storage-persister/package.json # packages/react-query-devtools/package.json # packages/react-query/package.json
no effect runs on the server, and there is no real advantage to useLayoutEffect on the client; somehow, this dynamic check creates problems with the production build of the devtools
nodenext needs 4.7
@sachinraja could you have a look a this PR please to see if we're missing something? All the reported issues and things we've identified seems to be resolved with this. It is also already available as beta release:
Thank you |
This pull request is automatically built and testable in CodeSandbox. To see build info of the built libraries, click here or the icon next to each commit SHA. Latest deployment of this branch, based on commit 712475b:
|
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.
Looks mostly good but had a few concerns
@@ -0,0 +1,3 @@ | |||
// Temporary workaround due to an issue with react-native uSES - https://github.com/TanStack/query/pull/3601 | |||
// @ts-ignore | |||
export { useSyncExternalStore } from 'use-sync-external-store/shim/index.native.js' |
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.
This doesn't work since the code is bundled. Since rollup only sees useSyncExternalStore.ts
, it just bundle it into build/lib/index.mjs
and build/lib/index.js
.
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.
How can we fix 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.
I guess we could create a proxy package with proper exports so we would not need to think about bundling.
But there are two more .native.
files in source. One for logger and one for batched updates.
The logger is not needed anymore as someone mentioned in some ticket. And even if so it can be configured with QueryClient.
Not sure about batched updates though.
"default": "./build/lib/index.js" | ||
}, | ||
"./production": { | ||
"types": "./build/lib/index.d.ts", |
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.
These exports won't work with TypeScript versions < 4.7 or bundlers that don't support exports
(older bundlers and React Native). Not sure what platforms/versions we're targeting but something to keep in mind.
If we do want to support them, we'll need to match the top level file structure of the package with the entry points in exports
.
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.
yep, we are aware of that. I've documented that here: Do you think it's good enough?
https://github.com/TanStack/query/blob/beta/docs/devtools.md#modern-bundlers
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.
Ah yeah that's good
@@ -0,0 +1,3 @@ | |||
// Temporary workaround due to an issue with react-native uSES - https://github.com/TanStack/query/pull/3601 | |||
// @ts-ignore | |||
export { useSyncExternalStore } from 'use-sync-external-store/shim/index.native.js' |
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.
Same issue as what I mentioned before for react-query-devtools
* fix: support react-native * chore: remove banner from build
Codecov ReportBase: 96.36% // Head: 96.82% // Increases project coverage by
Additional details and impacted files@@ Coverage Diff @@
## main #4092 +/- ##
==========================================
+ Coverage 96.36% 96.82% +0.46%
==========================================
Files 45 58 +13
Lines 2281 2676 +395
Branches 640 786 +146
==========================================
+ Hits 2198 2591 +393
- Misses 80 83 +3
+ Partials 3 2 -1 Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here. ☔ View full report at Codecov. |
fixes #3882