Repository navigation
fix(router): warm prefetched route navigations - #2100
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub. 1 Skipped Deployment
|
|
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. |
commit: |
dai-shi
left a comment
dai-shi
left a comment
There was a problem hiding this comment.
Thanks for working on it!! Do you think you can add some tests to cover the fix? I would like to refactor based on it.
| export type INTERNAL_PrefetchedEntry = { | ||
| [KEY_RSC_PATH]: string; | ||
| [KEY_RESPONSE]: Promise<Response>; | ||
| [KEY_ELEMENTS]?: unknown; |
There was a problem hiding this comment.
Now, it feels like we should drop KEY_RESPONSE and make KEY_ELEEMENTS required. This will require separating initial server prefetch (bad naming) structure from client prefetch structure. I'll try that in a follow-up PR.
| }); | ||
| if (prefetchOnly) { | ||
| const elements = createElements(); | ||
| Promise.resolve(elements).catch(() => {}); |
There was a problem hiding this comment.
// Observe failed prefetches until a matching navigation consumes them.
Hmmm, I'm not sure how it helps.
There was a problem hiding this comment.
Is it to hide unhandled promise error? Makes sense, but the comment can be different?
There was a problem hiding this comment.
I think the error is handled if the prefetched entry is actually "used". So, this workaround is only for the case when the prefetched entry is never used. Is it correct?
There was a problem hiding this comment.
yeah, I think if the prefetched entry is consumed by navigation, unstable_fetchRsc reuses the decoded elements promise and the rejection is handled through that path. This catch is only for abandoned prefetches that are never consumed, where nothing else observes the promise. can update the comment.
| Object.entries(path2moduleIds).forEach(([path, pathIds]) => { | ||
| path2idxs[path] = pathIds.map((id) => ids.indexOf(id)); |
There was a problem hiding this comment.
Oh my... This is such a fundamental bug... Thanks for the catch.
|
Closing in favor of the focused replacement PR: #2102. It keeps only the router prefetch module id mapping fix. |
|
#2102 is merged, so reopened. |
dai-shi
left a comment
dai-shi
left a comment
There was a problem hiding this comment.
Now, I'm comfortable with this.
(I think Unstable_FetchRscStore should be revisited entirely in the near future.)
Fixes #2099.
Router prefetch now maps route module ids against the shared prefetch id table, which keeps production preload callbacks aligned with the chunks for the requested route.
RSC prefetch now starts decoding the prefetched response and stores the resulting elements with the prefetched entry. Matching navigations reuse those elements instead of creating them for the first time on click.