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

Commit

Permalink
feat(nuxt): add warning in dev mode if layouts/pages do not have a si…
Browse files Browse the repository at this point in the history
…ngle root node (#5469)
  • Loading branch information
danielroe authored Aug 23, 2022
1 parent 2802638 commit cfb7e59
Show file tree
Hide file tree
Showing 9 changed files with 112 additions and 12 deletions.
30 changes: 26 additions & 4 deletions packages/nuxt/src/app/components/layout.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import { defineComponent, isRef, Ref, Transition } from 'vue'
import { defineComponent, isRef, nextTick, onMounted, Ref, Transition, VNode } from 'vue'
import { _wrapIf } from './utils'
import { useRoute } from '#app'
// @ts-ignore
Expand All @@ -16,6 +16,18 @@ export default defineComponent({
setup (props, context) {
const route = useRoute()

let vnode: VNode
let _layout: string | false
if (process.dev && process.client) {
onMounted(() => {
nextTick(() => {
if (_layout && ['#comment', '#text'].includes(vnode?.el?.nodeName)) {
console.warn(`[nuxt] \`${_layout}\` layout does not have a single root node and will cause errors when navigating between routes.`)
}
})
})
}

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

Expand All @@ -24,10 +36,20 @@ export default defineComponent({
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 && (route.meta.layoutTransition ?? defaultLayoutTransition),
_wrapIf(layouts[layout], hasLayout, context.slots)
).default()
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()
}
}
})
37 changes: 30 additions & 7 deletions packages/nuxt/src/pages/runtime/page.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,7 @@
import { computed, DefineComponent, defineComponent, h, inject, provide, reactive, Suspense, Transition } from 'vue'
import { RouteLocation, RouteLocationNormalized, RouteLocationNormalizedLoaded, RouterView } from 'vue-router'
import { computed, defineComponent, h, inject, provide, reactive, onMounted, nextTick, Suspense, Transition } from 'vue'
import type { DefineComponent, VNode } from 'vue'
import { RouteLocationNormalized, RouteLocationNormalizedLoaded, RouterView } from 'vue-router'
import type { RouteLocation } from 'vue-router'

import { generateRouteKey, RouterViewSlotProps, wrapInKeepAlive } from './utils'
import { useNuxtApp } from '#app'
Expand Down Expand Up @@ -34,15 +36,16 @@ export default defineComponent({
if (!routeProps.Component) { return }

const key = generateRouteKey(props.pageKey, routeProps)
const transitionProps = routeProps.route.meta.pageTransition ?? defaultPageTransition

return _wrapIf(Transition, routeProps.route.meta.pageTransition ?? defaultPageTransition,
return _wrapIf(Transition, transitionProps,
wrapInKeepAlive(routeProps.route.meta.keepalive, isNested && nuxtApp.isHydrating
// Include route children in parent suspense
? h(Component, { key, routeProps, pageKey: key } as {})
? 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 } as {}) })
}, { default: () => h(Component, { key, routeProps, pageKey: key, hasTransition: !!transitionProps } as {}) })
)).default()
}
})
Expand All @@ -60,7 +63,7 @@ const defaultPageTransition = { name: 'page', mode: 'out-in' }
const Component = defineComponent({
// TODO: Type props
// eslint-disable-next-line vue/require-prop-types
props: ['routeProps', 'pageKey'],
props: ['routeProps', 'pageKey', 'hasTransition'],
setup (props) {
// Prevent reactivity when the page will be rerendered in a different suspense fork
const previousKey = props.pageKey
Expand All @@ -73,6 +76,26 @@ const Component = defineComponent({
}

provide('_route', reactive(route))
return () => h(props.routeProps.Component)

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.`)
}
})
})
}

return () => {
if (process.dev && process.client) {
vnode = h(props.routeProps.Component)
return vnode
}

return h(props.routeProps.Component)
}
}
})
25 changes: 24 additions & 1 deletion test/basic.test.ts
Original file line number Diff line number Diff line change
@@ -1,8 +1,9 @@
import { fileURLToPath } from 'node:url'
import { describe, expect, it } from 'vitest'
import { joinURL } from 'ufo'
// import { isWindows } from 'std-env'
import { setup, fetch, $fetch, startServer } from '@nuxt/test-utils'
import { expectNoClientErrors } from './utils'
import { expectNoClientErrors, renderPage } from './utils'

await setup({
rootDir: fileURLToPath(new URL('./fixtures/basic', import.meta.url)),
Expand Down Expand Up @@ -359,6 +360,28 @@ describe('automatically keyed composables', () => {
})
})

if (process.env.NUXT_TEST_DEV) {
describe('detecting invalid root nodes', () => {
it('should detect invalid root nodes in pages', async () => {
for (const path of ['1', '2', '3', '4']) {
const { consoleLogs } = await renderPage(joinURL('/invalid-root', path))
const consoleLogsWarns = consoleLogs.filter(i => i.type === 'warning').map(w => w.text).join('\n')
expect(consoleLogsWarns).toContain('does not have a single root node and will cause errors when navigating between routes')
}
})

it('should not complain if there is no transition', async () => {
for (const path of ['fine']) {
const { consoleLogs } = await renderPage(joinURL('/invalid-root', path))

const consoleLogsWarns = consoleLogs.filter(i => i.type === 'warning')

expect(consoleLogsWarns.length).toEqual(0)
}
})
})
}

describe('dynamic paths', () => {
if (process.env.NUXT_TEST_DEV) {
// TODO:
Expand Down
4 changes: 4 additions & 0 deletions test/fixtures/basic/layouts/invalid-root.vue
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
<template>
<div />
<slot />
</template>
4 changes: 4 additions & 0 deletions test/fixtures/basic/pages/invalid-root/1.vue
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
<template>
<!-- comment -->
<div>Test</div>
</template>
3 changes: 3 additions & 0 deletions test/fixtures/basic/pages/invalid-root/2.vue
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
<template>
Just some text
</template>
4 changes: 4 additions & 0 deletions test/fixtures/basic/pages/invalid-root/3.vue
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
<template>
<div>Multiple</div>
<div>elements</div>
</template>
7 changes: 7 additions & 0 deletions test/fixtures/basic/pages/invalid-root/4.vue
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
<template>
<div>Fine</div>
</template>

<script setup>
definePageMeta({ layout: 'invalid-root' })
</script>
10 changes: 10 additions & 0 deletions test/fixtures/basic/pages/invalid-root/fine.vue
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
<template>
<div>Multiple</div>
<div>elements</div>
</template>

<script setup>
definePageMeta({
pageTransition: false
})
</script>

0 comments on commit cfb7e59

Please sign in to comment.