fix(nuxt): page hydration and double load (#7940)

Co-authored-by: Daniel Roe <daniel@roe.dev>

packages/nuxt/src/app/components/layout.ts

@@ -1,4 +1,5 @@
-import { defineComponent, unref, nextTick, onMounted, Ref, Transition, VNode } from 'vue'
+import { computed, defineComponent, h, inject, nextTick, onMounted, Ref, Transition, unref, VNode } from 'vue'
+import { RouteLocationNormalizedLoaded, useRoute as useVueRouterRoute } from 'vue-router'
 import { _wrapIf } from './utils'
 import { useRoute } from '#app'
 // @ts-ignore
@@ -6,6 +7,36 @@ import layouts from '#build/layouts'
 // @ts-ignore
 import { appLayoutTransition as defaultLayoutTransition } from '#build/nuxt.config.mjs'
 
+// TODO: revert back to defineAsyncComponent when https://github.com/vuejs/core/issues/6638 is resolved
+const LayoutLoader = defineComponent({
+  props: {
+    name: String,
+    ...process.dev ? { hasTransition: Boolean } : {}
+  },
+  async setup (props, context) {
+    let vnode: VNode
+
+    if (process.dev && process.client) {
+      onMounted(() => {
+        nextTick(() => {
+          if (props.name && ['#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.`)
+          }
+        })
+      })
+    }
+
+    const LayoutComponent = await layouts[props.name]().then((r: any) => r.default || r)
+
+    return () => {
+      if (process.dev && process.client && props.hasTransition) {
+        vnode = h(LayoutComponent, {}, context.slots)
+        return vnode
+      }
+      return h(LayoutComponent, {}, context.slots)
+    }
+  }
+})
 export default defineComponent({
   props: {
     name: {
@@ -14,7 +45,10 @@ export default defineComponent({
     }
   },
   setup (props, context) {
-    const route = useRoute()
+    // 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 route = injectedRoute === useRoute() ? useVueRouterRoute() : injectedRoute
+    const layout = computed(() => unref(props.name) ?? route.meta.layout as string ?? 'default')
 
     let vnode: VNode
     let _layout: string | false
@@ -29,26 +63,16 @@ export default defineComponent({
     }
 
     return () => {
-      const layout = unref(props.name) ?? route.meta.layout as string ?? 'default'
+      const hasLayout = layout.value && layout.value in layouts
-
+      if (process.dev && layout.value && !hasLayout && layout.value !== 'default') {
-      const hasLayout = layout && layout in layouts
+        console.warn(`Invalid layout \`${layout.value}\` selected.`)
-      if (process.dev && layout && !hasLayout && layout !== 'default') {
-        console.warn(`Invalid layout \`${layout}\` selected.`)
       }
 
       const transitionProps = route.meta.layoutTransition ?? defaultLayoutTransition
 
       // We avoid rendering layout transition if there is no layout to render
       return _wrapIf(Transition, hasLayout && transitionProps, {
-        default: () => {
+        default: () => _wrapIf(LayoutLoader, hasLayout && { key: layout.value, name: layout.value, hasTransition: !!transitionProps }, context.slots).default()
-          if (process.dev && process.client && transitionProps) {
-            _layout = layout
-            vnode = _wrapIf(layouts[layout], hasLayout, context.slots).default()
-            return vnode
-          }
-
-          return _wrapIf(layouts[layout], hasLayout, context.slots).default()
-        }
       }).default()
     }
   }

packages/nuxt/src/app/components/nuxt-root.vue

@@ -12,7 +12,7 @@ import { callWithNuxt, isNuxtError, showError, useError, useRoute, useNuxtApp }
 const ErrorComponent = defineAsyncComponent(() => import('#build/error-component.mjs').then(r => r.default || r))
 
 const nuxtApp = useNuxtApp()
-const onResolve = () => nuxtApp.callHook('app:suspense:resolve')
+const onResolve = nuxtApp.deferHydration()
 
 // Inject default route (outside of pages) as active route
 provide('_route', useRoute())

packages/nuxt/src/app/entry.ts

@@ -58,10 +58,6 @@ if (process.client) {
 
     const nuxt = createNuxtApp({ vueApp })
 
-    nuxt.hooks.hookOnce('app:suspense:resolve', () => {
-      nuxt.isHydrating = false
-    })
-
     try {
       await applyPlugins(nuxt, plugins)
     } catch (err) {

packages/nuxt/src/app/nuxt.ts

@@ -70,7 +70,10 @@ interface _NuxtApp {
     data: Ref<any>
     pending: Ref<boolean>
     error: Ref<any>
-  } | undefined>,
+  } | undefined>
+
+  isHydrating?: boolean
+  deferHydration: () => () => void | Promise<void>
 
   ssrContext?: NuxtSSRContext
   payload: {
@@ -108,6 +111,7 @@ export interface CreateOptions {
 }
 
 export function createNuxtApp (options: CreateOptions) {
+  let hydratingCount = 0
   const nuxtApp: NuxtApp = {
     provide: undefined,
     globalName: 'nuxt',
@@ -118,6 +122,24 @@ export function createNuxtApp (options: CreateOptions) {
       ...(process.client ? window.__NUXT__ : { serverRendered: true })
     }),
     isHydrating: process.client,
+    deferHydration () {
+      if (!nuxtApp.isHydrating) { return () => {} }
+
+      hydratingCount++
+      let called = false
+
+      return () => {
+        if (called) { return }
+
+        called = true
+        hydratingCount--
+
+        if (hydratingCount === 0) {
+          nuxtApp.isHydrating = false
+          return nuxtApp.callHook('app:suspense:resolve')
+        }
+      }
+    },
     _asyncDataPromises: {},
     _asyncData: {},
     ...options

packages/nuxt/src/core/templates.ts

@@ -156,10 +156,9 @@ export const layoutTemplate: NuxtTemplate<TemplateContext> = {
   filename: 'layouts.mjs',
   getContents ({ app }) {
     const layoutsObject = genObjectFromRawEntries(Object.values(app.layouts).map(({ name, file }) => {
-      return [name, `defineAsyncComponent(${genDynamicImport(file, { interopDefault: true })})`]
+      return [name, genDynamicImport(file, { interopDefault: true })]
     }))
     return [
-      'import { defineAsyncComponent } from \'vue\'',
       `export default ${layoutsObject}`
     ].join('\n')
   }

packages/nuxt/src/pages/runtime/page.ts

@@ -1,4 +1,4 @@
-import { computed, defineComponent, h, inject, provide, reactive, onMounted, nextTick, Suspense, Transition, KeepAliveProps, TransitionProps } from 'vue'
+import { computed, defineComponent, h, provide, reactive, onMounted, nextTick, Suspense, Transition, KeepAliveProps, TransitionProps } from 'vue'
 import type { DefineComponent, VNode } from 'vue'
 import { RouteLocationNormalized, RouteLocationNormalizedLoaded, RouterView } from 'vue-router'
 import type { RouteLocation } from 'vue-router'
@@ -9,8 +9,6 @@ import { _wrapIf } from '#app/components/utils'
 // @ts-ignore
 import { appPageTransition as defaultPageTransition, appKeepalive as defaultKeepaliveConfig } from '#build/nuxt.config.mjs'
 
-const isNestedKey = Symbol('isNested')
-
 export default defineComponent({
   name: 'NuxtPage',
   inheritAttrs: false,
@@ -37,9 +35,6 @@ export default defineComponent({
   setup (props, { attrs }) {
     const nuxtApp = useNuxtApp()
 
-    const isNested = inject(isNestedKey, false)
-    provide(isNestedKey, true)
-
     return () => {
       return h(RouterView, { name: props.name, route: props.route, ...attrs }, {
         default: (routeProps: RouterViewSlotProps) => {
@@ -48,13 +43,12 @@ export default defineComponent({
           const key = generateRouteKey(props.pageKey, routeProps)
           const transitionProps = props.transition ?? routeProps.route.meta.pageTransition ?? (defaultPageTransition as TransitionProps)
 
+          const done = nuxtApp.deferHydration()
+
           return _wrapIf(Transition, transitionProps,
-            wrapInKeepAlive(props.keepalive ?? routeProps.route.meta.keepalive ?? (defaultKeepaliveConfig as KeepAliveProps), isNested && nuxtApp.isHydrating
+            wrapInKeepAlive(props.keepalive ?? routeProps.route.meta.keepalive ?? (defaultKeepaliveConfig as KeepAliveProps), h(Suspense, {
-              // Include route children in parent suspense
-              ? h(Component, { key, routeProps, pageKey: key, hasTransition: !!transitionProps } as {})
-              : h(Suspense, {
               onPending: () => nuxtApp.callHook('page:start', routeProps.Component),
-                onResolve: () => nuxtApp.callHook('page:finish', routeProps.Component)
+              onResolve: () => nuxtApp.callHook('page:finish', routeProps.Component).finally(done)
             }, { default: () => h(Component, { key, routeProps, pageKey: key, hasTransition: !!transitionProps } as {}) })
             )).default()
         }

test/basic.test.ts

@@ -4,7 +4,7 @@ import { joinURL } from 'ufo'
 import { isWindows } from 'std-env'
 import { setup, fetch, $fetch, startServer, createPage, url } from '@nuxt/test-utils'
 // eslint-disable-next-line import/order
-import { expectNoClientErrors, renderPage } from './utils'
+import { expectNoClientErrors, renderPage, withLogs } from './utils'
 
 await setup({
   rootDir: fileURLToPath(new URL('./fixtures/basic', import.meta.url)),
@@ -471,6 +471,93 @@ describe('extends support', () => {
   })
 })
 
+// Bug #7337
+describe('deferred app suspense resolve', () => {
+  async function behaviour (path: string) {
+    await withLogs(async (page, logs) => {
+      await page.goto(url(path))
+      await page.waitForLoadState('networkidle')
+
+      // Wait for all pending micro ticks to be cleared in case hydration haven't finished yet.
+      await page.evaluate(() => new Promise(resolve => setTimeout(resolve, 0)))
+
+      const hydrationLogs = logs.filter(log => log.includes('isHydrating'))
+      expect(hydrationLogs.length).toBe(3)
+      expect(hydrationLogs.every(log => log === 'isHydrating: true'))
+    })
+  }
+  it('should wait for all suspense instance on initial hydration', async () => {
+    await behaviour('/async-parent/child')
+  })
+  it('should wait for all suspense instance on initial hydration', async () => {
+    await behaviour('/internal-layout/async-parent/child')
+  })
+})
+
+// Bug #6592
+describe('page key', () => {
+  it('should not cause run of setup if navigation not change page key and layout', async () => {
+    async function behaviour (path: string) {
+      await withLogs(async (page, logs) => {
+        await page.goto(url(`${path}/0`))
+        await page.waitForLoadState('networkidle')
+
+        await page.click(`[href="${path}/1"]`)
+        await page.waitForSelector('#page-1')
+
+        // Wait for all pending micro ticks to be cleared,
+        // so we are not resolved too early when there are repeated page loading
+        await page.evaluate(() => new Promise(resolve => setTimeout(resolve, 0)))
+
+        expect(logs.filter(l => l.includes('Child Setup')).length).toBe(1)
+      })
+    }
+    await behaviour('/fixed-keyed-child-parent')
+    await behaviour('/internal-layout/fixed-keyed-child-parent')
+  })
+  it('will cause run of setup if navigation changed page key', async () => {
+    async function behaviour (path: string) {
+      await withLogs(async (page, logs) => {
+        await page.goto(url(`${path}/0`))
+        await page.waitForLoadState('networkidle')
+
+        await page.click(`[href="${path}/1"]`)
+        await page.waitForSelector('#page-1')
+
+        // Wait for all pending micro ticks to be cleared,
+        // so we are not resolved too early when there are repeated page loading
+        await page.evaluate(() => new Promise(resolve => setTimeout(resolve, 0)))
+
+        expect(logs.filter(l => l.includes('Child Setup')).length).toBe(2)
+      })
+    }
+    await behaviour('/keyed-child-parent')
+    await behaviour('/internal-layout/keyed-child-parent')
+  })
+})
+
+// Bug #6592
+describe('layout change not load page twice', () => {
+  async function behaviour (path1: string, path2: string) {
+    await withLogs(async (page, logs) => {
+      await page.goto(url(path1))
+      await page.waitForLoadState('networkidle')
+      await page.click(`[href="${path2}"]`)
+      await page.waitForSelector('#with-layout2')
+
+      // Wait for all pending micro ticks to be cleared,
+      // so we are not resolved too early when there are repeated page loading
+      await page.evaluate(() => new Promise(resolve => setTimeout(resolve, 0)))
+
+      expect(logs.filter(l => l.includes('Layout2 Page Setup')).length).toBe(1)
+    })
+  }
+  it('should not cause run of page setup to repeat if layout changed', async () => {
+    await behaviour('/with-layout', '/with-layout2')
+    await behaviour('/internal-layout/with-layout', '/internal-layout/with-layout2')
+  })
+})
+
 describe('automatically keyed composables', () => {
   it('should automatically generate keys', async () => {
     const html = await $fetch('/keyed-composables')

test/fixtures/basic/layouts/custom-async.vue

@@ -0,0 +1,11 @@
+<template>
+  <div>
+    Custom Async Layout:
+    <slot />
+  </div>
+</template>
+
+<script setup>
+await Promise.resolve()
+console.log('isHydrating: ' + useNuxtApp().isHydrating)
+</script>

test/fixtures/basic/layouts/custom2.vue

@@ -0,0 +1,6 @@
+<template>
+  <div>
+    Custom2 Layout:
+    <slot />
+  </div>
+</template>

test/fixtures/basic/nuxt.config.ts

@@ -1,5 +1,7 @@
 import { addComponent, addVitePlugin, addWebpackPlugin } from '@nuxt/kit'
+import type { NuxtPage } from '@nuxt/schema'
 import { createUnplugin } from 'unplugin'
+import { withoutLeadingSlash } from 'ufo'
 
 export default defineNuxtConfig({
   app: {
@@ -50,6 +52,30 @@ export default defineNuxtConfig({
       }))
       addVitePlugin(plugin.vite())
       addWebpackPlugin(plugin.webpack())
+    },
+    function (_options, nuxt) {
+      const routesToDuplicate = ['/async-parent', '/fixed-keyed-child-parent', '/keyed-child-parent', '/with-layout', '/with-layout2']
+      const stripLayout = (page: NuxtPage) => ({
+        ...page,
+        children: page.children?.map(child => stripLayout(child)),
+        name: 'internal-' + page.name,
+        path: withoutLeadingSlash(page.path),
+        meta: {
+          ...page.meta || {},
+          layout: undefined,
+          _layout: page.meta?.layout
+        }
+      })
+      nuxt.hook('pages:extend', (pages) => {
+        const newPages = []
+        for (const page of pages) {
+          if (routesToDuplicate.includes(page.path)) {
+            newPages.push(stripLayout(page))
+          }
+        }
+        const internalParent = pages.find(page => page.path === '/internal-layout')
+        internalParent!.children = newPages
+      })
     }
   ],
   hooks: {

test/fixtures/basic/pages/async-parent.vue

@@ -0,0 +1,14 @@
+<template>
+  <div>
+    async-parent
+    <NuxtPage />
+  </div>
+</template>
+
+<script setup>
+await Promise.resolve()
+console.log('isHydrating: ' + useNuxtApp().isHydrating)
+definePageMeta({
+  layout: 'custom'
+})
+</script>

test/fixtures/basic/pages/async-parent/child.vue

@@ -0,0 +1,13 @@
+<template>
+  <div>
+    another-parent/child
+  </div>
+</template>
+
+<script setup>
+await Promise.resolve()
+console.log('isHydrating: ' + useNuxtApp().isHydrating)
+definePageMeta({
+  layout: 'custom-async'
+})
+</script>

test/fixtures/basic/pages/fixed-keyed-child-parent.vue

@@ -0,0 +1,6 @@
+<template>
+  <div>
+    fixed-keyed-child-parent
+    <NuxtPage />
+  </div>
+</template>

test/fixtures/basic/pages/fixed-keyed-child-parent/[foo].vue

@@ -0,0 +1,17 @@
+<template>
+  <div :id="`page-${route.params.foo}`">
+    [fixed-keyed-child-parent/{{ route.params.foo }}]
+    <NuxtLink to="../fixed-keyed-child-parent/1">
+      To another
+    </NuxtLink>
+  </div>
+</template>
+
+<script setup>
+console.log('Running Child Setup')
+const route = useRoute()
+
+definePageMeta({
+  key: 'keyed'
+})
+</script>

test/fixtures/basic/pages/internal-layout.vue

@@ -0,0 +1,15 @@
+<script setup lang="ts">
+definePageMeta({
+  layout: false
+})
+
+const route = useRoute()
+</script>
+
+<template>
+  <div>
+    <NuxtLayout :name="route.meta._layout as string || 'default'">
+      <NuxtPage />
+    </NuxtLayout>
+  </div>
+</template>

test/fixtures/basic/pages/keyed-child-parent.vue

@@ -0,0 +1,6 @@
+<template>
+  <div>
+    keyed-child-parent
+    <NuxtPage />
+  </div>
+</template>

test/fixtures/basic/pages/keyed-child-parent/[foo].vue

@@ -0,0 +1,17 @@
+<template>
+  <div :id="`page-${route.params.foo}`">
+    [keyed-child-parent/{{ route.params.foo }}]
+    <NuxtLink to="../keyed-child-parent/1">
+      To another
+    </NuxtLink>
+  </div>
+</template>
+
+<script setup>
+console.log('Running Child Setup')
+const route = useRoute()
+
+definePageMeta({
+  key: r => 'keyed-' + r.params.foo
+})
+</script>

test/fixtures/basic/pages/with-layout.vue

@@ -7,5 +7,8 @@ definePageMeta({
 <template>
   <div>
     <div>with-layout.vue</div>
+    <NuxtLink to="./with-layout2">
+      to another page
+    </NuxtLink>
   </div>
 </template>

test/fixtures/basic/pages/with-layout2.vue

@@ -0,0 +1,13 @@
+
+<script setup>
+definePageMeta({
+  layout: 'custom2'
+})
+
+console.log('Running With Layout2 Page Setup')
+</script>
+<template>
+  <div id="with-layout2">
+    <div>with-layout2.vue</div>
+  </div>
+</template>

test/utils.ts

@@ -1,5 +1,6 @@
 import { expect } from 'vitest'
-import { getBrowser, url, useTestContext } from '@nuxt/test-utils'
+import type { Page } from 'playwright'
+import { createPage, getBrowser, url, useTestContext } from '@nuxt/test-utils'
 
 export async function renderPage (path = '/') {
   const ctx = useTestContext()
@@ -48,3 +49,22 @@ export async function expectNoClientErrors (path: string) {
   expect(consoleLogErrors).toEqual([])
   expect(consoleLogWarnings).toEqual([])
 }
+
+export async function withLogs (callback: (page: Page, logs: string[]) => Promise<void>) {
+  let done = false
+  const page = await createPage()
+  const logs: string[] = []
+  page.on('console', (msg) => {
+    const text = msg.text()
+    if (done) {
+      throw new Error('Test finished prematurely')
+    }
+    logs.push(text)
+  })
+
+  try {
+    await callback(page, logs)
+  } finally {
+    done = true
+  }
+}