fix(nuxt): preserve old vnode when leaving nested route (#21823)

This commit is contained in:
Daniel Roe 2023-07-05 12:39:39 +02:00 committed by GitHub
parent 00fb33379c
commit d0dde6426f
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
15 changed files with 305 additions and 84 deletions

View File

@ -0,0 +1,10 @@
import type { InjectionKey } from 'vue'
import type { RouteLocationNormalizedLoaded } from 'vue-router'
export interface LayoutMeta {
isCurrent: (route: RouteLocationNormalizedLoaded) => boolean
}
export const LayoutMetaSymbol: InjectionKey<LayoutMeta> = Symbol('layout-meta')
export const PageRouteSymbol: InjectionKey<RouteLocationNormalizedLoaded> = Symbol('route')

View File

@ -1,7 +1,9 @@
import type { InjectionKey, Ref, VNode } from 'vue'
import type { Ref, VNode } from 'vue'
import { Suspense, Transition, computed, defineComponent, h, inject, mergeProps, nextTick, onMounted, provide, ref, unref } from 'vue'
import type { RouteLocationNormalizedLoaded } from 'vue-router'
import { _wrapIf } from './utils'
import { LayoutMetaSymbol, PageRouteSymbol } from './injections'
import { useRoute } from '#app/composables/router'
// @ts-expect-error virtual file
import { useRoute as useVueRouterRoute } from '#build/pages'
@ -11,12 +13,6 @@ import layouts from '#build/layouts'
import { appLayoutTransition as defaultLayoutTransition } from '#build/nuxt.config.mjs'
import { useNuxtApp } from '#app'
export interface LayoutMeta {
isCurrent: (route: RouteLocationNormalizedLoaded) => boolean
}
export const LayoutMetaSymbol: InjectionKey<LayoutMeta> = Symbol('layout-meta')
export default defineComponent({
name: 'NuxtLayout',
inheritAttrs: false,
@ -29,7 +25,7 @@ export default defineComponent({
setup (props, context) {
const nuxtApp = useNuxtApp()
// Need to ensure (if we are not a child of `<NuxtPage>`) that we use synchronous route (not deferred)
const injectedRoute = inject('_route') as RouteLocationNormalizedLoaded
const injectedRoute = inject(PageRouteSymbol)
const route = injectedRoute === useRoute() ? useVueRouterRoute() : injectedRoute
const layout = computed(() => unref(props.name) ?? route.meta.layout as string ?? 'default')
@ -49,13 +45,14 @@ export default defineComponent({
// We avoid rendering layout transition if there is no layout to render
return _wrapIf(Transition, hasLayout && transitionProps, {
default: () => h(Suspense, { suspensible: true, onResolve: () => { nextTick(done) } }, {
default: () => _wrapIf(LayoutProvider, hasLayout && {
// @ts-expect-error seems to be an issue in vue types
default: () => h(LayoutProvider, {
layoutProps: mergeProps(context.attrs, { ref: layoutRef }),
key: layout.value,
name: layout.value,
shouldProvide: !props.name,
hasTransition: !!transitionProps
}, context.slots).default()
}, context.slots)
})
}).default()
}
@ -67,7 +64,7 @@ const LayoutProvider = defineComponent({
inheritAttrs: false,
props: {
name: {
type: String
type: [String, Boolean]
},
layoutProps: {
type: Object
@ -81,33 +78,45 @@ const LayoutProvider = defineComponent({
},
setup (props, context) {
// Prevent reactivity when the page will be rerendered in a different suspense fork
if (props.shouldProvide) {
// eslint-disable-next-line vue/no-setup-props-destructure
const name = props.name
if (props.shouldProvide) {
provide(LayoutMetaSymbol, {
isCurrent: (route: RouteLocationNormalizedLoaded) => name === (route.meta.layout ?? 'default')
})
}
let vnode: VNode
let vnode: VNode | undefined
if (process.dev && process.client) {
onMounted(() => {
nextTick(() => {
if (['#comment', '#text'].includes(vnode?.el?.nodeName)) {
console.warn(`[nuxt] \`${props.name}\` layout does not have a single root node and will cause errors when navigating between routes.`)
if (name) {
console.warn(`[nuxt] \`${name}\` layout does not have a single root node and will cause errors when navigating between routes.`)
} else {
console.warn('[nuxt] `<NuxtLayout>` needs to be passed a single root node in its default slot.')
}
}
})
})
}
return () => {
if (!name || (typeof name === 'string' && !(name in layouts))) {
if (process.dev && process.client && props.hasTransition) {
vnode = h(layouts[props.name], props.layoutProps, context.slots)
vnode = context.slots.default?.() as VNode | undefined
return vnode
}
return context.slots.default?.()
}
if (process.dev && process.client && props.hasTransition) {
vnode = h(layouts[name], props.layoutProps, context.slots)
return vnode
}
return h(layouts[props.name], props.layoutProps, context.slots)
return h(layouts[name], props.layoutProps, context.slots)
}
}
})

View File

@ -12,6 +12,7 @@ import { defineAsyncComponent, onErrorCaptured, onServerPrefetch, provide } from
import { useNuxtApp } from '#app/nuxt'
import { isNuxtError, showError, useError } from '#app/composables/error'
import { useRoute } from '#app/composables/router'
import { PageRouteSymbol } from '#app/components/injections'
import AppComponent from '#build/app-component.mjs'
import ErrorComponent from '#build/error-component.mjs'
@ -27,7 +28,7 @@ const SingleRenderer = process.test && process.dev && process.server && url.star
.then(r => r.default(process.server ? url : window.location.href)))
// Inject default route (outside of pages) as active route
provide('_route', useRoute())
provide(PageRouteSymbol, useRoute())
// vue:setup hook
const results = nuxtApp.hooks.callHookWith(hooks => hooks.map(hook => hook()), 'vue:setup')

View File

@ -0,0 +1,57 @@
import { computed, defineComponent, h, nextTick, onMounted, provide, reactive } from 'vue'
import type { Ref, VNode } from 'vue'
import type { RouteLocation, RouteLocationNormalizedLoaded } from '#vue-router'
import { PageRouteSymbol } from '#app/components/injections'
export const RouteProvider = defineComponent({
name: 'RouteProvider',
props: {
vnode: {
type: Object as () => VNode,
required: true
},
route: {
type: Object as () => RouteLocationNormalizedLoaded,
required: true
},
vnodeRef: Object as () => Ref<any>,
renderKey: String,
trackRootNodes: Boolean
},
setup (props) {
// Prevent reactivity when the page will be rerendered in a different suspense fork
// eslint-disable-next-line vue/no-setup-props-destructure
const previousKey = props.renderKey
// eslint-disable-next-line vue/no-setup-props-destructure
const previousRoute = props.route
// Provide a reactive route within the page
const route = {} as RouteLocation
for (const key in props.route) {
(route as any)[key] = computed(() => previousKey === props.renderKey ? props.route[key as keyof RouteLocationNormalizedLoaded] : previousRoute[key as keyof RouteLocationNormalizedLoaded])
}
provide(PageRouteSymbol, reactive(route))
let vnode: VNode
if (process.dev && process.client && props.trackRootNodes) {
onMounted(() => {
nextTick(() => {
if (['#comment', '#text'].includes(vnode?.el?.nodeName)) {
const filename = (vnode?.type as any).__file
console.warn(`[nuxt] \`${filename}\` does not have a single root node and will cause errors when navigating between routes.`)
}
})
})
}
return () => {
if (process.dev && process.client) {
vnode = h(props.vnode, { ref: props.vnodeRef })
return vnode
}
return h(props.vnode, { ref: props.vnodeRef })
}
}
})

View File

@ -10,6 +10,7 @@ import { createError, showError } from './error'
import { useState } from './state'
import type { PageMeta } from '#app'
import { PageRouteSymbol } from '#app/components/injections'
export const useRouter: typeof _useRouter = () => {
return useNuxtApp()?.$router as Router
@ -20,7 +21,7 @@ export const useRoute: typeof _useRoute = () => {
console.warn('[nuxt] Calling `useRoute` within middleware may lead to misleading results. Instead, use the (to, from) arguments passed to the middleware to access the new and old routes.')
}
if (hasInjectionContext()) {
return inject('_route', useNuxtApp()._route)
return inject(PageRouteSymbol, useNuxtApp()._route)
}
return useNuxtApp()._route
}

View File

@ -1,14 +1,15 @@
import { Suspense, Transition, computed, defineComponent, h, inject, nextTick, onMounted, provide, reactive, ref } from 'vue'
import { Suspense, Transition, defineComponent, h, inject, nextTick, ref } from 'vue'
import type { KeepAliveProps, TransitionProps, VNode } from 'vue'
import { RouterView } from '#vue-router'
import { defu } from 'defu'
import type { RouteLocation, RouteLocationNormalized, RouteLocationNormalizedLoaded } from '#vue-router'
import type { RouteLocationNormalized, RouteLocationNormalizedLoaded } from '#vue-router'
import type { RouterViewSlotProps } from './utils'
import { generateRouteKey, wrapInKeepAlive } from './utils'
import { RouteProvider } from '#app/components/route-provider'
import { useNuxtApp } from '#app/nuxt'
import { _wrapIf } from '#app/components/utils'
import { LayoutMetaSymbol } from '#app/components/layout'
import { LayoutMetaSymbol, PageRouteSymbol } from '#app/components/injections'
// @ts-expect-error virtual file
import { appKeepalive as defaultKeepaliveConfig, appPageTransition as defaultPageTransition } from '#build/nuxt.config.mjs'
@ -38,6 +39,7 @@ export default defineComponent({
setup (props, { attrs, expose }) {
const nuxtApp = useNuxtApp()
const pageRef = ref()
const forkRoute = inject(PageRouteSymbol, null)
expose({ pageRef })
@ -47,13 +49,32 @@ export default defineComponent({
return () => {
return h(RouterView, { name: props.name, route: props.route, ...attrs }, {
default: (routeProps: RouterViewSlotProps) => {
if (!routeProps.Component) { return }
const isRenderingNewRouteInOldFork = process.client && haveParentRoutesRendered(forkRoute, routeProps.route, routeProps.Component)
const hasSameChildren = process.client && forkRoute && forkRoute.matched.length === routeProps.route.matched.length
if (!routeProps.Component) {
// If we're rendering a `<NuxtPage>` child route on navigation to a route which lacks a child page
// we'll render the old vnode until the new route finishes resolving
if (process.client && vnode && !hasSameChildren) {
return vnode
}
return
}
// Return old vnode if we are rendering _new_ page suspense fork in _old_ layout suspense fork
if (vnode && _layoutMeta && !_layoutMeta.isCurrent(routeProps.route)) {
if (process.client && vnode && _layoutMeta && !_layoutMeta.isCurrent(routeProps.route)) {
return vnode
}
if (process.client && isRenderingNewRouteInOldFork && forkRoute && (!_layoutMeta || _layoutMeta?.isCurrent(forkRoute))) {
// if leaving a route with an existing child route, render the old vnode
if (hasSameChildren) {
return vnode
}
// If _leaving_ null child route, return null vnode
return null
}
const key = generateRouteKey(routeProps, props.pageKey)
const done = nuxtApp.deferHydration()
@ -70,7 +91,17 @@ export default defineComponent({
suspensible: true,
onPending: () => nuxtApp.callHook('page:start', routeProps.Component),
onResolve: () => { nextTick(() => nuxtApp.callHook('page:finish', routeProps.Component).finally(done)) }
}, { default: () => h(RouteProvider, { key, routeProps, pageKey: key, hasTransition, pageRef } as {}) })
}, {
// @ts-expect-error seems to be an issue in vue types
default: () => h(RouteProvider, {
key,
vnode: routeProps.Component,
route: routeProps.route,
renderKey: key,
trackRootNodes: hasTransition,
vnodeRef: pageRef
})
})
)).default()
return vnode
@ -92,45 +123,15 @@ function _mergeTransitionProps (routeProps: TransitionProps[]): TransitionProps
return defu(..._props as [TransitionProps, TransitionProps])
}
const RouteProvider = defineComponent({
name: 'RouteProvider',
// TODO: Type props
// eslint-disable-next-line vue/require-prop-types
props: ['routeProps', 'pageKey', 'hasTransition', 'pageRef'],
setup (props) {
// Prevent reactivity when the page will be rerendered in a different suspense fork
// eslint-disable-next-line vue/no-setup-props-destructure
const previousKey = props.pageKey
// eslint-disable-next-line vue/no-setup-props-destructure
const previousRoute = props.routeProps.route
function haveParentRoutesRendered (fork: RouteLocationNormalizedLoaded | null, newRoute: RouteLocationNormalizedLoaded, Component?: VNode) {
if (!fork) { return false }
// Provide a reactive route within the page
const route = {} as RouteLocation
for (const key in props.routeProps.route) {
(route as any)[key] = computed(() => previousKey === props.pageKey ? props.routeProps.route[key] : previousRoute[key])
}
const index = newRoute.matched.findIndex(m => m.components?.default === Component?.type)
if (!index || index === -1) { return false }
provide('_route', reactive(route))
let vnode: VNode
if (process.dev && process.client && props.hasTransition) {
onMounted(() => {
nextTick(() => {
if (['#comment', '#text'].includes(vnode?.el?.nodeName)) {
const filename = (vnode?.type as any).__file
console.warn(`[nuxt] \`${filename}\` does not have a single root node and will cause errors when navigating between routes.`)
// we only care whether the parent route components have had to rerender
return newRoute.matched.slice(0, index)
.some(
(c, i) => c.components?.default !== fork.matched[i]?.components?.default) ||
(Component && generateRouteKey({ route: newRoute, Component }) !== generateRouteKey({ route: fork, Component }))
}
})
})
}
return () => {
if (process.dev && process.client) {
vnode = h(props.routeProps.Component, { ref: props.pageRef })
return vnode
}
return h(props.routeProps.Component, { ref: props.pageRef })
}
}
})

View File

@ -1037,21 +1037,31 @@ describe('deferred app suspense resolve', () => {
})
describe('nested suspense', () => {
const navigations = [
const navigations = ([
['/suspense/sync-1/async-1/', '/suspense/sync-2/async-1/'],
['/suspense/sync-1/sync-1/', '/suspense/sync-2/async-1/'],
['/suspense/async-1/async-1/', '/suspense/async-2/async-1/'],
['/suspense/async-1/sync-1/', '/suspense/async-2/async-1/']
]
]).flatMap(([start, end]) => [
[start, end],
[start, end + '?layout=custom'],
[start + '?layout=custom', end]
])
it.each(navigations)('should navigate from %s to %s with no white flash', async (start, nav) => {
const page = await createPage(start, {})
const logs: string[] = []
page.on('console', (msg) => {
const text = msg.text()
if (text.includes('[vite]') || text.includes('<Suspense> is an experimental feature')) { return }
logs.push(msg.text())
})
await page.waitForLoadState('networkidle')
const slug = nav.replace(/[/-]+/g, '-')
const slug = nav.replace(/\?.*$/, '').replace(/[/-]+/g, '-')
await page.click(`[href^="${nav}"]`)
const text = await page.waitForFunction(slug => document.querySelector(`#${slug}`)?.innerHTML, slug)
const text = await page.waitForFunction(slug => document.querySelector(`main:has(#child${slug})`)?.innerHTML, slug)
// @ts-expect-error TODO: fix upstream in playwright - types for evaluate are broken
.then(r => r.evaluate(r => r))
@ -1061,6 +1071,114 @@ describe('nested suspense', () => {
// const text = await parent.innerText()
expect(text).toContain('Async child: 2 - 1')
expect(text).toContain('parent: 2')
const first = start.match(/\/suspense\/(?<parentType>a?sync)-(?<parentNum>\d)\/(?<childType>a?sync)-(?<childNum>\d)\//)!.groups!
const last = nav.match(/\/suspense\/(?<parentType>a?sync)-(?<parentNum>\d)\/(?<childType>a?sync)-(?<childNum>\d)\//)!.groups!
expect(logs.sort()).toEqual([
// [first load] from parent
`[${first.parentType}]`,
...first.parentType === 'async' ? ['[async] running async data'] : [],
// [first load] from child
`[${first.parentType}] [${first.childType}]`,
...first.childType === 'async' ? [`[${first.parentType}] [${first.parentNum}] [async] [${first.childNum}] running async data`] : [],
// [navigation] from parent
`[${last.parentType}]`,
...last.parentType === 'async' ? ['[async] running async data'] : [],
// [navigation] from child
`[${last.parentType}] [${last.childType}]`,
...last.childType === 'async' ? [`[${last.parentType}] [${last.parentNum}] [async] [${last.childNum}] running async data`] : []
].sort())
await page.close()
})
const outwardNavigations = [
['/suspense/async-2/async-1/', '/suspense/async-1/'],
['/suspense/async-2/sync-1/', '/suspense/async-1/']
]
it.each(outwardNavigations)('should navigate from %s to a parent %s with no white flash', async (start, nav) => {
const page = await createPage(start, {})
const logs: string[] = []
page.on('console', (msg) => {
const text = msg.text()
if (text.includes('[vite]') || text.includes('<Suspense> is an experimental feature')) { return }
logs.push(msg.text())
})
await page.waitForLoadState('networkidle')
await page.waitForSelector(`main:has(#child${start.replace(/[/-]+/g, '-')})`)
const slug = start.replace(/[/-]+/g, '-')
await page.click(`[href^="${nav}"]`)
// wait until child selector disappears and grab HTML of parent
const text = await page.waitForFunction(slug => document.querySelector(`main:not(:has(#child${slug}))`)?.innerHTML, slug)
// @ts-expect-error TODO: fix upstream in playwright - types for evaluate are broken
.then(r => r.evaluate(r => r))
expect(text).toContain('Async parent: 1')
const first = start.match(/\/suspense\/(?<parentType>a?sync)-(?<parentNum>\d)\/(?<childType>a?sync)-(?<childNum>\d)\//)!.groups!
const last = nav.match(/\/suspense\/(?<parentType>a?sync)-(?<parentNum>\d)\//)!.groups!
expect(logs.sort()).toEqual([
// [first load] from parent
`[${first.parentType}]`,
...first.parentType === 'async' ? ['[async] running async data'] : [],
// [first load] from child
`[${first.parentType}] [${first.childType}]`,
...first.childType === 'async' ? [`[${first.parentType}] [${first.parentNum}] [async] [${first.childNum}] running async data`] : [],
// [navigation] from parent
`[${last.parentType}]`,
...last.parentType === 'async' ? ['[async] running async data'] : []
].sort())
await page.close()
})
const inwardNavigations = [
['/suspense/async-2/', '/suspense/async-1/async-1/'],
['/suspense/async-2/', '/suspense/async-1/sync-1/']
]
it.each(inwardNavigations)('should navigate from %s to a child %s with no white flash', async (start, nav) => {
const page = await createPage(start, {})
const logs: string[] = []
page.on('console', (msg) => {
const text = msg.text()
if (text.includes('[vite]') || text.includes('<Suspense> is an experimental feature')) { return }
logs.push(msg.text())
})
await page.waitForLoadState('networkidle')
const slug = nav.replace(/[/-]+/g, '-')
await page.click(`[href^="${nav}"]`)
// wait until child selector appears and grab HTML of parent
const text = await page.waitForFunction(slug => document.querySelector(`main:has(#child${slug})`)?.innerHTML, slug)
// @ts-expect-error TODO: fix upstream in playwright - types for evaluate are broken
.then(r => r.evaluate(r => r))
// const text = await parent.innerText()
expect(text).toContain('Async parent: 1')
const first = start.match(/\/suspense\/(?<parentType>a?sync)-(?<parentNum>\d)\//)!.groups!
const last = nav.match(/\/suspense\/(?<parentType>a?sync)-(?<parentNum>\d)\/(?<childType>a?sync)-(?<childNum>\d)\//)!.groups!
expect(logs.sort()).toEqual([
// [first load] from parent
`[${first.parentType}]`,
...first.parentType === 'async' ? ['[async] running async data'] : [],
// [navigation] from parent
`[${last.parentType}]`,
...last.parentType === 'async' ? ['[async] running async data'] : [],
// [navigation] from child
`[${last.parentType}] [${last.childType}]`,
...last.childType === 'async' ? [`[${last.parentType}] [${last.parentNum}] [async] [${last.childNum}] running async data`] : []
].sort())
await page.close()
})

View File

@ -32,7 +32,7 @@ describe.skipIf(process.env.SKIP_BUNDLE_SIZE === 'true' || process.env.ECOSYSTEM
const serverDir = join(rootDir, '.output/server')
const serverStats = await analyzeSizes(['**/*.mjs', '!node_modules'], serverDir)
expect.soft(roundToKilobytes(serverStats.totalBytes)).toMatchInlineSnapshot('"64.0k"')
expect.soft(roundToKilobytes(serverStats.totalBytes)).toMatchInlineSnapshot('"64.1k"')
const modules = await analyzeSizes('node_modules/**/*', serverDir)
expect.soft(roundToKilobytes(modules.totalBytes)).toMatchInlineSnapshot('"2329k"')

View File

@ -1,19 +1,43 @@
<script setup>
const links = new Set(['sync', 'async'].flatMap(parent => [1, 2].flatMap(p => ['sync', 'async'].flatMap(child => [null, 1, 2].map(c => !c ? `/suspense/${parent}-${p}/` : `/suspense/${parent}-${p}/${child}-${c}/`)))))
definePageMeta({
// Nested <Suspense> + <Transition> is still buggy
pageTransition: false,
layoutTransition: false
layoutTransition: false,
middleware: (to) => {
if ('layout' in to.query) {
if (to.query.layout === 'false') {
to.meta.layout = false
} else {
to.meta.layout = to.query.layout
}
}
}
})
const links = ['sync', 'async'].flatMap(parent => [1, 2].flatMap(p => ['sync', 'async'].flatMap(child => [1, 2].map(c => `/suspense/${parent}-${p}/${child}-${c}/`))))
</script>
<template>
<div>
This exists to test synchronous transitions between nested Suspense components.
<hr>
<div style="display: flex; flex-direction: row; gap: 10vw;">
<div>
<h1>With extended layout</h1>
<NuxtLink v-for="link in links" :key="link" :to="link" style="display: block;">
{{ link }}
</NuxtLink>
</div>
<div>
<h1>With custom layout</h1>
<NuxtLink v-for="link in links" :key="link" :to="`${link}?layout=custom`" style="display: block;">
{{ link }}
</NuxtLink>
</div>
</div>
<NuxtLink to="/internal-layout/async-parent/child">
Internal layout page
</NuxtLink>
<hr>
<div>
<NuxtPage />

View File

@ -6,9 +6,9 @@ console.log('[async] running async data')
</script>
<template>
<div :id="route.path.replace(/[/-]+/g, '-')">
<main :id="route.path.replace(/[/-]+/g, '-')">
Async parent: {{ route.params.parent }}
<hr>
<NuxtPage />
</div>
</main>
</template>

View File

@ -7,7 +7,7 @@ const data = route.params
</script>
<template>
<div>
<div :id="'child' + route.path.replace(/[/-]+/g, '-')">
Async child: {{ route.params.parent }} - {{ route.params.child }}
<hr>
{{ data }}

View File

@ -4,7 +4,7 @@ const route = useRoute('suspense-async-parent-sync-child')
</script>
<template>
<div>
<div :id="'child' + route.path.replace(/[/-]+/g, '-')">
Sync child: {{ route.params.parent }} - {{ route.params.child }}
</div>
</template>

View File

@ -4,9 +4,9 @@ const route = useRoute('suspense-async-parent')
</script>
<template>
<div :id="route.path.replace(/[/-]+/g, '-')">
<main :id="route.path.replace(/[/-]+/g, '-')">
Sync parent: {{ route.params.parent }}
<hr>
<NuxtPage />
</div>
</main>
</template>

View File

@ -2,12 +2,12 @@
console.log('[sync] [async]')
const route = useRoute('suspense-async-parent-sync-child')
await new Promise(resolve => setTimeout(resolve, 500))
console.log(`[async] [${route.params.parent}] [async] [${route.params.child}] running async data`)
console.log(`[sync] [${route.params.parent}] [async] [${route.params.child}] running async data`)
const data = route.params
</script>
<template>
<div>
<div :id="'child' + route.path.replace(/[/-]+/g, '-')">
Async child: {{ route.params.parent }} - {{ route.params.child }}
<hr>
{{ data }}

View File

@ -4,7 +4,7 @@ const route = useRoute('suspense-sync-parent-sync-child')
</script>
<template>
<div>
<div :id="'child' + route.path.replace(/[/-]+/g, '-')">
Sync child: {{ route.params.parent }} - {{ route.params.child }}
</div>
</template>