Skip to content

Commit

Permalink
Check root layout change on client (#41475)
Browse files Browse the repository at this point in the history
Moves the logic that checks if there's a new root layout to the client.
Adds test for static and dynamic catchall.

Related: #41457

## Bug

- [ ] Related issues linked using `fixes #number`
- [ ] 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)

Co-authored-by: Tim Neutkens <tim@timneutkens.nl>
Co-authored-by: JJ Kasper <jj@jjsweb.site>
  • Loading branch information
3 people committed Oct 20, 2022
1 parent 765791a commit a5d5315
Show file tree
Hide file tree
Showing 11 changed files with 286 additions and 43 deletions.
146 changes: 136 additions & 10 deletions packages/next/client/components/reducer.ts
Expand Up @@ -406,7 +406,7 @@ function applyRouterStatePatchToTree(
flightRouterState: FlightRouterState,
treePatch: FlightRouterState
): FlightRouterState | null {
const [segment, parallelRoutes /* , url */] = flightRouterState
const [segment, parallelRoutes, , , isRootLayout] = flightRouterState

// Root refresh
if (flightSegmentPath.length === 1) {
Expand Down Expand Up @@ -451,6 +451,11 @@ function applyRouterStatePatchToTree(
},
]

// Current segment is the root layout
if (isRootLayout) {
tree[4] = true
}

// TODO-APP: Revisit
// if (url) {
// tree[2] = url
Expand Down Expand Up @@ -494,6 +499,47 @@ function shouldHardNavigate(
)
}

function isNavigatingToNewRootLayout(
currentTree: FlightRouterState,
nextTree: FlightRouterState
): boolean {
// Compare segments
const currentTreeSegment = currentTree[0]
const nextTreeSegment = nextTree[0]
// If any segment is different before we find the root layout, the root layout has changed.
// E.g. /same/(group1)/layout.js -> /same/(group2)/layout.js
// First segment is 'same' for both, keep looking. (group1) changed to (group2) before the root layout was found, it must have changed.
if (Array.isArray(currentTreeSegment) && Array.isArray(nextTreeSegment)) {
// Compare dynamic param name and type but ignore the value, different values would not affect the current root layout
// /[name] - /slug1 and /slug2, both values (slug1 & slug2) still has the same layout /[name]/layout.js
if (
currentTreeSegment[0] !== nextTreeSegment[0] ||
currentTreeSegment[2] !== nextTreeSegment[2]
) {
return true
}
} else if (currentTreeSegment !== nextTreeSegment) {
return true
}

// Current tree root layout found
if (currentTree[4]) {
// If the next tree doesn't have the root layout flag, it must have changed.
return !nextTree[4]
}
// Current tree didn't have its root layout here, must have changed.
if (nextTree[4]) {
return true
}
// We can't assume it's `parallelRoutes.children` here in case the root layout is `app/@something/layout.js`
// But it's not possible to be more than one parallelRoutes before the root layout is found
// TODO-APP: change to traverse all parallel routes
const currentTreeChild = Object.values(currentTree[1])[0]
const nextTreeChild = Object.values(nextTree[1])[0]
if (!currentTreeChild || !nextTreeChild) return true
return isNavigatingToNewRootLayout(currentTreeChild, nextTreeChild)
}

