From 3aa51143b8b976e36a345cb0e1f057908e81d2e1 Mon Sep 17 00:00:00 2001 From: Mmis1000 Date: Thu, 15 Sep 2022 22:37:53 +0800 Subject: [PATCH 01/11] Try to fix nuxtApp.isHydration by using ref count Use ref count to wait for all existing suspense instance to finish before the `app:suspense:resolve` is called --- .../nuxt/src/app/components/nuxt-root.vue | 6 ++++- packages/nuxt/src/app/entry.ts | 4 ---- packages/nuxt/src/app/nuxt.ts | 23 +++++++++++++++++++ packages/nuxt/src/pages/runtime/page.ts | 13 +++++++++-- 4 files changed, 39 insertions(+), 7 deletions(-) diff --git a/packages/nuxt/src/app/components/nuxt-root.vue b/packages/nuxt/src/app/components/nuxt-root.vue index 173eddf9808..695931936bf 100644 --- a/packages/nuxt/src/app/components/nuxt-root.vue +++ b/packages/nuxt/src/app/components/nuxt-root.vue @@ -12,7 +12,11 @@ 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 done = nuxtApp.deferHydration() + +const onResolve = () => { + done() +} // Inject default route (outside of pages) as active route provide('_route', useRoute()) diff --git a/packages/nuxt/src/app/entry.ts b/packages/nuxt/src/app/entry.ts index f612d4ffb78..c71de3a5f42 100644 --- a/packages/nuxt/src/app/entry.ts +++ b/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) { diff --git a/packages/nuxt/src/app/nuxt.ts b/packages/nuxt/src/app/nuxt.ts index 881d9a248a0..f14b817a6b3 100644 --- a/packages/nuxt/src/app/nuxt.ts +++ b/packages/nuxt/src/app/nuxt.ts @@ -108,6 +108,7 @@ export interface CreateOptions { } export function createNuxtApp (options: CreateOptions) { + let hydratingCount: number = 0 const nuxtApp: NuxtApp = { provide: undefined, globalName: 'nuxt', @@ -118,6 +119,28 @@ export function createNuxtApp (options: CreateOptions) { ...(process.client ? window.__NUXT__ : { serverRendered: true }) }), isHydrating: process.client, + deferHydration () { + if (nuxtApp.isHydrating) { + hydratingCount++ + let called = false + return () => { + if (called) { + return + } else { + called = true + } + + hydratingCount-- + + if (hydratingCount === 0) { + nuxtApp.isHydrating = false + nuxtApp.callHook('app:suspense:resolve') + } + } + } else { + return () => {} + } + }, _asyncDataPromises: {}, _asyncData: {}, ...options diff --git a/packages/nuxt/src/pages/runtime/page.ts b/packages/nuxt/src/pages/runtime/page.ts index 1254b8a43d3..c2558a77213 100644 --- a/packages/nuxt/src/pages/runtime/page.ts +++ b/packages/nuxt/src/pages/runtime/page.ts @@ -48,13 +48,22 @@ export default defineComponent({ const key = generateRouteKey(props.pageKey, routeProps) const transitionProps = props.transition ?? routeProps.route.meta.pageTransition ?? (defaultPageTransition as TransitionProps) + let done: () => {} + + if (!isNested) { + 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), isNested // 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: () => { + done?.() + nuxtApp.callHook('page:finish', routeProps.Component) + } }, { default: () => h(Component, { key, routeProps, pageKey: key, hasTransition: !!transitionProps } as {}) }) )).default() } From d85d0f17a6ddf8906863feb27760c888c49fa231 Mon Sep 17 00:00:00 2001 From: Mmis1000 Date: Fri, 30 Sep 2022 23:12:39 +0800 Subject: [PATCH 02/11] Fix issue #7337 and #6592 --- packages/nuxt/src/app/components/layout.ts | 59 +++++++++++++++------- packages/nuxt/src/core/templates.ts | 3 +- packages/nuxt/src/pages/runtime/page.ts | 30 ++++------- 3 files changed, 51 insertions(+), 41 deletions(-) diff --git a/packages/nuxt/src/app/components/layout.ts b/packages/nuxt/src/app/components/layout.ts index bf6333d3d02..81c6d555a53 100644 --- a/packages/nuxt/src/app/components/layout.ts +++ b/packages/nuxt/src/app/components/layout.ts @@ -1,11 +1,41 @@ -import { defineComponent, unref, nextTick, onMounted, Ref, Transition, VNode } from 'vue' +import { computed, defineComponent, h, isRef, nextTick, onMounted, Ref, Transition, VNode } from 'vue' import { _wrapIf } from './utils' -import { useRoute } from '#app' +import { useRouter } 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: { @@ -14,7 +44,10 @@ export default defineComponent({ } }, setup (props, context) { - const route = useRoute() + // Use router.currentRoute.value instead because this must be changed synchronized with route + const router = useRouter() + const route = computed(() => router.currentRoute.value) + const layout = computed(() => (isRef(props.name) ? props.name.value : props.name) ?? route.value.meta.layout as string ?? 'default') let vnode: VNode let _layout: string | false @@ -29,26 +62,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 + const transitionProps = route.value.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() } } diff --git a/packages/nuxt/src/core/templates.ts b/packages/nuxt/src/core/templates.ts index 3dbf745dee4..62a6d01a876 100644 --- a/packages/nuxt/src/core/templates.ts +++ b/packages/nuxt/src/core/templates.ts @@ -154,10 +154,9 @@ export const layoutTemplate: NuxtTemplate = { 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') } diff --git a/packages/nuxt/src/pages/runtime/page.ts b/packages/nuxt/src/pages/runtime/page.ts index c2558a77213..0756c9f4366 100644 --- a/packages/nuxt/src/pages/runtime/page.ts +++ b/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,23 +43,16 @@ export default defineComponent({ const key = generateRouteKey(props.pageKey, routeProps) const transitionProps = props.transition ?? routeProps.route.meta.pageTransition ?? (defaultPageTransition as TransitionProps) - let done: () => {} - - if (!isNested) { - done = nuxtApp.deferHydration() - } + const done: () => {} = nuxtApp.deferHydration() return _wrapIf(Transition, transitionProps, - wrapInKeepAlive(props.keepalive ?? routeProps.route.meta.keepalive ?? (defaultKeepaliveConfig as KeepAliveProps), isNested - // 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: () => { - done?.() - 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: () => { + done?.() + nuxtApp.callHook('page:finish', routeProps.Component) + } + }, { default: () => h(Component, { key, routeProps, pageKey: key, hasTransition: !!transitionProps } as {}) }) )).default() } }) From 92c2ccf73a775e75efe8c55be4882737f183586b Mon Sep 17 00:00:00 2001 From: Mmis1000 Date: Sat, 1 Oct 2022 01:17:03 +0800 Subject: [PATCH 03/11] Add proper test for issue #7337 ans #6592 --- test/basic.test.ts | 100 ++++++++++++++++++ test/fixtures/basic/layouts/custom-async.vue | 11 ++ test/fixtures/basic/layouts/custom2.vue | 6 ++ test/fixtures/basic/pages/async-parent.vue | 14 +++ .../basic/pages/async-parent/child.vue | 13 +++ .../basic/pages/keyed-child-parent.vue | 6 ++ .../basic/pages/keyed-child-parent/[foo].vue | 17 +++ test/fixtures/basic/pages/with-layout.vue | 3 + test/fixtures/basic/pages/with-layout2.vue | 13 +++ 9 files changed, 183 insertions(+) create mode 100644 test/fixtures/basic/layouts/custom-async.vue create mode 100644 test/fixtures/basic/layouts/custom2.vue create mode 100644 test/fixtures/basic/pages/async-parent.vue create mode 100644 test/fixtures/basic/pages/async-parent/child.vue create mode 100644 test/fixtures/basic/pages/keyed-child-parent.vue create mode 100644 test/fixtures/basic/pages/keyed-child-parent/[foo].vue create mode 100644 test/fixtures/basic/pages/with-layout2.vue diff --git a/test/basic.test.ts b/test/basic.test.ts index 2fe9c019dba..628986a637d 100644 --- a/test/basic.test.ts +++ b/test/basic.test.ts @@ -409,6 +409,106 @@ describe('extends support', () => { }) }) +// Bug #7337 +describe('deferred app suspense resolve', () => { + it('should wait for all suspense instance on initial hydration', async () => { + let done = false + const page = await createPage() + const logs: string[] = [] + page.on('console', (msg) => { + const text = msg.text() + if (text.includes('isHydrating')) { + if (done) { + throw new Error('Test finished prematurely') + } + logs.push(text) + } + }) + + try { + await page.goto(url('/async-parent/child')) + 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))) + + expect(logs.length).toBe(3) + expect(logs.every(log => log === 'isHydrating: true')) + } finally { + done = true + } + }) +}) + +// Bug #6592 +describe('page key', () => { + it('should not cause run of setup if navigation not change page key and layout', async () => { + let done = false + const page = await createPage() + const logs: string[] = [] + + page.on('console', (msg) => { + const text = msg.text() + if (text.includes('Running Child Setup')) { + if (done) { + throw new Error('Test finished prematurely') + } + logs.push(text) + } + }) + + try { + await page.goto(url('/keyed-child-parent/0')) + await page.waitForLoadState('networkidle') + + await page.click('[href="/keyed-child-parent/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.length).toBe(1) + } finally { + done = true + } + }) +}) + +// Bug #6592 +describe('layout change not load page twice', () => { + it('should not cause run of page setup to repeat if layout changed', async () => { + let done = false + const page = await createPage() + const logs: string[] = [] + + page.on('console', (msg) => { + const text = msg.text() + if (text.includes('Running With Layout2 Page Setup')) { + if (done) { + throw new Error('Test finished prematurely') + } + logs.push(text) + } + }) + + try { + await page.goto(url('/with-layout')) + await page.waitForLoadState('networkidle') + await page.click('[href="/with-layout2"]') + 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.length).toBe(1) + } finally { + done = true + } + }) +}) + describe('automatically keyed composables', () => { it('should automatically generate keys', async () => { const html = await $fetch('/keyed-composables') diff --git a/test/fixtures/basic/layouts/custom-async.vue b/test/fixtures/basic/layouts/custom-async.vue new file mode 100644 index 00000000000..92d244a9aa1 --- /dev/null +++ b/test/fixtures/basic/layouts/custom-async.vue @@ -0,0 +1,11 @@ + + + diff --git a/test/fixtures/basic/layouts/custom2.vue b/test/fixtures/basic/layouts/custom2.vue new file mode 100644 index 00000000000..35236542c81 --- /dev/null +++ b/test/fixtures/basic/layouts/custom2.vue @@ -0,0 +1,6 @@ + diff --git a/test/fixtures/basic/pages/async-parent.vue b/test/fixtures/basic/pages/async-parent.vue new file mode 100644 index 00000000000..7eda70a494f --- /dev/null +++ b/test/fixtures/basic/pages/async-parent.vue @@ -0,0 +1,14 @@ + + + diff --git a/test/fixtures/basic/pages/async-parent/child.vue b/test/fixtures/basic/pages/async-parent/child.vue new file mode 100644 index 00000000000..c0b52bb9dfb --- /dev/null +++ b/test/fixtures/basic/pages/async-parent/child.vue @@ -0,0 +1,13 @@ + + + diff --git a/test/fixtures/basic/pages/keyed-child-parent.vue b/test/fixtures/basic/pages/keyed-child-parent.vue new file mode 100644 index 00000000000..1151f3b3231 --- /dev/null +++ b/test/fixtures/basic/pages/keyed-child-parent.vue @@ -0,0 +1,6 @@ + diff --git a/test/fixtures/basic/pages/keyed-child-parent/[foo].vue b/test/fixtures/basic/pages/keyed-child-parent/[foo].vue new file mode 100644 index 00000000000..8f4ed5ac946 --- /dev/null +++ b/test/fixtures/basic/pages/keyed-child-parent/[foo].vue @@ -0,0 +1,17 @@ + + + diff --git a/test/fixtures/basic/pages/with-layout.vue b/test/fixtures/basic/pages/with-layout.vue index c7f73bc774d..bfe044dd865 100644 --- a/test/fixtures/basic/pages/with-layout.vue +++ b/test/fixtures/basic/pages/with-layout.vue @@ -7,5 +7,8 @@ definePageMeta({ diff --git a/test/fixtures/basic/pages/with-layout2.vue b/test/fixtures/basic/pages/with-layout2.vue new file mode 100644 index 00000000000..b256deb3cae --- /dev/null +++ b/test/fixtures/basic/pages/with-layout2.vue @@ -0,0 +1,13 @@ + + + From 28d99ee9acfe6e013225b30ba16b894649775d2a Mon Sep 17 00:00:00 2001 From: Mmis1000 Date: Sat, 1 Oct 2022 01:35:04 +0800 Subject: [PATCH 04/11] Also add test to ensure page key do work --- test/basic.test.ts | 33 ++++++++++++++++++- .../basic/pages/fixed-keyed-child-parent.vue | 6 ++++ .../pages/fixed-keyed-child-parent/[foo].vue | 17 ++++++++++ .../basic/pages/keyed-child-parent/[foo].vue | 2 +- 4 files changed, 56 insertions(+), 2 deletions(-) create mode 100644 test/fixtures/basic/pages/fixed-keyed-child-parent.vue create mode 100644 test/fixtures/basic/pages/fixed-keyed-child-parent/[foo].vue diff --git a/test/basic.test.ts b/test/basic.test.ts index 628986a637d..03674468338 100644 --- a/test/basic.test.ts +++ b/test/basic.test.ts @@ -457,6 +457,37 @@ describe('page key', () => { } }) + try { + await page.goto(url('/fixed-keyed-child-parent/0')) + await page.waitForLoadState('networkidle') + + await page.click('[href="/fixed-keyed-child-parent/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.length).toBe(1) + } finally { + done = true + } + }) + it('will cause run of setup if navigation changed page key', async () => { + let done = false + const page = await createPage() + const logs: string[] = [] + + page.on('console', (msg) => { + const text = msg.text() + if (text.includes('Running Child Setup')) { + if (done) { + throw new Error('Test finished prematurely') + } + logs.push(text) + } + }) + try { await page.goto(url('/keyed-child-parent/0')) await page.waitForLoadState('networkidle') @@ -468,7 +499,7 @@ describe('page key', () => { // so we are not resolved too early when there are repeated page loading await page.evaluate(() => new Promise(resolve => setTimeout(resolve, 0))) - expect(logs.length).toBe(1) + expect(logs.length).toBe(2) } finally { done = true } diff --git a/test/fixtures/basic/pages/fixed-keyed-child-parent.vue b/test/fixtures/basic/pages/fixed-keyed-child-parent.vue new file mode 100644 index 00000000000..557fe9260e3 --- /dev/null +++ b/test/fixtures/basic/pages/fixed-keyed-child-parent.vue @@ -0,0 +1,6 @@ + diff --git a/test/fixtures/basic/pages/fixed-keyed-child-parent/[foo].vue b/test/fixtures/basic/pages/fixed-keyed-child-parent/[foo].vue new file mode 100644 index 00000000000..90bfa13cab7 --- /dev/null +++ b/test/fixtures/basic/pages/fixed-keyed-child-parent/[foo].vue @@ -0,0 +1,17 @@ + + + diff --git a/test/fixtures/basic/pages/keyed-child-parent/[foo].vue b/test/fixtures/basic/pages/keyed-child-parent/[foo].vue index 8f4ed5ac946..d847b5ddee6 100644 --- a/test/fixtures/basic/pages/keyed-child-parent/[foo].vue +++ b/test/fixtures/basic/pages/keyed-child-parent/[foo].vue @@ -12,6 +12,6 @@ console.log('Running Child Setup') const route = useRoute() definePageMeta({ - key: 'keyed' + key: r => 'keyed-' + r.params.foo }) From 25e22fe614ed963ad70adebba2c24cf56f688d6e Mon Sep 17 00:00:00 2001 From: Daniel Roe Date: Sat, 1 Oct 2022 13:45:51 +0100 Subject: [PATCH 05/11] fix: re-implement #7818 --- packages/nuxt/src/app/components/layout.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/packages/nuxt/src/app/components/layout.ts b/packages/nuxt/src/app/components/layout.ts index 81c6d555a53..e961039023a 100644 --- a/packages/nuxt/src/app/components/layout.ts +++ b/packages/nuxt/src/app/components/layout.ts @@ -1,4 +1,4 @@ -import { computed, defineComponent, h, isRef, nextTick, onMounted, Ref, Transition, VNode } from 'vue' +import { computed, defineComponent, h, nextTick, onMounted, Ref, Transition, unref, VNode } from 'vue' import { _wrapIf } from './utils' import { useRouter } from '#app' // @ts-ignore @@ -47,7 +47,7 @@ export default defineComponent({ // Use router.currentRoute.value instead because this must be changed synchronized with route const router = useRouter() const route = computed(() => router.currentRoute.value) - const layout = computed(() => (isRef(props.name) ? props.name.value : props.name) ?? route.value.meta.layout as string ?? 'default') + const layout = computed(() => unref(props.name) ?? route.value.meta.layout as string ?? 'default') let vnode: VNode let _layout: string | false From 21d872ca3f1b51da7fa2787551db2d85acce24e6 Mon Sep 17 00:00:00 2001 From: Daniel Roe Date: Sat, 1 Oct 2022 14:14:56 +0100 Subject: [PATCH 06/11] fix: ensure we use route fork if used within page --- packages/nuxt/src/app/components/layout.ts | 15 ++++++++------- 1 file changed, 8 insertions(+), 7 deletions(-) diff --git a/packages/nuxt/src/app/components/layout.ts b/packages/nuxt/src/app/components/layout.ts index e961039023a..9356fb241f6 100644 --- a/packages/nuxt/src/app/components/layout.ts +++ b/packages/nuxt/src/app/components/layout.ts @@ -1,6 +1,7 @@ -import { computed, defineComponent, h, nextTick, onMounted, Ref, Transition, unref, VNode } from 'vue' +import { computed, defineComponent, h, inject, nextTick, onMounted, Ref, Transition, unref, VNode } from 'vue' import { _wrapIf } from './utils' -import { useRouter } from '#app' +import { RouteLocationNormalizedLoaded, useRoute as useVueRouterRoute } from 'vue-router' +import { useRoute } from '#app' // @ts-ignore import layouts from '#build/layouts' // @ts-ignore @@ -44,10 +45,10 @@ export default defineComponent({ } }, setup (props, context) { - // Use router.currentRoute.value instead because this must be changed synchronized with route - const router = useRouter() - const route = computed(() => router.currentRoute.value) - const layout = computed(() => unref(props.name) ?? route.value.meta.layout as string ?? 'default') + // Need to ensure (if we are not a child of ``) 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 @@ -67,7 +68,7 @@ export default defineComponent({ console.warn(`Invalid layout \`${layout.value}\` selected.`) } - const transitionProps = route.value.meta.layoutTransition ?? defaultLayoutTransition + const transitionProps = route.meta.layoutTransition ?? defaultLayoutTransition // We avoid rendering layout transition if there is no layout to render return _wrapIf(Transition, hasLayout && transitionProps, { From 1c9dd5fdb1eb8184b3b4702f5a7e3e8fd40f342d Mon Sep 17 00:00:00 2001 From: Daniel Roe Date: Sat, 1 Oct 2022 14:22:04 +0100 Subject: [PATCH 07/11] feat: add types and ensure order of hooks --- .../nuxt/src/app/components/nuxt-root.vue | 6 +-- packages/nuxt/src/app/nuxt.ts | 39 +++++++++---------- packages/nuxt/src/pages/runtime/page.ts | 7 +--- 3 files changed, 22 insertions(+), 30 deletions(-) diff --git a/packages/nuxt/src/app/components/nuxt-root.vue b/packages/nuxt/src/app/components/nuxt-root.vue index 695931936bf..5fb19ec758b 100644 --- a/packages/nuxt/src/app/components/nuxt-root.vue +++ b/packages/nuxt/src/app/components/nuxt-root.vue @@ -12,11 +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 done = nuxtApp.deferHydration() - -const onResolve = () => { - done() -} +const onResolve = nuxtApp.deferHydration() // Inject default route (outside of pages) as active route provide('_route', useRoute()) diff --git a/packages/nuxt/src/app/nuxt.ts b/packages/nuxt/src/app/nuxt.ts index f14b817a6b3..da86fdd4706 100644 --- a/packages/nuxt/src/app/nuxt.ts +++ b/packages/nuxt/src/app/nuxt.ts @@ -70,7 +70,10 @@ interface _NuxtApp { data: Ref pending: Ref error: Ref - } | undefined>, + } | undefined> + + isHydrating?: boolean + deferHydration: () => () => void | Promise ssrContext?: NuxtSSRContext payload: { @@ -108,7 +111,7 @@ export interface CreateOptions { } export function createNuxtApp (options: CreateOptions) { - let hydratingCount: number = 0 + let hydratingCount = 0 const nuxtApp: NuxtApp = { provide: undefined, globalName: 'nuxt', @@ -120,25 +123,21 @@ export function createNuxtApp (options: CreateOptions) { }), isHydrating: process.client, deferHydration () { - if (nuxtApp.isHydrating) { - hydratingCount++ - let called = false - return () => { - if (called) { - return - } else { - called = true - } - - hydratingCount-- - - if (hydratingCount === 0) { - nuxtApp.isHydrating = false - nuxtApp.callHook('app:suspense:resolve') - } + 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') } - } else { - return () => {} } }, _asyncDataPromises: {}, diff --git a/packages/nuxt/src/pages/runtime/page.ts b/packages/nuxt/src/pages/runtime/page.ts index 0756c9f4366..8ed936f3cdb 100644 --- a/packages/nuxt/src/pages/runtime/page.ts +++ b/packages/nuxt/src/pages/runtime/page.ts @@ -43,15 +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() + const done = nuxtApp.deferHydration() return _wrapIf(Transition, transitionProps, wrapInKeepAlive(props.keepalive ?? routeProps.route.meta.keepalive ?? (defaultKeepaliveConfig as KeepAliveProps), h(Suspense, { onPending: () => nuxtApp.callHook('page:start', routeProps.Component), - onResolve: () => { - done?.() - 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() } From ac8b0a87b3969627e81a6bbafc04110de3cfa74e Mon Sep 17 00:00:00 2001 From: Daniel Roe Date: Sat, 1 Oct 2022 14:29:29 +0100 Subject: [PATCH 08/11] style: import order --- packages/nuxt/src/app/components/layout.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/nuxt/src/app/components/layout.ts b/packages/nuxt/src/app/components/layout.ts index 9356fb241f6..ae82998ddfa 100644 --- a/packages/nuxt/src/app/components/layout.ts +++ b/packages/nuxt/src/app/components/layout.ts @@ -1,6 +1,6 @@ import { computed, defineComponent, h, inject, nextTick, onMounted, Ref, Transition, unref, VNode } from 'vue' -import { _wrapIf } from './utils' import { RouteLocationNormalizedLoaded, useRoute as useVueRouterRoute } from 'vue-router' +import { _wrapIf } from './utils' import { useRoute } from '#app' // @ts-ignore import layouts from '#build/layouts' From 99b0cb43cc8fc16cc3d6a1d83a2e2d8994db39ca Mon Sep 17 00:00:00 2001 From: Daniel Roe Date: Sat, 8 Oct 2022 14:09:16 +0100 Subject: [PATCH 09/11] test: refactor to add `withLogs` test util and add internal layout test suite --- test/basic.test.ts | 143 ++++++------------ test/fixtures/basic/nuxt.config.ts | 26 ++++ .../pages/fixed-keyed-child-parent/[foo].vue | 2 +- test/fixtures/basic/pages/internal-layout.vue | 15 ++ .../basic/pages/keyed-child-parent/[foo].vue | 2 +- test/fixtures/basic/pages/with-layout.vue | 2 +- test/utils.ts | 24 ++- 7 files changed, 116 insertions(+), 98 deletions(-) create mode 100644 test/fixtures/basic/pages/internal-layout.vue diff --git a/test/basic.test.ts b/test/basic.test.ts index 03674468338..f106b504c88 100644 --- a/test/basic.test.ts +++ b/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)), @@ -411,132 +411,89 @@ describe('extends support', () => { // Bug #7337 describe('deferred app suspense resolve', () => { - it('should wait for all suspense instance on initial hydration', async () => { - let done = false - const page = await createPage() - const logs: string[] = [] - page.on('console', (msg) => { - const text = msg.text() - if (text.includes('isHydrating')) { - if (done) { - throw new Error('Test finished prematurely') - } - logs.push(text) - } - }) + async function behaviour (path: string) { - try { - await page.goto(url('/async-parent/child')) + 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))) - expect(logs.length).toBe(3) - expect(logs.every(log => log === 'isHydrating: true')) - } finally { - done = true - } + 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 () => { - let done = false - const page = await createPage() - const logs: string[] = [] - - page.on('console', (msg) => { - const text = msg.text() - if (text.includes('Running Child Setup')) { - if (done) { - throw new Error('Test finished prematurely') - } - logs.push(text) - } - }) - - try { - await page.goto(url('/fixed-keyed-child-parent/0')) - await page.waitForLoadState('networkidle') + async function behaviour (path: string) { + await withLogs(async (page, logs) => { + await page.goto(url(`${path}/0`)) + await page.waitForLoadState('networkidle') - await page.click('[href="/fixed-keyed-child-parent/1"]') - await page.waitForSelector('#page-1') + 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))) + // 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.length).toBe(1) - } finally { - done = true + 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 () => { - let done = false - const page = await createPage() - const logs: string[] = [] - - page.on('console', (msg) => { - const text = msg.text() - if (text.includes('Running Child Setup')) { - if (done) { - throw new Error('Test finished prematurely') - } - logs.push(text) - } - }) - - try { - await page.goto(url('/keyed-child-parent/0')) - await page.waitForLoadState('networkidle') + async function behaviour (path: string) { + await withLogs(async (page, logs) => { + await page.goto(url(`${path}/0`)) + await page.waitForLoadState('networkidle') - await page.click('[href="/keyed-child-parent/1"]') - await page.waitForSelector('#page-1') + 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))) + // 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.length).toBe(2) - } finally { - done = true + 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', () => { - it('should not cause run of page setup to repeat if layout changed', async () => { - let done = false - const page = await createPage() - const logs: string[] = [] - - page.on('console', (msg) => { - const text = msg.text() - if (text.includes('Running With Layout2 Page Setup')) { - if (done) { - throw new Error('Test finished prematurely') - } - logs.push(text) - } - }) - - try { - await page.goto(url('/with-layout')) + 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="/with-layout2"]') + 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.length).toBe(1) - } finally { - done = true - } + 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') }) }) diff --git a/test/fixtures/basic/nuxt.config.ts b/test/fixtures/basic/nuxt.config.ts index f95ce54fbd0..78a508c610d 100644 --- a/test/fixtures/basic/nuxt.config.ts +++ b/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: { diff --git a/test/fixtures/basic/pages/fixed-keyed-child-parent/[foo].vue b/test/fixtures/basic/pages/fixed-keyed-child-parent/[foo].vue index 90bfa13cab7..cdf89a95678 100644 --- a/test/fixtures/basic/pages/fixed-keyed-child-parent/[foo].vue +++ b/test/fixtures/basic/pages/fixed-keyed-child-parent/[foo].vue @@ -1,7 +1,7 @@