feat(hydration): move hydration utilities to core, keeping old exports

[DamianOsipiuk]

Following: #2370

Move hydration utilities to the core.
Retain old exports for backward compatibility.
Export additional type that was missed before.

[vercel]

This pull request is being automatically deployed with Vercel (learn more).
To see the status of your deployment, click below or on the icon next to each commit.

🔍 Inspect: https://vercel.com/tannerlinsley/react-query/F8EYs2PR87N4j8d1x2fgcDyLPy7F
✅ Preview: https://react-query-git-fork-damianosipiuk-feat-mo-a846a9-tannerlinsley.vercel.app

[codesandbox-ci]

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 7670638:

Sandbox	Source
tannerlinsley/react-query: basic	Configuration
tannerlinsley/react-query: basic-typescript	Configuration

[TkDodo]

since hydration.ts is now part of core, shouldn't some of the imports change?

for example, it currently imports:

import type { QueryClient } from '../core/queryClient'

shouldn't that now just be:

import type { QueryClient } from './queryClient'

?

[TkDodo]

also, just questioning if this is really a feat and thus being released as a new minor release? There aren't really any features in here, I'd categorize it more as refactor ?

[DamianOsipiuk]

since hydration.ts is now part of core, shouldn't some of the imports change?

for example, it currently imports:

import type { QueryClient } from '../core/queryClient'

shouldn't that now just be:

import type { QueryClient } from './queryClient'

?

Good catch 🥇

also, just questioning if this is really a feat and thus being released as a new minor release? There aren't really any features in here, I'd categorize it more as refactor ?

Well, we are exporting new things from the core. Therefore I would say it's a feature.
Additionally refactor according to semantic-bot is not releasable by default. So to leverage this PR I would have to wait until some new fix or feature would be released.

[Ephem]

Since the hydration package is built as a standalone package, this re-export will result in these functions being included in both core and hydration. I don't think this is a big problem since it's meant to be deprecated, there are no singletons there that could mess things up if you import things from both places, and the payload is small anyway.

This double payload will affect everyone using the React bindings though since those are currently only included in /hydration. I wonder if this can be solved by changing this to export { dehydrate, hydrate } from 'react-query'? Not sure how re-exporting from a package configured as external works though, so would need to be tested.

[Ephem]

I'm afk until next Wednesday so only gave this a quick glance and didn't test it, but looks good to me if it's thoroughly tested. Probably good to check that the bundle sizes haven't changed unexpectedly much as well, though I don't expect they should have. 😄

[benbender]

Hey there, any eta when this will land? Would be really cool!
I played with react-query in svelte lately and stumbled upon the forced react-import when I try to use the hydration-module. The only other solution at hand would be to mirror hydration.ts into the package (as vue-query does). That, in turn, would increase the bundle-size of this thin layer on top of react-query significantly...

[TkDodo]

@benbender there is an open question from @Ephem that I would ideally like to be resolved before merging

[DamianOsipiuk]

@Ephem @TkDodo So I just tested this and it appears that es5 and es6 builds are unaffected. But UMD build just duplicates the code, growing in size.
Therefore is you need UMD to build then it's better to import from react-query on submodules than relative paths.
TIL

[TkDodo]

@Ephem can you take another look please? I'm really no expert in this area 😅

[Ephem]

@DamianOsipiuk Sorry for this taking so long, I've been pretty busy the last few weeks. I'll have a look within the next few days!

[Ephem]

This looks good to me! I verified the build is still working in a real project and that the output of the build was as expected (no duplicated code). I think Core grows around 3% or so, probably with some variance for which build and conditions (compression etc).

I noticed the React parts that were left in the hydration package were really tiny, even compared to the things that were already moved, so I took the liberty of pushing a commit to your branch that moved those into the react package (and re-tested after of course). This will let us entirely remove the separate hydration package in a future major.

So to summarize, this MR:

• Moves core hydration utils into the core package
• Moves React hydration-integration into the react package
• Keeps the exports around in the hydration package to avoid breaking change
• UMD and es-builds are both tested to work
• Unlocks hydration for libraries other than React! 🎉

The only downside is that the core and React packages are slightly larger (unless tree-shaking, which should remove this), and any future additions to hydration functionality will affect those instead of a separate package. Since any future changes are likely to be small and of the bug fix variety and the alternative to moving this into core is to maintain a whole bunch of extra packages I think this is the right way to go.

@TkDodo If this seems reasonable to you too, could you do a final check so I haven't missed anything obvious (I'm a bit tired right now..) and then merge this when the timing is good? 😄

@DamianOsipiuk Great work on this! 🌹 Sorry again to have kept you waiting.

[TkDodo]

The only downside is that the core and React packages are slightly larger (unless tree-shaking, which should remove this), and any future additions to hydration functionality will affect those instead of a separate package. Since any future changes are likely to be small and of the bug fix variety and the alternative to moving this into core is to maintain a whole bunch of extra packages I think this is the right way to go.

I'm personally not so concerned about bundle size, but other people very much are, especially with competition (see swr) is coming with smaller bundle sizes. Which absolute numbers are we talking here, and can we somehow verify that treeshaking (like a standard create-react-app or rollup build) removes the hydration code if not needed?

