Skip to content

Commit

Permalink
Fix prefetch for new router (#41119)
Browse files Browse the repository at this point in the history
- Add a failing test for navigating between many levels of dynamic routes
- Create router tree during prefetch action so that it can be reused across multiple urls
- Ensure segmentPath is correct when rendering a subtree. Previously it would generate a segmentPath that starts at the level it renders at which causes the layout-router fetchServerResponse to inject `refetch` at the wrong level.
- Fixed a case where Segment was compared using `===` which is no longer valid as dynamic parameters are expressed as arrays. Used `matchSegment` helper instead.



## Bug

- [ ] Related issues linked using `fixes #number`
- [x] Integration tests added
- [ ] Errors have a helpful link attached, see `contributing.md`

## Feature

- [ ] Implements an existing feature request or RFC. Make sure the feature request has been accepted for implementation before opening a PR.
- [ ] Related issues linked using `fixes #number`
- [ ] Integration tests added
- [ ] Documentation added
- [ ] Telemetry added. In case of a feature if it's used or not.
- [ ] Errors have a helpful link attached, see `contributing.md`

## Documentation / Examples

- [ ] Make sure the linting passes by running `pnpm lint`
- [ ] The "examples guidelines" are followed from [our contributing doc](https://github.com/vercel/next.js/blob/canary/contributing/examples/adding-examples.md)
  • Loading branch information
timneutkens committed Oct 5, 2022
1 parent 5f2e44d commit 81b8185
Show file tree
Hide file tree
Showing 14 changed files with 307 additions and 82 deletions.
58 changes: 30 additions & 28 deletions packages/next/client/components/app-router.client.tsx
Expand Up @@ -15,7 +15,7 @@ import type {
import type { FlightRouterState, FlightData } from '../../server/app-render'
import {
ACTION_NAVIGATE,
// ACTION_PREFETCH,
ACTION_PREFETCH,
ACTION_RELOAD,
ACTION_RESTORE,
ACTION_SERVER_PATCH,
Expand Down Expand Up @@ -97,7 +97,7 @@ function ErrorOverlay({ children }: PropsWithChildren<{}>): ReactElement {
let initialParallelRoutes: CacheNode['parallelRoutes'] =
typeof window === 'undefined' ? null! : new Map()

// const prefetched = new Set<string>()
const prefetched = new Set<string>()

/**
* The global router that wraps the application components.
Expand Down Expand Up @@ -208,32 +208,34 @@ export default function AppRouter({

const routerInstance: AppRouterInstance = {
// TODO-APP: implement prefetching of flight
prefetch: async (_href) => {
prefetch: async (href) => {
// If prefetch has already been triggered, don't trigger it again.
// if (prefetched.has(href)) {
// return
// }
// prefetched.add(href)
// const url = new URL(href, location.origin)
// try {
// // TODO-APP: handle case where history.state is not the new router history entry
// const serverResponse = await fetchServerResponse(
// url,
// // initialTree is used when history.state.tree is missing because the history state is set in `useEffect` below, it being missing means this is the hydration case.
// window.history.state?.tree || initialTree,
// true
// )
// // @ts-ignore startTransition exists
// React.startTransition(() => {
// dispatch({
// type: ACTION_PREFETCH,
// url,
// serverResponse,
// })
// })
// } catch (err) {
// console.error('PREFETCH ERROR', err)
// }
if (prefetched.has(href)) {
return
}
prefetched.add(href)
const url = new URL(href, location.origin)
try {
const routerTree = window.history.state?.tree || initialTree
// TODO-APP: handle case where history.state is not the new router history entry
const serverResponse = await fetchServerResponse(
url,
// initialTree is used when history.state.tree is missing because the history state is set in `useEffect` below, it being missing means this is the hydration case.
routerTree,
true
)
// @ts-ignore startTransition exists
React.startTransition(() => {
dispatch({
type: ACTION_PREFETCH,
url,
tree: routerTree,
serverResponse,
})
})
} catch (err) {
console.error('PREFETCH ERROR', err)
}
},
replace: (href, options = {}) => {
// @ts-ignore startTransition exists
Expand Down Expand Up @@ -266,7 +268,7 @@ export default function AppRouter({
}

return routerInstance
}, [dispatch /*, initialTree*/])
}, [dispatch, initialTree])

useEffect(() => {
// When mpaNavigation flag is set do a hard navigation to the new url.
Expand Down
24 changes: 2 additions & 22 deletions packages/next/client/components/layout-router.client.tsx
Expand Up @@ -29,27 +29,7 @@ import {
import { fetchServerResponse } from './app-router.client'
import { createInfinitePromise } from './infinite-promise'

// import { matchSegment } from './match-segments'

/**
* Check if every segment in array a and b matches
*/
// function equalSegmentPaths(a: Segment[], b: Segment[]) {
// // Comparing length is a fast path.
// return a.length === b.length && a.every((val, i) => matchSegment(val, b[i]))
// }

/**
* Check if flightDataPath matches layoutSegmentPath
*/
// function segmentPathMatches(
// flightDataPath: FlightDataPath,
// layoutSegmentPath: FlightSegmentPath
// ): boolean {
// // The last three items are the current segment, tree, and subTreeData
// const pathToLayout = flightDataPath.slice(0, -3)
// return equalSegmentPaths(layoutSegmentPath, pathToLayout)
// }
import { matchSegment } from './match-segments'

/**
* Add refetch marker to router state at the point of the current layout segment.
Expand All @@ -63,7 +43,7 @@ function walkAddRefetch(
const [segment, parallelRouteKey] = segmentPathToWalk
const isLast = segmentPathToWalk.length === 2

if (treeToRecreate[0] === segment) {
if (matchSegment(treeToRecreate[0], segment)) {
if (treeToRecreate[1].hasOwnProperty(parallelRouteKey)) {
if (isLast) {
const subTree = walkAddRefetch(
Expand Down
37 changes: 24 additions & 13 deletions packages/next/client/components/reducer.ts
Expand Up @@ -590,6 +590,7 @@ interface ServerPatchAction {
interface PrefetchAction {
type: typeof ACTION_PREFETCH
url: URL
tree: FlightRouterState
serverResponse: Awaited<ReturnType<typeof fetchServerResponse>>
}

Expand Down Expand Up @@ -627,7 +628,7 @@ type AppRouterState = {
string,
{
flightSegmentPath: FlightSegmentPath
treePatch: FlightRouterState
tree: FlightRouterState
canonicalUrlOverride: URL | undefined
}
>
Expand Down Expand Up @@ -691,16 +692,11 @@ function clientReducer(
const prefetchValues = state.prefetchCache.get(href)
if (prefetchValues) {
// The one before last item is the router state tree patch
const { flightSegmentPath, treePatch, canonicalUrlOverride } =
prefetchValues

// Create new tree based on the flightSegmentPath and router state patch
const newTree = applyRouterStatePatchToTree(
// TODO-APP: remove ''
['', ...flightSegmentPath],
state.tree,
treePatch
)
const {
flightSegmentPath,
tree: newTree,
canonicalUrlOverride,
} = prefetchValues

if (newTree !== null) {
mutable.previousTree = state.tree
Expand Down Expand Up @@ -1130,11 +1126,26 @@ function clientReducer(
fillCacheWithPrefetchedSubTreeData(state.cache, flightDataPath)
}

const flightSegmentPath = flightDataPath.slice(0, -2)

const newTree = applyRouterStatePatchToTree(
// TODO-APP: remove ''
['', ...flightSegmentPath],
state.tree,
treePatch
)

// Patch did not apply correctly
if (newTree === null) {
return state
}

// Create new tree based on the flightSegmentPath and router state patch
state.prefetchCache.set(href, {
// Path without the last segment, router state, and the subTreeData
flightSegmentPath: flightDataPath.slice(0, -2),
treePatch,
flightSegmentPath,
// Create new tree based on the flightSegmentPath and router state patch
tree: newTree,
canonicalUrlOverride,
})

Expand Down
59 changes: 41 additions & 18 deletions packages/next/server/app-render.tsx
Expand Up @@ -1126,12 +1126,21 @@ export async function renderToHTMLOrFlight(
* Use router state to decide at what common layout to render the page.
* This can either be the common layout between two pages or a specific place to start rendering from using the "refetch" marker in the tree.
*/
const walkTreeWithFlightRouterState = async (
loaderTreeToFilter: LoaderTree,
parentParams: { [key: string]: string | string[] },
flightRouterState?: FlightRouterState,
const walkTreeWithFlightRouterState = async ({
createSegmentPath,
loaderTreeToFilter,
parentParams,
isFirst,
flightRouterState,
parentRendered,
}: {
createSegmentPath: CreateSegmentPath
loaderTreeToFilter: LoaderTree
parentParams: { [key: string]: string | string[] }
isFirst: boolean
flightRouterState?: FlightRouterState
parentRendered?: boolean
): Promise<FlightDataPath> => {
}): Promise<FlightDataPath> => {
const [segment, parallelRoutes] = loaderTreeToFilter
const parallelRoutesKeys = Object.keys(parallelRoutes)

Expand Down Expand Up @@ -1176,10 +1185,12 @@ export async function renderToHTMLOrFlight(
await createComponentTree(
// This ensures flightRouterPath is valid and filters down the tree
{
createSegmentPath: (child) => child,
createSegmentPath: (child) => {
return createSegmentPath(child)
},
loaderTree: loaderTreeToFilter,
parentParams: currentParams,
firstItem: true,
firstItem: isFirst,
}
)
).Component
Expand All @@ -1190,12 +1201,22 @@ export async function renderToHTMLOrFlight(
// Walk through all parallel routes.
for (const parallelRouteKey of parallelRoutesKeys) {
const parallelRoute = parallelRoutes[parallelRouteKey]
const path = await walkTreeWithFlightRouterState(
parallelRoute,
currentParams,
flightRouterState && flightRouterState[1][parallelRouteKey],
parentRendered || renderComponentsOnThisLevel
)

const currentSegmentPath: FlightSegmentPath = isFirst
? [parallelRouteKey]
: [actualSegment, parallelRouteKey]

const path = await walkTreeWithFlightRouterState({
createSegmentPath: (child) => {
return createSegmentPath([...currentSegmentPath, ...child])
},
loaderTreeToFilter: parallelRoute,
parentParams: currentParams,
flightRouterState:
flightRouterState && flightRouterState[1][parallelRouteKey],
parentRendered: parentRendered || renderComponentsOnThisLevel,
isFirst: false,
})

if (typeof path[path.length - 1] !== 'string') {
return [actualSegment, parallelRouteKey, ...path]
Expand All @@ -1210,11 +1231,13 @@ export async function renderToHTMLOrFlight(
const flightData: FlightData = [
// TODO-APP: change walk to output without ''
(
await walkTreeWithFlightRouterState(
loaderTree,
{},
providedFlightRouterState
)
await walkTreeWithFlightRouterState({
createSegmentPath: (child) => child,
loaderTreeToFilter: loaderTree,
parentParams: {},
flightRouterState: providedFlightRouterState,
isFirst: true,
})
).slice(1),
]

Expand Down
28 changes: 28 additions & 0 deletions test/e2e/app-dir/app/app/nested-navigation/CategoryNav.js
@@ -0,0 +1,28 @@
'client'

import { TabNavItem } from './TabNavItem'
import { useSelectedLayoutSegment } from 'next/dist/client/components/hooks-client'

const CategoryNav = ({ categories }) => {
const selectedLayoutSegment = useSelectedLayoutSegment()

return (
<div style={{ display: 'flex' }}>
<TabNavItem href="/nested-navigation" isActive={!selectedLayoutSegment}>
Home
</TabNavItem>

{categories.map((item) => (
<TabNavItem
key={item.slug}
href={`/nested-navigation/${item.slug}`}
isActive={item.slug === selectedLayoutSegment}
>
{item.name}
</TabNavItem>
))}
</div>
)
}

export default CategoryNav
9 changes: 9 additions & 0 deletions test/e2e/app-dir/app/app/nested-navigation/TabNavItem.js
@@ -0,0 +1,9 @@
import Link from 'next/link'

export const TabNavItem = ({ children, href }) => {
return (
<Link href={href}>
<a style={{ margin: '10px', display: 'block' }}>{children}</a>
</Link>
)
}
@@ -0,0 +1,31 @@
'client'

import { TabNavItem } from '../TabNavItem'
import { useSelectedLayoutSegment } from 'next/dist/client/components/hooks-client'

const SubCategoryNav = ({ category }) => {
const selectedLayoutSegment = useSelectedLayoutSegment()

return (
<div style={{ display: 'flex' }}>
<TabNavItem
href={`/nested-navigation/${category.slug}`}
isActive={!selectedLayoutSegment}
>
All
</TabNavItem>

{category.items.map((item) => (
<TabNavItem
key={item.slug}
href={`/nested-navigation/${category.slug}/${item.slug}`}
isActive={item.slug === selectedLayoutSegment}
>
{item.name}
</TabNavItem>
))}
</div>
)
}

export default SubCategoryNav
@@ -0,0 +1,11 @@
import { experimental_use as use } from 'react'
import { fetchSubCategory } from '../../getCategories'

export default function Page({ params }) {
const category = use(
fetchSubCategory(params.categorySlug, params.subCategorySlug)
)
if (!category) return null

return <h1 id={category.name.toLowerCase()}>{category.name}</h1>
}
@@ -0,0 +1,14 @@
import { experimental_use as use } from 'react'
import { fetchCategoryBySlug } from '../getCategories'
import SubCategoryNav from './SubCategoryNav'

export default function Layout({ children, params }) {
const category = use(fetchCategoryBySlug(params.categorySlug))
if (!category) return null
return (
<>
<SubCategoryNav category={category} />
{children}
</>
)
}
@@ -0,0 +1,9 @@
import { experimental_use as use } from 'react'
import { fetchCategoryBySlug } from '../getCategories'

export default function Page({ params }) {
const category = use(fetchCategoryBySlug(params.categorySlug))
if (!category) return null

return <h1 id={`all-${category.name.toLowerCase()}`}>All {category.name}</h1>
}

0 comments on commit 81b8185

Please sign in to comment.