Skip to content
This repository has been archived by the owner on Apr 6, 2023. It is now read-only.

Commit

Permalink
fix(nuxt): page hydration and double load (#7940)
Browse files Browse the repository at this point in the history
Co-authored-by: Daniel Roe <daniel@roe.dev>
  • Loading branch information
mmis1000 and danielroe authored Oct 8, 2022
1 parent 186a626 commit c404cb1
Show file tree
Hide file tree
Showing 20 changed files with 329 additions and 40 deletions.
56 changes: 40 additions & 16 deletions packages/nuxt/src/app/components/layout.ts
Original file line number Diff line number Diff line change
@@ -1,11 +1,42 @@
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
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: {
Expand All @@ -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
Expand All @@ -29,26 +63,16 @@ export default defineComponent({
}

return () => {
const layout = unref(props.name) ?? route.meta.layout as string ?? 'default'

const hasLayout = layout && layout in layouts
if (process.dev && layout && !hasLayout && layout !== 'default') {
console.warn(`Invalid layout \`${layout}\` selected.`)
const hasLayout = layout.value && layout.value in layouts
if (process.dev && layout.value && !hasLayout && layout.value !== 'default') {
console.warn(`Invalid layout \`${layout.value}\` 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: () => {
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: () => _wrapIf(LayoutLoader, hasLayout && { key: layout.value, name: layout.value, hasTransition: !!transitionProps }, context.slots).default()
}).default()
}
}
Expand Down
2 changes: 1 addition & 1 deletion packages/nuxt/src/app/components/nuxt-root.vue
Original file line number Diff line number Diff line change
Expand Up @@ -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())
Expand Down
4 changes: 0 additions & 4 deletions packages/nuxt/src/app/entry.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down
24 changes: 23 additions & 1 deletion packages/nuxt/src/app/nuxt.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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: {
Expand Down Expand Up @@ -108,6 +111,7 @@ export interface CreateOptions {
}

export function createNuxtApp (options: CreateOptions) {
let hydratingCount = 0
const nuxtApp: NuxtApp = {
provide: undefined,
globalName: 'nuxt',
Expand All @@ -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
Expand Down
3 changes: 1 addition & 2 deletions packages/nuxt/src/core/templates.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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')
}
Expand Down
20 changes: 7 additions & 13 deletions packages/nuxt/src/pages/runtime/page.ts
Original file line number Diff line number Diff line change
@@ -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'
Expand All @@ -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,
Expand All @@ -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) => {
Expand All @@ -48,14 +43,13 @@ 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
// 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)
}, { default: () => h(Component, { key, routeProps, pageKey: key, hasTransition: !!transitionProps } as {}) })
wrapInKeepAlive(props.keepalive ?? routeProps.route.meta.keepalive ?? (defaultKeepaliveConfig as KeepAliveProps), h(Suspense, {
onPending: () => nuxtApp.callHook('page:start', routeProps.Component),
onResolve: () => nuxtApp.callHook('page:finish', routeProps.Component).finally(done)
}, { default: () => h(Component, { key, routeProps, pageKey: key, hasTransition: !!transitionProps } as {}) })
)).default()
}
})
Expand Down
89 changes: 88 additions & 1 deletion test/basic.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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)),
Expand Down Expand Up @@ -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')
Expand Down
11 changes: 11 additions & 0 deletions test/fixtures/basic/layouts/custom-async.vue
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
<template>
<div>
Custom Async Layout:
<slot />
</div>
</template>

<script setup>
await Promise.resolve()
console.log('isHydrating: ' + useNuxtApp().isHydrating)
</script>
6 changes: 6 additions & 0 deletions test/fixtures/basic/layouts/custom2.vue
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
<template>
<div>
Custom2 Layout:
<slot />
</div>
</template>
26 changes: 26 additions & 0 deletions test/fixtures/basic/nuxt.config.ts
Original file line number Diff line number Diff line change
@@ -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: {
Expand Down Expand Up @@ -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: {
Expand Down
14 changes: 14 additions & 0 deletions test/fixtures/basic/pages/async-parent.vue
Original file line number Diff line number Diff line change
@@ -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>
13 changes: 13 additions & 0 deletions test/fixtures/basic/pages/async-parent/child.vue
Original file line number Diff line number Diff line change
@@ -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>
Loading

0 comments on commit c404cb1

Please sign in to comment.