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.

Sign up for GitHub

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

[metadata] Align prefetch head type with head #74161

Merged
merged 1 commit into from
Dec 20, 2024
Merged

[metadata] Align prefetch head type with head #74161

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
17 changes: 10 additions & 7 deletions packages/next/src/client/components/router-reducer/ppr-navigations.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -71,7 +71,7 @@ export function updateCacheNodeOnNavigation(
oldRouterState: FlightRouterState,
newRouterState: FlightRouterState,
prefetchData: CacheNodeSeedData | null,
prefetchHead: HeadData,
prefetchHead: HeadData | null,
isPrefetchHeadPartial: boolean
): Task | null {
// Diff the old and new trees to reuse the shared layouts.
Expand Down Expand Up @@ -286,7 +286,7 @@ export function updateCacheNodeOnNavigation(
function createCacheNodeOnNavigation(
routerState: FlightRouterState,
prefetchData: CacheNodeSeedData | null,
possiblyPartialPrefetchHead: HeadData,
possiblyPartialPrefetchHead: HeadData | null,
isPrefetchHeadPartial: boolean
): Task {
// Same traversal as updateCacheNodeNavigation, but we switch to this path
Expand Down Expand Up @@ -387,7 +387,10 @@ function createCacheNodeOnNavigation(
// `prefetchRsc` field.
rsc,
prefetchRsc: null,
head: isLeafSegment ? possiblyPartialPrefetchHead : [null, null],
head: (isLeafSegment ? possiblyPartialPrefetchHead : null) ?? [
null,
null,
],
Comment on lines +390 to +393
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm a bit confused: shouldn't we either standardize on null representing [null, null]? Or consistently always do [null, null]?

Suggested change
head: (isLeafSegment ? possiblyPartialPrefetchHead : null) ?? [
null,
null,
],
head: isLeafSegment ? possiblyPartialPrefetchHead : [null, null],

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

possiblyPartialPrefetchHead is still nullable. I tried all in [null, null] first but feels it's bit redudant as prefetched data doesn't have to always hold a [null, null] as default value. When you want to check if it's nullable and fallback to current head, [null, null] would be a bit confusing to represent null and to check the values.

This will be a temeporary state, we'll have the [null, null] for short time then metadata will go to stay with page component. Then head and prefetchedHead will be back to single ReactNode

prefetchHead: null,
loading,
parallelRoutes: cacheNodeChildren,
Expand Down Expand Up @@ -422,7 +425,7 @@ function patchRouterStateWithNewChildren(
function spawnPendingTask(
routerState: FlightRouterState,
prefetchData: CacheNodeSeedData | null,
prefetchHead: React.ReactNode | null,
prefetchHead: HeadData | null,
isPrefetchHeadPartial: boolean
): Task {
// Create a task that will later be fulfilled by data from the server.
Expand Down Expand Up @@ -645,7 +648,7 @@ function finishTaskUsingDynamicDataPayload(
function createPendingCacheNode(
routerState: FlightRouterState,
prefetchData: CacheNodeSeedData | null,
prefetchHead: React.ReactNode | null,
prefetchHead: HeadData | null,
isPrefetchHeadPartial: boolean
): ReadyCacheNode {
const routerStateChildren = routerState[1]
Expand Down Expand Up @@ -685,7 +688,7 @@ function createPendingCacheNode(
parallelRoutes: parallelRoutes,

prefetchRsc: maybePrefetchRsc !== undefined ? maybePrefetchRsc : null,
prefetchHead: isLeafSegment ? prefetchHead : null,
prefetchHead: isLeafSegment ? prefetchHead : [null, null],

// TODO: Technically, a loading boundary could contain dynamic data. We must
// have separate `loading` and `prefetchLoading` fields to handle this, like
Expand Down Expand Up @@ -940,7 +943,7 @@ export function updateCacheNodeOnPopstateRestoration(
rsc,
head: oldCacheNode.head,

prefetchHead: shouldUsePrefetch ? oldCacheNode.prefetchHead : null,
prefetchHead: shouldUsePrefetch ? oldCacheNode.prefetchHead : [null, null],
prefetchRsc: shouldUsePrefetch ? oldCacheNode.prefetchRsc : null,
loading: oldCacheNode.loading,

Expand Down
4 changes: 2 additions & 2 deletions packages/next/src/client/components/segment-cache/navigation.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -126,7 +126,7 @@ function navigateUsingPrefetchedRouteTree(
currentFlightRouterState: FlightRouterState,
prefetchFlightRouterState: FlightRouterState,
prefetchSeedData: CacheNodeSeedData | null,
prefetchHead: HeadData,
prefetchHead: HeadData | null,
isPrefetchHeadPartial: boolean,
canonicalUrl: string
): SuccessfulNavigationResult | NoOpNavigationResult {
Expand Down Expand Up @@ -302,7 +302,7 @@ async function navigateDynamicallyWithNoPrefetch(
// In our simulated prefetch payload, we pretend that there's no seed data
// nor a prefetch head.
const prefetchSeedData = null
const prefetchHead: [null, null] = [null, null]
const prefetchHead = null
const isPrefetchHeadPartial = true

const canonicalUrl = createCanonicalUrl(
Expand Down
4 changes: 2 additions & 2 deletions packages/next/src/shared/lib/app-router-context.shared-runtime.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -56,7 +56,7 @@ export type LazyCacheNode = {
*/
lazyData: Promise<FetchServerResponseResult> | null

prefetchHead: React.ReactNode
prefetchHead: HeadData | null

head: HeadData

Expand Down Expand Up @@ -99,7 +99,7 @@ export type ReadyCacheNode = {
* There should never be a lazy data request in this case.
*/
lazyData: null
prefetchHead: React.ReactNode
prefetchHead: HeadData | null

head: HeadData

Expand Down
Loading