export type FocusAndScrollRef = {
/**
* If focus and scroll should be set in the layout-router's useEffect()
Expand All @@ -518,6 +564,7 @@ interface RefreshAction {
mutable: {
previousTree?: FlightRouterState
patchedTree?: FlightRouterState
mpaNavigation?: boolean
canonicalUrlOverride?: string
}
}
Expand Down Expand Up @@ -553,6 +600,7 @@ interface NavigateAction {
forceOptimisticNavigation: boolean
cache: CacheNode
mutable: {
mpaNavigation?: boolean
previousTree?: FlightRouterState
patchedTree?: FlightRouterState
canonicalUrlOverride?: string
Expand Down Expand Up @@ -587,6 +635,7 @@ interface ServerPatchAction {
cache: CacheNode
mutable: {
patchedTree?: FlightRouterState
mpaNavigation?: boolean
canonicalUrlOverride?: string
}
}
Expand Down Expand Up @@ -671,18 +720,42 @@ function clientReducer(
const href = createHrefFromUrl(url)
const pendingPush = navigateType === 'push'

// Handle concurrent rendering / strict mode case where the cache and tree were already populated.
if (
mutable.patchedTree &&
const isForCurrentTree =
JSON.stringify(mutable.previousTree) === JSON.stringify(state.tree)
) {

if (mutable.mpaNavigation && isForCurrentTree) {
return {
// Set href.
canonicalUrl: mutable.canonicalUrlOverride
? mutable.canonicalUrlOverride
: href,
// TODO-APP: verify mpaNavigation not being set is correct here.
pushRef: {
pendingPush,
mpaNavigation: mutable.mpaNavigation,
},
// All navigation requires scroll and focus management to trigger.
focusAndScrollRef: { apply: false },
// Apply cache.
cache: state.cache,
prefetchCache: state.prefetchCache,
// Apply patched router state.
tree: state.tree,
}
}

// Handle concurrent rendering / strict mode case where the cache and tree were already populated.
if (mutable.patchedTree && isForCurrentTree) {
return {
// Set href.
canonicalUrl: mutable.canonicalUrlOverride
? mutable.canonicalUrlOverride
: href,
// TODO-APP: verify mpaNavigation not being set is correct here.
pushRef: { pendingPush, mpaNavigation: false },
pushRef: {
pendingPush,
mpaNavigation: false,
},
// All navigation requires scroll and focus management to trigger.
focusAndScrollRef: { apply: true },
// Apply cache.
Expand All @@ -705,6 +778,10 @@ function clientReducer(
if (newTree !== null) {
mutable.previousTree = state.tree
mutable.patchedTree = newTree
mutable.mpaNavigation = isNavigatingToNewRootLayout(
state.tree,
newTree
)

const hardNavigate =
// TODO-APP: Revisit if this is correct.
Expand Down Expand Up @@ -793,6 +870,10 @@ function clientReducer(
if (!res?.bailOptimistic) {
mutable.previousTree = state.tree
mutable.patchedTree = optimisticTree
mutable.mpaNavigation = isNavigatingToNewRootLayout(
state.tree,
optimisticTree
)
return {
// Set href.
canonicalUrl: href,
Expand Down Expand Up @@ -867,6 +948,7 @@ function clientReducer(
}
mutable.previousTree = state.tree
mutable.patchedTree = newTree
mutable.mpaNavigation = isNavigatingToNewRootLayout(state.tree, newTree)

if (flightDataPath.length === 2) {
cache.subTreeData = subTreeData
Expand Down Expand Up @@ -906,6 +988,27 @@ function clientReducer(
return state
}

if (mutable.mpaNavigation) {
return {
// Set href.
canonicalUrl: mutable.canonicalUrlOverride
? mutable.canonicalUrlOverride
: state.canonicalUrl,
// TODO-APP: verify mpaNavigation not being set is correct here.
pushRef: {
pendingPush: true,
mpaNavigation: mutable.mpaNavigation,
},
// All navigation requires scroll and focus management to trigger.
focusAndScrollRef: { apply: false },
// Apply cache.
cache: state.cache,
prefetchCache: state.prefetchCache,
// Apply patched router state.
tree: state.tree,
}
}

// Handle concurrent rendering / strict mode case where the cache and tree were already populated.
if (mutable.patchedTree) {
return {
Expand Down Expand Up @@ -968,6 +1071,7 @@ function clientReducer(
}

mutable.patchedTree = newTree
mutable.mpaNavigation = isNavigatingToNewRootLayout(state.tree, newTree)

// Root refresh
if (flightDataPath.length === 2) {
Expand Down Expand Up @@ -1013,11 +1117,32 @@ function clientReducer(
const { cache, mutable } = action
const href = state.canonicalUrl

// Handle concurrent rendering / strict mode case where the cache and tree were already populated.
if (
mutable.patchedTree &&
const isForCurrentTree =
JSON.stringify(mutable.previousTree) === JSON.stringify(state.tree)
) {

if (mutable.mpaNavigation && isForCurrentTree) {
return {
// Set href.
canonicalUrl: mutable.canonicalUrlOverride
? mutable.canonicalUrlOverride
: state.canonicalUrl,
// TODO-APP: verify mpaNavigation not being set is correct here.
pushRef: {
pendingPush: true,
mpaNavigation: mutable.mpaNavigation,
},
// All navigation requires scroll and focus management to trigger.
focusAndScrollRef: { apply: false },
// Apply cache.
cache: state.cache,
prefetchCache: state.prefetchCache,
// Apply patched router state.
tree: state.tree,
}
}

// Handle concurrent rendering / strict mode case where the cache and tree were already populated.
if (mutable.patchedTree && isForCurrentTree) {
return {
// Set href.
canonicalUrl: mutable.canonicalUrlOverride
Expand Down Expand Up @@ -1095,6 +1220,7 @@ function clientReducer(

mutable.previousTree = state.tree
mutable.patchedTree = newTree
mutable.mpaNavigation = isNavigatingToNewRootLayout(state.tree, newTree)

// Set subTreeData for the root node of the cache.
cache.subTreeData = subTreeData
Expand Down
35 changes: 20 additions & 15 deletions packages/next/server/app-render.tsx
Expand Up @@ -443,7 +443,8 @@ export type FlightRouterState = [
segment: Segment,
parallelRoutes: { [parallelRouterKey: string]: FlightRouterState },
url?: string,
refresh?: 'refetch'
refresh?: 'refetch',
isRootLayout?: boolean
]

/**
Expand Down Expand Up @@ -849,29 +850,33 @@ export async function renderToHTMLOrFlight(
}
}

const createFlightRouterStateFromLoaderTree = ([
segment,
parallelRoutes,
]: LoaderTree): FlightRouterState => {
const createFlightRouterStateFromLoaderTree = (
[segment, parallelRoutes, { layout }]: LoaderTree,
rootLayoutIncluded = false
): FlightRouterState => {
const dynamicParam = getDynamicParamFromSegment(segment)

const segmentTree: FlightRouterState = [
dynamicParam ? dynamicParam.treeSegment : segment,
{},
]

if (parallelRoutes) {
segmentTree[1] = Object.keys(parallelRoutes).reduce(
(existingValue, currentValue) => {
existingValue[currentValue] = createFlightRouterStateFromLoaderTree(
parallelRoutes[currentValue]
)
return existingValue
},
{} as FlightRouterState[1]
)
if (!rootLayoutIncluded && typeof layout !== 'undefined') {
rootLayoutIncluded = true
segmentTree[4] = true
}

segmentTree[1] = Object.keys(parallelRoutes).reduce(
(existingValue, currentValue) => {
existingValue[currentValue] = createFlightRouterStateFromLoaderTree(
parallelRoutes[currentValue],
rootLayoutIncluded
)
return existingValue
},
{} as FlightRouterState[1]
)

return segmentTree
}

Expand Down
58 changes: 52 additions & 6 deletions test/e2e/app-dir/root-layout.test.ts
Expand Up @@ -4,7 +4,7 @@ import { NextInstance } from 'test/lib/next-modes/base'
import webdriver from 'next-webdriver'
import { getRedboxSource, hasRedbox } from 'next-test-utils'

describe.skip('app-dir root layout', () => {
describe('app-dir root layout', () => {
const isDev = (global as any).isNextDev

if ((global as any).isNextDeploy) {
Expand Down Expand Up @@ -151,18 +151,18 @@ describe.skip('app-dir root layout', () => {
})

it('should work with dynamic routes', async () => {
const browser = await webdriver(next.url, '/dynamic/first/route')
const browser = await webdriver(next.url, '/dynamic/first')

expect(await browser.elementById('dynamic-route').text()).toBe(
'dynamic route'
expect(await browser.elementById('dynamic-first').text()).toBe(
'dynamic first'
)
await browser.eval('window.__TEST_NO_RELOAD = true')

// Navigate to page with same root layout
await browser.elementByCss('a').click()
expect(
await browser.waitForElementByCss('#dynamic-second-hello').text()
).toBe('dynamic hello')
await browser.waitForElementByCss('#dynamic-first-second').text()
).toBe('dynamic first second')
expect(await browser.eval('window.__TEST_NO_RELOAD')).toBeTrue()

// Navigate to page with different root layout
Expand All @@ -172,5 +172,51 @@ describe.skip('app-dir root layout', () => {
).toBe('Inner basic route')
expect(await browser.eval('window.__TEST_NO_RELOAD')).toBeUndefined()
})

it('should work with dynamic catchall routes', async () => {
const browser = await webdriver(next.url, '/dynamic-catchall/slug')

expect(await browser.elementById('catchall-slug').text()).toBe(
'catchall slug'
)
await browser.eval('window.__TEST_NO_RELOAD = true')

// Navigate to page with same root layout
await browser.elementById('to-next-url').click()
expect(
await browser.waitForElementByCss('#catchall-slug-slug').text()
).toBe('catchall slug slug')
expect(await browser.eval('window.__TEST_NO_RELOAD')).toBeTrue()

// Navigate to page with different root layout
await browser.elementById('to-dynamic-first').click()
expect(await browser.elementById('dynamic-first').text()).toBe(
'dynamic first'
)
expect(await browser.eval('window.__TEST_NO_RELOAD')).toBeUndefined()
})

it('should work with static routes', async () => {
const browser = await webdriver(next.url, '/static-mpa-navigation/slug1')

expect(await browser.elementById('static-slug1').text()).toBe(
'static slug1'
)
await browser.eval('window.__TEST_NO_RELOAD = true')

// Navigate to page with same root layout
await browser.elementByCss('a').click()
expect(await browser.waitForElementByCss('#static-slug2').text()).toBe(
'static slug2'
)
expect(await browser.eval('window.__TEST_NO_RELOAD')).toBeTrue()

// Navigate to page with different root layout
await browser.elementByCss('a').click()
expect(await browser.elementById('basic-route').text()).toBe(
'Basic route'
)
expect(await browser.eval('window.__TEST_NO_RELOAD')).toBeUndefined()
})
})
})

0 comments on commit a5d5315

Please sign in to comment.