RSC preloading mechanism #258
Conversation
phryneas
changed the title
size-limit report 📦
|
#194 Bundle Size — 1.03MiB (+2.47%).Warning Bundle contains 1 duplicate package – View duplicate packages Bundle metrics
|
| Current #194 |
Baseline #189 |
|
|---|---|---|
 Initial JS |
898.87KiB(+0.82%) |
891.55KiB |
| Initial CSS |
70B(+100%) |
0B |
 Cache Invalidation |
88.27% |
0.04% |
| Chunks |
29(+20.83%) |
24 |
| Assets |
53(+17.78%) |
45 |
| Modules |
528(+3.13%) |
512 |
| Duplicate Modules |
47(+56.67%) |
30 |
| Duplicate Code |
2.57%(+99.22%) |
1.29% |
 Packages |
26(-10.34%) |
29 |
 Duplicate Packages |
1 |
1 |
Bundle size by type 3 changes 3 regressions
| Current #194 |
Baseline #189 |
|
|---|---|---|
| JS |
1.02MiB (+2.28%) |
1MiB |
| Other |
8.06KiB (+34.41%) |
5.99KiB |
| CSS |
70B (+100%) |
0B |
Bundle analysis report Branch pr/rsc-preload Project dashboard
integration-test/package.json
Outdated
| @@ -9,7 +9,7 @@ | |||
| "*" | |||
| ], | |||
| "scripts": { | |||
| "trigger-rebuild": "find . -regextype posix-extended -regex '.*/node_modules/@apollo/(client-react-streaming|experimental-nextjs-app-support)' -printf 'rm -r %p\n' -exec rm -r {} +; glob \"../.yarn/cache/@apollo-*exec*\" \"$HOME/.yarn/berry/cache/@apollo-*exec*\" --cmd='rm -v' ; yarn" | |||
| "trigger:rebuild": "find . -regextype posix-extended -regex '.*/node_modules/@apollo/(client-react-streaming|experimental-nextjs-app-support)' -printf 'rm -r %p\n' -exec rm -r {} +; glob \"../.yarn/cache/@apollo-*exec*\" \"$HOME/.yarn/berry/cache/@apollo-*exec*\" --cmd='rm -v' ; yarn" | |||
There was a problem hiding this comment.
Scripts with a : can be called from any child project in a yarn monorepo. Big DX improvement for a single char :D
|
Quick thought: we need some warning if people add |
integration-test/nextjs/src/app/rsc/dynamic/PreloadQuery/queryRef-useReadQuery/ClientChild.tsx
Show resolved
Hide resolved
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
…rappedApolloClient.tsx Co-authored-by: Jerel Miller <[email protected]>
| const unwrapped = unwrapQueryRef<any>(queryRef); | ||
| useEffect(() => { | ||
| if (cacheKey) { | ||
| if (unwrapped.disposed) { | ||
| getSuspenseCache(client).add(cacheKey, unwrapped); | ||
| } | ||
| } | ||
| // Omitting the deps is intentional. This avoids stale closures and the | ||
| // conditional ensures we aren't running the logic on each render. | ||
| }); |
There was a problem hiding this comment.
@jerelmiller should I still do all this?
There was a problem hiding this comment.
I left it for useBackgroundQuery here to avoid issues where the queryRef gets disposed by the auto dispose timeout and the hook re-renders (which was a bug before that change in 3.9.10). I did update that check to use a setTimeout though since dispose is now again wrapped in a setTimeout.
I don't remember exactly where you use this hook, but if you're in the same situation as what I described above, then keeping this makes sense.
There was a problem hiding this comment.
This wraps around useReadQuery and useQueryRefHandlers, and it can also create a queryRef by reviving a transportedQueryRef. We can remove these effects once React deduplicates transported objects, but for now they're in here.
yarn.lock
Outdated
| @@ -49,14 +49,14 @@ __metadata: | |||
| version: 0.0.0-use.local | |||
| resolution: "@apollo/client-react-streaming@workspace:packages/client-react-streaming" | |||
| dependencies: | |||
| "@apollo/client": "npm:3.9.9" | |||
| "@apollo/client": "npm:3.10.1" | |||
There was a problem hiding this comment.
We might want to get apollographql/apollo-client#11821 out before we go live with the change in useTransportedQueryRef just to make sure its still compatible with the change in that PR.
There was a problem hiding this comment.
Should be included in the latest build from apollographql/apollo-client#11824
There was a problem hiding this comment.
Love it! Let's test this with apollographql/apollo-client#11821 just to make sure that doesn't break something here as well, but otherwise this is incredible work!
| useEffect(() => { | ||
| // We only want this to execute if the queryRef is a transported query. | ||
| if (!isTransported) return; | ||
| // We want to always keep this queryRef in the suspense cache in case another component has another instance of this transported queryRef. | ||
| // This effect could be removed after https://github.com/facebook/react/pull/28996 has been merged and we've updated deps to that version. | ||
| if (cacheKey) { | ||
| if (unwrapped.disposed) { | ||
| getSuspenseCache(client).add(cacheKey, unwrapped); | ||
| } | ||
| } | ||
| // Omitting the deps is intentional. This avoids stale closures and the | ||
| // conditional ensures we aren't running the logic on each render. | ||
| }); | ||
| // Soft-retaining because useQueryRefHandlers doesn't do it for us. | ||
| // This effect could be removed after https://github.com/facebook/react/pull/28996 has been merged and we've updated deps to that version. | ||
| useEffect(() => { | ||
| if (isTransported) { | ||
| return unwrapped.softRetain(); | ||
| } | ||
| }, [isTransported, unwrapped]); |
There was a problem hiding this comment.
@jerelmiller could you please take a final look at this bit before I merge this in?
Resolves #212 and also prepares for #248 a bit.
Previews:
https://apollo-git-32cae2-apollo-client-next-package-integration-tests.vercel.app//rsc/dynamic/PreloadQuery/useSuspenseQuery
https://apollo-git-32cae2-apollo-client-next-package-integration-tests.vercel.app//rsc/dynamic/PreloadQuery/queryRef-useReadQuery
https://apollo-git-32cae2-apollo-client-next-package-integration-tests.vercel.app//rsc/dynamic/PreloadQuery/queryRef-refTest
On each you can call
__APOLLO_CLIENT__[Symbol.for("apollo.suspenseCache")].queryRefsto investigate the queryRefs (if there are any)