I'm also not so sure if future changes will be small. The persistor plugins use hydration under the hood, and we might need some additional functionality like allowlist / blocklist per query key or so that we might need to build into hydration itself....

[Ephem]

I built master and then rebased this branch to master locally and built that. Here's the comparison numbers for the UMD builds (only including gzip size since we usually look at that):

react-query-core.production.min.js.gz
Master: 9992 bytes
This PR: 10290 bytes
Diff: 298 bytes (~3%)

react-query.production.min.js.gz
Master: 11090 bytes
This PR: 11457 bytes
Diff: 367 bytes (~3%)

The alternative if we want to support hydration in other frameworks is to have one hydration-core.js-package, as well as a hydration-react.js, hydration-vue.js etc (since if for example the regular react package would import the hydration-core directly that would get included in the package). Doesn't seem worth it to me. 🤷‍♂️

we might need some additional functionality like allowlist / blocklist per query key or so

Slightly off topic perhaps, but would the shouldDehydrateQuery option in dehydrate cover this case?

can we somehow verify that treeshaking (like a standard create-react-app or rollup build) removes the hydration code if not needed?

I linked this branch to the basic React Query example, which uses react-scripts. Building the example resulted in this vendor chunk:

58.05 KB build/static/js/2.c235f902.chunk.js

I then added a useHydrate import and call to that in the example. The resulting vendor chunk:

58.28 KB (+237 B) build/static/js/2.d86e66df.chunk.js

So treeshaking does seem to work, at least to a big extent.

[TkDodo]

@Ephem I just build the basic example locally on master, and it results in:

58.04 KB  build/static/js/2.545ac94e.chunk.js

so there seems to be some difference to your numbers?

[Ephem]

My numbers was this PR, first without using any hydrate functionality (treeshaking condition), second using useHydrate (using some but not all of the hydration functionality).

I guess your number from master means there is a whopping 0.01kB that can't be treeshaken then. 😄

Edit: Note that this is after gzip which is more effective on larger files though, so details may vary of course, but definitely looks like treeshaking works at least with react-scripts.

[TkDodo]

yep, I know. I wanted to have a comparison between the basic example on master vs the basic example on this PR. I tried it out locally now with this PR, depending on

"react-query": "https://pkg.csb.dev/tannerlinsley/react-query/commit/76706385/react-query"

and it actually got me down to 57.88 KB (-164 B)

so, does this mean that this PR actually make the lib 164 bytes smaller? 🤷‍♂️

[Ephem]

yep, I know. I wanted to have a comparison between the basic example on master vs the basic example on this PR.

Oh, I see, sorry, I misunderstood. 😄

so, does this mean that this PR actually make the lib 164 bytes smaller?

This PR is missing a couple of commits from master which is probably why. When I tried this I first rebased locally and used that version. Didn't want to force push the rebase to someone elses branch though. 😄

[TkDodo]

right, it's based on 3.19.1, so I compared with that:

master, 3.19.1: 57.88 KB build/static/js/2.d16d2887.chunk.js (on disk, before gzip: 197.716 bytes )
this PR: 57.88 KB (-1 B) build/static/js/2.e3fcee0b.chunk.js (on disk, before gzip: 197.718 bytes)

so, two bytes more, but one byte less after gzip or so. Anyhow, I think this is negligible and since treeshaking seems to work, it will also not affect future additions. Good work 👍

[tannerlinsley]

🎉 This PR is included in version 3.22.0 🎉

The release is available on:

• npm package (@latest dist-tag)
• GitHub release

Your semantic-release bot 📦🚀

[TkDodo]

as a side-effect, on bundlephobia, we now show up as 12.2kb in v3.22.0 vs 11.8kb in v3.23.1 ☹️

[cloudmu]

This appears to cause error for createAsyncStoragePersistor which replies on hydration utilities.

[TkDodo]

Can you elaborate a bit please?

[cloudmu]

I am using createAsyncStoragePersistor, along with persistQueryClient , since v3.19.0
Following compiling error would occur after upgrading to version v3.22.0 and after:

Unable to resolve module ./hydration from node_modules\react-query\lib\hydration\index.js:

I was able to bypass the error by changing the following import in my local npm package: node_modules\react-query\lib\persistQueryClient-experimental\index.js:
from:
var _hydration = require("../hydration");
to:
var _hydration = require("../core/hydration");

My fix was a hack during my debugging. I haven't gone deeper but I believe the error is associated with changes made in v3.22.0

[TkDodo]

@Ephem @DamianOsipiuk would we need to change the imports for persistQueryClient as well? I'm looking at:

https://github.com/tannerlinsley/react-query/blob/9686426915ffb4401685ab27cf6656ac5ae72feb/src/persistQueryClient-experimental/index.ts#L3-L9

[DamianOsipiuk]

@TkDodo This could potentially solve the issue, but I do not understand why it stopped working, to be honest.
It seems to be working when importing directly from react-query/hydration

[TkDodo]

Can you PR it please, then we can try it out

[DamianOsipiuk]

@cloudmu Could you provide a codesandbox with a reproduction of the issue? Cause I just tried to do it locally and everything works for me.
Not sure if it requires specific packages versions or build config.

I will provide PR to change the imports anyway, but I want to understand what caused this issue on your setup.