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.

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

Ensure router.refresh() matches revalidatePath('/') behavior #46723

Merged
39 changes: 9 additions & 30 deletions packages/next/src/client/components/app-router.tsx
Expand Up @@ -38,7 +38,6 @@ import {
createInitialRouterState,
InitialRouterStateParameters,
} from './router-reducer/create-initial-router-state'
import { fetchServerResponse } from './router-reducer/fetch-server-response'
import { isBot } from '../../shared/lib/router/utils/is-bot'
import { addBasePath } from '../add-base-path'
import { AppRouterAnnouncer } from './app-router-announcer'
Expand All @@ -64,8 +63,6 @@ const HotReloader:
: (require('./react-dev-overlay/hot-reloader-client')
.default as typeof import('./react-dev-overlay/hot-reloader-client').default)

const prefetched = new Set<string>()

type AppRouterProps = Omit<
Omit<InitialRouterStateParameters, 'isServer' | 'location'>,
'initialParallelRoutes'
Expand Down Expand Up @@ -184,41 +181,23 @@ function Router({
back: () => window.history.back(),
forward: () => window.history.forward(),
prefetch: async (href) => {
const hrefWithBasePath = addBasePath(href)

// If prefetch has already been triggered, don't trigger it again.
if (
prefetched.has(hrefWithBasePath) ||
(typeof window !== 'undefined' && isBot(window.navigator.userAgent))
) {
if (isBot(window.navigator.userAgent)) {
return
}
prefetched.add(hrefWithBasePath)
const url = new URL(hrefWithBasePath, location.origin)
const url = new URL(addBasePath(href), location.origin)
// External urls can't be prefetched in the same way.
if (isExternalURL(url)) {
return
}
try {
const routerTree = window.history.state?.tree || initialTree
const serverResponse = await fetchServerResponse(

// @ts-ignore startTransition exists
React.startTransition(() => {
Copy link
Contributor

Choose a reason for hiding this comment

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

nit: can you extend it at the global level in a def file?

Copy link
Member Author

Choose a reason for hiding this comment

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

This one is covered in the latest @types/react, still has to be upgraded, not super important right now 👍

dispatch({
type: ACTION_PREFETCH,
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) {
Copy link
Contributor

Choose a reason for hiding this comment

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

do we not need to catch it anymore?

Copy link
Member Author

Choose a reason for hiding this comment

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

The promise is now awaited at navigation time, it's fine if it throws there

console.error('PREFETCH ERROR', err)
}
})
},
replace: (href, options = {}) => {
// @ts-ignore startTransition exists
Expand Down Expand Up @@ -251,7 +230,7 @@ function Router({
}

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

useEffect(() => {
// When mpaNavigation flag is set do a hard navigation to the new url.
Expand Down
@@ -0,0 +1,33 @@
import { CacheNode, CacheStates } from '../../../shared/lib/app-router-context'
import { FlightDataPath } from '../../../server/app-render/types'
import { fillLazyItemsTillLeafWithHead } from './fill-lazy-items-till-leaf-with-head'
import { fillCacheWithNewSubTreeData } from './fill-cache-with-new-subtree-data'
import { ReadonlyReducerState } from './router-reducer-types'

export function applyFlightData(
state: ReadonlyReducerState,
cache: CacheNode,
flightDataPath: FlightDataPath
): boolean {
// The one before last item is the router state tree patch
const [treePatch, subTreeData, head] = flightDataPath.slice(-3)

// Handles case where prefetch only returns the router tree patch without rendered components.
if (subTreeData === null) {
return false
}

if (flightDataPath.length === 3) {
cache.status = CacheStates.READY
cache.subTreeData = subTreeData
fillLazyItemsTillLeafWithHead(cache, state.cache, treePatch, head)
} else {
// Copy subTreeData for the root node of the cache.
cache.status = CacheStates.READY
cache.subTreeData = state.cache.subTreeData
// Create a copy of the existing cache with the subTreeData applied.
fillCacheWithNewSubTreeData(cache, state.cache, flightDataPath)
}

return true
}
@@ -1,5 +1,6 @@
export function createHrefFromUrl(
url: Pick<URL, 'pathname' | 'search' | 'hash'>
url: Pick<URL, 'pathname' | 'search' | 'hash'>,
includeHash: boolean = true
): string {
return url.pathname + url.search + url.hash
return url.pathname + url.search + (includeHash ? url.hash : '')
}
Expand Up @@ -40,8 +40,6 @@ describe('createInitialRouterState', () => {
initialHead: <title>Test</title>,
})

console.log(initialParallelRoutes)

const state2 = createInitialRouterState({
initialTree,
initialCanonicalUrl,
Expand Down
@@ -0,0 +1,54 @@
import {
Mutable,
ReadonlyReducerState,
ReducerState,
} from './router-reducer-types'

export function handleMutable(
state: ReadonlyReducerState,
mutable: Mutable
): ReducerState {
return {
// Set href.
canonicalUrl:
typeof mutable.canonicalUrl !== 'undefined'
? mutable.canonicalUrl === state.canonicalUrl
? state.canonicalUrl
: mutable.canonicalUrl
: state.canonicalUrl,
pushRef: {
pendingPush:
typeof mutable.pendingPush !== 'undefined'
? mutable.pendingPush
: state.pushRef.pendingPush,
mpaNavigation:
typeof mutable.mpaNavigation !== 'undefined'
Copy link
Contributor

Choose a reason for hiding this comment

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

nit: mutable?.mpaNavigation ?? state.pushRef.mpaNavigation

same for the rest of the file

Copy link
Member Author

Choose a reason for hiding this comment

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

Intentionally didn't use ?? as it's likely it'll be compiled out based on the current target.

? mutable.mpaNavigation
: state.pushRef.mpaNavigation,
},
// All navigation requires scroll and focus management to trigger.
focusAndScrollRef: {
apply:
typeof mutable.applyFocusAndScroll !== 'undefined'
? mutable.applyFocusAndScroll
: state.focusAndScrollRef.apply,
hashFragment:
// Empty hash should trigger default behavior of scrolling layout into view.
// #top is handled in layout-router.
mutable.hashFragment && mutable.hashFragment !== ''
? // Remove leading # and decode hash to make non-latin hashes work.
decodeURIComponent(mutable.hashFragment.slice(1))
: null,
},
// Apply cache.
cache: mutable.cache ? mutable.cache : state.cache,
prefetchCache: mutable.prefetchCache
? mutable.prefetchCache
: state.prefetchCache,
// Apply patched router state.
tree:
typeof mutable.patchedTree !== 'undefined'
? mutable.patchedTree
: state.tree,
}
}
Expand Up @@ -68,6 +68,7 @@ jest.mock('../fetch-server-response', () => {
},
}
})

import { FlightRouterState } from '../../../../server/app-render/types'
import {
CacheNode,
Expand All @@ -82,7 +83,7 @@ import {
} from '../router-reducer-types'
import { navigateReducer } from './navigate-reducer'
import { prefetchReducer } from './prefetch-reducer'
import { fetchServerResponse } from '../fetch-server-response'
import { createRecordFromThenable } from '../create-record-from-thenable'

const getInitialRouterStateTree = (): FlightRouterState => [
'',
Expand Down Expand Up @@ -987,12 +988,9 @@ describe('navigateReducer', () => {
])

const url = new URL('/linking/about', 'https://localhost')
const serverResponse = await fetchServerResponse(url, initialTree, true)
const prefetchAction: PrefetchAction = {
type: ACTION_PREFETCH,
url,
tree: initialTree,
serverResponse,
}

const state = createInitialRouterState({
Expand All @@ -1007,6 +1005,8 @@ describe('navigateReducer', () => {

await runPromiseThrowChain(() => prefetchReducer(state, prefetchAction))

await state.prefetchCache.get(url.pathname + url.search)?.data

const state2 = createInitialRouterState({
initialTree,
initialHead: null,
Expand All @@ -1018,6 +1018,7 @@ describe('navigateReducer', () => {
})

await runPromiseThrowChain(() => prefetchReducer(state2, prefetchAction))
await state2.prefetchCache.get(url.pathname + url.search)?.data

const action: NavigateAction = {
type: ACTION_NAVIGATE,
Expand All @@ -1041,42 +1042,43 @@ describe('navigateReducer', () => {
navigateReducer(state2, action)
)

const prom = Promise.resolve([
[
[
'children',
'linking',
'children',
'about',
[
'about',
{
children: ['', {}],
},
],
<h1>About Page!</h1>,
<React.Fragment>
<title>About page!</title>
</React.Fragment>,
],
],
undefined,
] as any)
const record = createRecordFromThenable(prom)
await prom

const expectedState: ReturnType<typeof navigateReducer> = {
prefetchCache: new Map([
[
'/linking/about',
{
canonicalUrlOverride: undefined,
flightData: [
[
'children',
'linking',
'children',
'about',
[
'about',
{
children: ['', {}],
},
],
<h1>About Page!</h1>,
<React.Fragment>
<title>About page!</title>
</React.Fragment>,
],
],
tree: [
data: record,
treeAtTimeOfPrefetch: [
'',
{
children: [
'linking',
{
children: [
'about',
{
children: ['', {}],
},
],
children: ['', {}],
},
],
},
Expand Down