From 220cc502a14f191445d62c4719120c7844c4c3f9 Mon Sep 17 00:00:00 2001 From: Daniel Roe Date: Thu, 13 Jun 2024 17:59:24 +0100 Subject: [PATCH] fix(nuxt): preserve route metadata assigned outside page (#27587) --- packages/nuxt/src/pages/utils.ts | 121 +++++++------- .../pages-override-meta-enabled.test.ts.snap | 2 +- packages/nuxt/test/page-metadata.test.ts | 148 ++++++++++++++++++ test/basic.test.ts | 15 ++ test/fixtures/basic/nuxt.config.ts | 11 ++ test/fixtures/basic/pages/meta.vue | 36 +++++ 6 files changed, 274 insertions(+), 59 deletions(-) create mode 100644 packages/nuxt/test/page-metadata.test.ts create mode 100644 test/fixtures/basic/pages/meta.vue diff --git a/packages/nuxt/src/pages/utils.ts b/packages/nuxt/src/pages/utils.ts index 63bb9cacf9..1e713075dd 100644 --- a/packages/nuxt/src/pages/utils.ts +++ b/packages/nuxt/src/pages/utils.ts @@ -9,6 +9,7 @@ import { filename } from 'pathe/utils' import { hash } from 'ohash' import { transform } from 'esbuild' import { parse } from 'acorn' +import { walk } from 'estree-walker' import type { CallExpression, ExpressionStatement, ObjectExpression, Program, Property } from 'estree' import type { NuxtPage } from 'nuxt/schema' @@ -173,7 +174,7 @@ const DYNAMIC_META_KEY = '__nuxt_dynamic_meta_key' as const const pageContentsCache: Record = {} const metaCache: Record>> = {} -async function getRouteMeta (contents: string, absolutePath: string): Promise>> { +export async function getRouteMeta (contents: string, absolutePath: string): Promise>> { // set/update pageContentsCache, invalidate metaCache on cache mismatch if (!(absolutePath in pageContentsCache) || pageContentsCache[absolutePath] !== contents) { pageContentsCache[absolutePath] = contents @@ -199,70 +200,77 @@ async function getRouteMeta (contents: string, absolutePath: string): Promise node.type === 'ExpressionStatement' && node.expression.type === 'CallExpression' && node.expression.callee.type === 'Identifier' && node.expression.callee.name === 'definePageMeta') - if (!pageMetaAST) { - metaCache[absolutePath] = {} - return {} - } - const pageMetaArgument = ((pageMetaAST as ExpressionStatement).expression as CallExpression).arguments[0] as ObjectExpression const extractedMeta = {} as Partial> const extractionKeys = ['name', 'path', 'alias', 'redirect'] as const const dynamicProperties = new Set() - for (const key of extractionKeys) { - const property = pageMetaArgument.properties.find(property => property.type === 'Property' && property.key.type === 'Identifier' && property.key.name === key) as Property - if (!property) { continue } + let foundMeta = false - if (property.value.type === 'ObjectExpression') { - const valueString = js.code.slice(property.value.range![0], property.value.range![1]) - try { - extractedMeta[key] = JSON.parse(runInNewContext(`JSON.stringify(${valueString})`, {})) - } catch { - console.debug(`[nuxt] Skipping extraction of \`${key}\` metadata as it is not JSON-serializable (reading \`${absolutePath}\`).`) - dynamicProperties.add(key) - continue - } - } + walk(ast, { + enter (node) { + if (foundMeta) { return } - if (property.value.type === 'ArrayExpression') { - const values = [] - for (const element of property.value.elements) { - if (!element) { + if (node.type !== 'ExpressionStatement' || node.expression.type !== 'CallExpression' || node.expression.callee.type !== 'Identifier' || node.expression.callee.name !== 'definePageMeta') { return } + + foundMeta = true + const pageMetaArgument = ((node as ExpressionStatement).expression as CallExpression).arguments[0] as ObjectExpression + + for (const key of extractionKeys) { + const property = pageMetaArgument.properties.find(property => property.type === 'Property' && property.key.type === 'Identifier' && property.key.name === key) as Property + if (!property) { continue } + + if (property.value.type === 'ObjectExpression') { + const valueString = js.code.slice(property.value.range![0], property.value.range![1]) + try { + extractedMeta[key] = JSON.parse(runInNewContext(`JSON.stringify(${valueString})`, {})) + } catch { + console.debug(`[nuxt] Skipping extraction of \`${key}\` metadata as it is not JSON-serializable (reading \`${absolutePath}\`).`) + dynamicProperties.add(key) + continue + } + } + + if (property.value.type === 'ArrayExpression') { + const values = [] + for (const element of property.value.elements) { + if (!element) { + continue + } + if (element.type !== 'Literal' || typeof element.value !== 'string') { + console.debug(`[nuxt] Skipping extraction of \`${key}\` metadata as it is not an array of string literals (reading \`${absolutePath}\`).`) + dynamicProperties.add(key) + continue + } + values.push(element.value) + } + extractedMeta[key] = values continue } - if (element.type !== 'Literal' || typeof element.value !== 'string') { - console.debug(`[nuxt] Skipping extraction of \`${key}\` metadata as it is not an array of string literals (reading \`${absolutePath}\`).`) + + if (property.value.type !== 'Literal' || typeof property.value.value !== 'string') { + console.debug(`[nuxt] Skipping extraction of \`${key}\` metadata as it is not a string literal or array of string literals (reading \`${absolutePath}\`).`) dynamicProperties.add(key) continue } - values.push(element.value) + extractedMeta[key] = property.value.value } - extractedMeta[key] = values - continue - } - if (property.value.type !== 'Literal' || typeof property.value.value !== 'string') { - console.debug(`[nuxt] Skipping extraction of \`${key}\` metadata as it is not a string literal or array of string literals (reading \`${absolutePath}\`).`) - dynamicProperties.add(key) - continue - } - extractedMeta[key] = property.value.value - } + const extraneousMetaKeys = pageMetaArgument.properties + .filter(property => property.type === 'Property' && property.key.type === 'Identifier' && !(extractionKeys as unknown as string[]).includes(property.key.name)) + // @ts-expect-error inferred types have been filtered out + .map(property => property.key.name) - const extraneousMetaKeys = pageMetaArgument.properties - .filter(property => property.type === 'Property' && property.key.type === 'Identifier' && !(extractionKeys as unknown as string[]).includes(property.key.name)) - // @ts-expect-error inferred types have been filtered out - .map(property => property.key.name) + if (extraneousMetaKeys.length) { + dynamicProperties.add('meta') + } - if (extraneousMetaKeys.length) { - dynamicProperties.add('meta') - } - - if (dynamicProperties.size) { - extractedMeta.meta ??= {} - extractedMeta.meta[DYNAMIC_META_KEY] = dynamicProperties - } + if (dynamicProperties.size) { + extractedMeta.meta ??= {} + extractedMeta.meta[DYNAMIC_META_KEY] = dynamicProperties + } + }, + }) metaCache[absolutePath] = extractedMeta return extractedMeta @@ -505,19 +513,20 @@ async function createClientPage(loader) { }`) } - if (route.children != null) { + if (route.children) { metaRoute.children = route.children } - if (overrideMeta) { - metaRoute.name = `${metaImportName}?.name` - metaRoute.path = `${metaImportName}?.path ?? ''` + if (route.meta) { + metaRoute.meta = `{ ...(${metaImportName} || {}), ...${route.meta} }` + } + if (overrideMeta) { // skip and retain fallback if marked dynamic // set to extracted value or fallback if none extracted for (const key of ['name', 'path'] satisfies NormalizedRouteKeys) { if (markedDynamic.has(key)) { continue } - metaRoute[key] = route[key] ?? metaRoute[key] + metaRoute[key] = route[key] ?? `${metaImportName}?.${key}` } // set to extracted value or delete if none extracted @@ -532,10 +541,6 @@ async function createClientPage(loader) { metaRoute[key] = route[key] } } else { - if (route.meta != null) { - metaRoute.meta = `{ ...(${metaImportName} || {}), ...${route.meta} }` - } - if (route.alias != null) { metaRoute.alias = `${route.alias}.concat(${metaImportName}?.alias || [])` } diff --git a/packages/nuxt/test/__snapshots__/pages-override-meta-enabled.test.ts.snap b/packages/nuxt/test/__snapshots__/pages-override-meta-enabled.test.ts.snap index 88989345d1..9fb308e7c1 100644 --- a/packages/nuxt/test/__snapshots__/pages-override-meta-enabled.test.ts.snap +++ b/packages/nuxt/test/__snapshots__/pages-override-meta-enabled.test.ts.snap @@ -303,7 +303,7 @@ "alias": "mockMeta?.alias || []", "component": "() => import("pages/index.vue").then(m => m.default || m)", "meta": "mockMeta || {}", - "name": "mockMeta?.name", + "name": "mockMeta?.name ?? "index"", "path": ""/"", "redirect": "mockMeta?.redirect", }, diff --git a/packages/nuxt/test/page-metadata.test.ts b/packages/nuxt/test/page-metadata.test.ts new file mode 100644 index 0000000000..4431b80f2f --- /dev/null +++ b/packages/nuxt/test/page-metadata.test.ts @@ -0,0 +1,148 @@ +import { describe, expect, it } from 'vitest' +import { getRouteMeta, normalizeRoutes } from '../src/pages/utils' +import type { NuxtPage } from '../schema' + +const filePath = '/app/pages/index.vue' + +describe('page metadata', () => { + it('should not extract metadata from empty files', async () => { + expect(await getRouteMeta('', filePath)).toEqual({}) + expect(await getRouteMeta('', filePath)).toEqual({}) + }) + + it('should use and invalidate cache', async () => { + const fileContents = `` + const meta = await getRouteMeta(fileContents, filePath) + expect(meta === await getRouteMeta(fileContents, filePath)).toBeTruthy() + expect(meta === await getRouteMeta(fileContents, '/app/pages/other.vue')).toBeFalsy() + expect(meta === await getRouteMeta('' + fileContents, filePath)).toBeFalsy() + }) + + it('should extract serialisable metadata', async () => { + const meta = await getRouteMeta(` + + `, filePath) + + expect(meta).toMatchInlineSnapshot(` + { + "meta": { + "__nuxt_dynamic_meta_key": Set { + "meta", + }, + }, + "name": "some-custom-name", + "path": "/some-custom-path", + } + `) + }) + + it('should extract serialisable metadata in options api', async () => { + const meta = await getRouteMeta(` + + `, filePath) + + expect(meta).toMatchInlineSnapshot(` + { + "meta": { + "__nuxt_dynamic_meta_key": Set { + "meta", + }, + }, + "name": "some-custom-name", + "path": "/some-custom-path", + } + `) + }) +}) + +describe('normalizeRoutes', () => { + it('should produce valid route objects when used with extracted meta', async () => { + const page: NuxtPage = { path: '/', file: filePath } + Object.assign(page, await getRouteMeta(` + + `, filePath)) + + page.meta ||= {} + page.meta.layout = 'test' + page.meta.foo = 'bar' + + const { routes, imports } = normalizeRoutes([page], new Set(), true) + expect({ routes, imports }).toMatchInlineSnapshot(` + { + "imports": Set { + "import { default as indexN6pT4Un8hYMeta } from "/app/pages/index.vue?macro=true";", + }, + "routes": "[ + { + name: "some-custom-name", + path: indexN6pT4Un8hYMeta?.path ?? "/", + meta: { ...(indexN6pT4Un8hYMeta || {}), ...{"layout":"test","foo":"bar"} }, + redirect: "/", + component: () => import("/app/pages/index.vue").then(m => m.default || m) + } + ]", + } + `) + }) + + it('should produce valid route objects when used without extracted meta', () => { + const page: NuxtPage = { path: '/', file: filePath } + page.meta ||= {} + page.meta.layout = 'test' + page.meta.foo = 'bar' + + const { routes, imports } = normalizeRoutes([page], new Set()) + expect({ routes, imports }).toMatchInlineSnapshot(` + { + "imports": Set { + "import { default as indexN6pT4Un8hYMeta } from "/app/pages/index.vue?macro=true";", + }, + "routes": "[ + { + name: indexN6pT4Un8hYMeta?.name ?? undefined, + path: indexN6pT4Un8hYMeta?.path ?? "/", + meta: { ...(indexN6pT4Un8hYMeta || {}), ...{"layout":"test","foo":"bar"} }, + alias: indexN6pT4Un8hYMeta?.alias || [], + redirect: indexN6pT4Un8hYMeta?.redirect, + component: () => import("/app/pages/index.vue").then(m => m.default || m) + } + ]", + } + `) + }) +}) diff --git a/test/basic.test.ts b/test/basic.test.ts index f86faf36d1..864e48726e 100644 --- a/test/basic.test.ts +++ b/test/basic.test.ts @@ -166,6 +166,21 @@ describe('pages', () => { expect(res.headers.get('x-extend')).toEqual('added in pages:extend') }) + it('preserves page metadata added in pages:extend hook', async () => { + const html = await $fetch('/some-custom-path') + expect (html.match(/
([^<]*)<\/pre>/)?.[1]?.trim().replace(/"/g, '"').replace(/>/g, '>')).toMatchInlineSnapshot(`
+      "{
+        "name": "some-custom-name",
+        "path": "/some-custom-path",
+        "validate": "() => true",
+        "middleware": [
+          "() => true"
+        ],
+        "otherValue": "{\\"foo\\":\\"bar\\"}"
+      }"
+    `)
+  })
+
   it('validates routes', async () => {
     const { status, headers } = await fetch('/forbidden')
     expect(status).toEqual(404)
diff --git a/test/fixtures/basic/nuxt.config.ts b/test/fixtures/basic/nuxt.config.ts
index d312d55944..08f427b2a6 100644
--- a/test/fixtures/basic/nuxt.config.ts
+++ b/test/fixtures/basic/nuxt.config.ts
@@ -151,6 +151,17 @@ export default defineNuxtConfig({
         internalParent!.children = newPages
       })
     },
+    function (_options, nuxt) {
+      // to check that page metadata is preserved
+      nuxt.hook('pages:extend', (pages) => {
+        const customName = pages.find(page => page.name === 'some-custom-name')
+        if (!customName) { throw new Error('Page with custom name not found') }
+        if (customName.path !== '/some-custom-path') { throw new Error('Page path not extracted') }
+
+        customName.meta ||= {}
+        customName.meta.someProp = true
+      })
+    },
     // To test falsy module values
     undefined,
   ],
diff --git a/test/fixtures/basic/pages/meta.vue b/test/fixtures/basic/pages/meta.vue
new file mode 100644
index 0000000000..7983bde1ee
--- /dev/null
+++ b/test/fixtures/basic/pages/meta.vue
@@ -0,0 +1,36 @@
+
+
+