Zengtudor/Nuxt
1
0
Fork 0
mirror of https://github.com/nuxt/nuxt.git synced 2025-01-18 17:35:57 +00:00
Code Issues Packages Projects Releases Wiki Activity

fix(nuxt): don't hoist identifiers declared locally in definePageMeta when extracting page metadata (#30490)

Browse Source
This commit is contained in:
Matej Černý 2025-01-09 16:11:44 +01:00 committed by GitHub
parent 41cdaf6702
commit 04e02986cd
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
4 changed files with 360 additions and 23 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

82
packages/nuxt/src/core/utils/parse.ts
View File

@ -53,12 +53,30 @@ export function withLocations<T> (node: T): WithLocations<T> {
return node as WithLocations<T>
}
/**
* A function to check whether scope A is a child of scope B.
* @example
* ```ts
* isChildScope('0-1-2', '0-1') // true
* isChildScope('0-1', '0-1') // false
* ```
*
* @param a the child scope
* @param b the parent scope
* @returns true if scope A is a child of scope B, false otherwise (also when they are the same)
*/
function isChildScope (a: string, b: string) {
return a.startsWith(b) && a.length > b.length
}
abstract class BaseNode<T extends Node = Node> {
abstract type: string
readonly scope: string
node: WithLocations<T>
constructor (node: WithLocations<T>) {
constructor (node: WithLocations<T>, scope: string) {
this.node = node
this.scope = scope
}
/**
@ -72,6 +90,14 @@ abstract class BaseNode<T extends Node = Node> {
* For instance, for a function parameter, this would be the end of the function declaration.
*/
abstract get end (): number
/**
* Check if the node is defined under a specific scope.
* @param scope
*/
isUnderScope (scope: string) {
return isChildScope(this.scope, scope)
}
}
class IdentifierNode extends BaseNode<Identifier> {
@ -90,8 +116,8 @@ class FunctionParamNode extends BaseNode {
type = 'FunctionParam' as const
fnNode: WithLocations<FunctionDeclaration | FunctionExpression | ArrowFunctionExpression>
constructor (node: WithLocations<Node>, fnNode: WithLocations<FunctionDeclaration | FunctionExpression | ArrowFunctionExpression>) {
super(node)
constructor (node: WithLocations<Node>, scope: string, fnNode: WithLocations<FunctionDeclaration | FunctionExpression | ArrowFunctionExpression>) {
super(node, scope)
this.fnNode = fnNode
}
@ -120,8 +146,8 @@ class VariableNode extends BaseNode<Identifier> {
type = 'Variable' as const
variableNode: WithLocations<VariableDeclaration>
constructor (node: WithLocations<Identifier>, variableNode: WithLocations<VariableDeclaration>) {
super(node)
constructor (node: WithLocations<Identifier>, scope: string, variableNode: WithLocations<VariableDeclaration>) {
super(node, scope)
this.variableNode = variableNode
}
@ -138,8 +164,8 @@ class ImportNode extends BaseNode<ImportSpecifier | ImportDefaultSpecifier | Imp
type = 'Import' as const
importNode: WithLocations<Node>
constructor (node: WithLocations<ImportSpecifier | ImportDefaultSpecifier | ImportNamespaceSpecifier>, importNode: WithLocations<Node>) {
super(node)
constructor (node: WithLocations<ImportSpecifier | ImportDefaultSpecifier | ImportNamespaceSpecifier>, scope: string, importNode: WithLocations<Node>) {
super(node, scope)
this.importNode = importNode
}
@ -156,8 +182,8 @@ class CatchParamNode extends BaseNode {
type = 'CatchParam' as const
catchNode: WithLocations<CatchClause>
constructor (node: WithLocations<Node>, catchNode: WithLocations<CatchClause>) {
super(node)
constructor (node: WithLocations<Node>, scope: string, catchNode: WithLocations<CatchClause>) {
super(node, scope)
this.catchNode = catchNode
}
@ -264,7 +290,7 @@ export class ScopeTracker {
const identifiers = getPatternIdentifiers(param)
for (const identifier of identifiers) {
this.declareIdentifier(identifier.name, new FunctionParamNode(identifier, fn))
this.declareIdentifier(identifier.name, new FunctionParamNode(identifier, this.scopeIndexKey, fn))
}
}
@ -276,10 +302,10 @@ export class ScopeTracker {
this.declareIdentifier(
identifier.name,
parent.type === 'VariableDeclaration'
? new VariableNode(identifier, parent)
? new VariableNode(identifier, this.scopeIndexKey, parent)
: parent.type === 'CatchClause'
? new CatchParamNode(identifier, parent)
: new FunctionParamNode(identifier, parent),
? new CatchParamNode(identifier, this.scopeIndexKey, parent)
: new FunctionParamNode(identifier, this.scopeIndexKey, parent),
)
}
}
@ -295,7 +321,7 @@ export class ScopeTracker {
case 'FunctionDeclaration':
// declare function name for named functions, skip for `export default`
if (node.id?.name) {
this.declareIdentifier(node.id.name, new FunctionNode(node))
this.declareIdentifier(node.id.name, new FunctionNode(node, this.scopeIndexKey))
}
this.pushScope()
for (const param of node.params) {
@ -309,7 +335,7 @@ export class ScopeTracker {
this.pushScope()
// can be undefined, for example in class method definitions
if (node.id?.name) {
this.declareIdentifier(node.id.name, new FunctionNode(node))
this.declareIdentifier(node.id.name, new FunctionNode(node, this.scopeIndexKey))
}
this.pushScope()
@ -333,7 +359,7 @@ export class ScopeTracker {
case 'ClassDeclaration':
// declare class name for named classes, skip for `export default`
if (node.id?.name) {
this.declareIdentifier(node.id.name, new IdentifierNode(withLocations(node.id)))
this.declareIdentifier(node.id.name, new IdentifierNode(withLocations(node.id), this.scopeIndexKey))
}
break
@ -342,13 +368,13 @@ export class ScopeTracker {
// e.g. const MyClass = class InternalClassName { // InternalClassName is only available within the class body
this.pushScope()
if (node.id?.name) {
this.declareIdentifier(node.id.name, new IdentifierNode(withLocations(node.id)))
this.declareIdentifier(node.id.name, new IdentifierNode(withLocations(node.id), this.scopeIndexKey))
}
break
case 'ImportDeclaration':
for (const specifier of node.specifiers) {
this.declareIdentifier(specifier.local.name, new ImportNode(withLocations(specifier), node))
this.declareIdentifier(specifier.local.name, new ImportNode(withLocations(specifier), this.scopeIndexKey, node))
}
break
@ -429,6 +455,26 @@ export class ScopeTracker {
return null
}
getCurrentScope () {
return this.scopeIndexKey
}
/**
* Check if the current scope is a child of a specific scope.
* @example
* ```ts
* // current scope is 0-1
* isCurrentScopeUnder('0') // true
* isCurrentScopeUnder('0-1') // false
* ```
*
* @param scope the parent scope
* @returns `true` if the current scope is a child of the specified scope, `false` otherwise (also when they are the same)
*/
isCurrentScopeUnder (scope: string) {
return isChildScope(this.scopeIndexKey, scope)
}
/**
* Freezes the scope tracker, preventing further declarations.
* It also resets the scope index stack to its initial state, so that the scope tracker can be reused.

20
packages/nuxt/src/pages/plugins/page-meta.ts
View File

@ -228,6 +228,8 @@ export const PageMetaPlugin = (options: PageMetaPluginOptions = {}) => createUnp
if (!meta) { return }
const definePageMetaScope = scopeTracker.getCurrentScope()
walk(meta, {
scopeTracker,
enter (node, parent) {
@ -236,10 +238,24 @@ export const PageMetaPlugin = (options: PageMetaPluginOptions = {}) => createUnp
|| node.type !== 'Identifier' // checking for `node.type` to narrow down the type
) { return }
const declaration = scopeTracker.getDeclaration(node.name)
if (declaration) {
// check if the declaration was made inside `definePageMeta` and if so, do not process it
// (ensures that we don't hoist local variables in inline middleware, for example)
if (
declaration.isUnderScope(definePageMetaScope)
// ensures that we compare the correct declaration to the reference
// (when in the same scope, the declaration must come before the reference, otherwise it must be in a parent scope)
&& (scopeTracker.isCurrentScopeUnder(declaration.scope) || declaration.start < node.start)
) {
return
}
}
if (isStaticIdentifier(node.name)) {
addImport(node.name)
} else {
processDeclaration(scopeTracker.getDeclaration(node.name))
} else if (declaration) {
processDeclaration(declaration)
}
},
})

196
packages/nuxt/test/page-metadata.test.ts
View File

@ -393,6 +393,188 @@ definePageMeta({
`)
})
it('should not import static identifiers when shadowed in the same scope', () => {
const sfc = `
<script setup lang="ts">
import { useState } from '#app/composables/state'
definePageMeta({
middleware: () => {
const useState = (key) => ({ value: { isLoggedIn: false } })
const auth = useState('auth')
if (!auth.value.isLoggedIn) {
return navigateTo('/login')
}
},
})
</script>
`
const res = compileScript(parse(sfc).descriptor, { id: 'component.vue' })
expect(transformPlugin.transform.call({
parse: (code: string, opts: any = {}) => Parser.parse(code, {
sourceType: 'module',
ecmaVersion: 'latest',
locations: true,
...opts,
}),
}, res.content, 'component.vue?macro=true')?.code).toMatchInlineSnapshot(`
"const __nuxt_page_meta = {
middleware: () => {
const useState = (key) => ({ value: { isLoggedIn: false } })
const auth = useState('auth')
if (!auth.value.isLoggedIn) {
return navigateTo('/login')
}
},
}
export default __nuxt_page_meta"
`)
})
it('should not import static identifiers when shadowed in parent scope', () => {
const sfc = `
<script setup lang="ts">
import { useState } from '#app/composables/state'
definePageMeta({
middleware: () => {
function isLoggedIn() {
const auth = useState('auth')
return auth.value.isLoggedIn
}
const useState = (key) => ({ value: { isLoggedIn: false } })
if (!isLoggedIn()) {
return navigateTo('/login')
}
},
})
</script>
`
const res = compileScript(parse(sfc).descriptor, { id: 'component.vue' })
expect(transformPlugin.transform.call({
parse: (code: string, opts: any = {}) => Parser.parse(code, {
sourceType: 'module',
ecmaVersion: 'latest',
locations: true,
...opts,
}),
}, res.content, 'component.vue?macro=true')?.code).toMatchInlineSnapshot(`
"const __nuxt_page_meta = {
middleware: () => {
function isLoggedIn() {
const auth = useState('auth')
return auth.value.isLoggedIn
}
const useState = (key) => ({ value: { isLoggedIn: false } })
if (!isLoggedIn()) {
return navigateTo('/login')
}
},
}
export default __nuxt_page_meta"
`)
})
it('should import static identifiers when a shadowed and a non-shadowed one is used', () => {
const sfc = `
<script setup lang="ts">
import { useState } from '#app/composables/state'
definePageMeta({
middleware: [
() => {
const useState = (key) => ({ value: { isLoggedIn: false } })
const auth = useState('auth')
if (!auth.value.isLoggedIn) {
return navigateTo('/login')
}
},
() => {
const auth = useState('auth')
if (!auth.value.isLoggedIn) {
return navigateTo('/login')
}
}
]
})
</script>
`
const res = compileScript(parse(sfc).descriptor, { id: 'component.vue' })
expect(transformPlugin.transform.call({
parse: (code: string, opts: any = {}) => Parser.parse(code, {
sourceType: 'module',
ecmaVersion: 'latest',
locations: true,
...opts,
}),
}, res.content, 'component.vue?macro=true')?.code).toMatchInlineSnapshot(`
"import { useState } from '#app/composables/state'
const __nuxt_page_meta = {
middleware: [
() => {
const useState = (key) => ({ value: { isLoggedIn: false } })
const auth = useState('auth')
if (!auth.value.isLoggedIn) {
return navigateTo('/login')
}
},
() => {
const auth = useState('auth')
if (!auth.value.isLoggedIn) {
return navigateTo('/login')
}
}
]
}
export default __nuxt_page_meta"
`)
})
it('should import static identifiers when a shadowed and a non-shadowed one is used in the same scope', () => {
const sfc = `
<script setup lang="ts">
import { useState } from '#app/composables/state'
definePageMeta({
middleware: () => {
const auth1 = useState('auth')
const useState = (key) => ({ value: { isLoggedIn: false } })
const auth2 = useState('auth')
if (!auth1.value.isLoggedIn || !auth2.value.isLoggedIn) {
return navigateTo('/login')
}
},
})
</script>
`
const res = compileScript(parse(sfc).descriptor, { id: 'component.vue' })
expect(transformPlugin.transform.call({
parse: (code: string, opts: any = {}) => Parser.parse(code, {
sourceType: 'module',
ecmaVersion: 'latest',
locations: true,
...opts,
}),
}, res.content, 'component.vue?macro=true')?.code).toMatchInlineSnapshot(`
"import { useState } from '#app/composables/state'
const __nuxt_page_meta = {
middleware: () => {
const auth1 = useState('auth')
const useState = (key) => ({ value: { isLoggedIn: false } })
const auth2 = useState('auth')
if (!auth1.value.isLoggedIn || !auth2.value.isLoggedIn) {
return navigateTo('/login')
}
},
}
export default __nuxt_page_meta"
`)
})
it('should work with esbuild.keepNames = true', async () => {
const sfc = `
<script setup lang="ts">
@ -516,7 +698,12 @@ definePageMeta({
test () {}
}
console.log(hoisted.value)
const someFunction = () => {
const someValue = 'someValue'
console.log(someValue)
}
console.log(hoisted.value, val)
},
],
validate: (route) => {
@ -564,7 +751,12 @@ const hoisted = ref('hoisted')
test () {}
}
console.log(hoisted.value)
const someFunction = () => {
const someValue = 'someValue'
console.log(someValue)
}
console.log(hoisted.value, val)
},
],
validate: (route) => {

85
packages/nuxt/test/parse.test.ts
View File

@ -1,4 +1,4 @@
import { describe, expect, it } from 'vitest'
import { assert, describe, expect, it } from 'vitest'
import { getUndeclaredIdentifiersInFunction, parseAndWalk } from '../src/core/utils/parse'
import { TestScopeTracker } from './fixture/scope-tracker'
@ -667,4 +667,87 @@ describe('parsing', () => {
expect(processedFunctions).toBe(5)
})
it ('should correctly compare identifiers defined in different scopes', () => {
const code = `
// ""
const a = 1
// ""
const func = () => {
// "0-0"
const b = 2
// "0-0"
function foo() {
// "0-0-0-0"
const c = 3
}
}
// ""
const func2 = () => {
// "1-0"
const d = 2
// "1-0"
function bar() {
// "1-0-0-0"
const e = 3
}
}
// ""
const f = 4
`
const scopeTracker = new TestScopeTracker({
keepExitedScopes: true,
})
parseAndWalk(code, filename, {
scopeTracker,
})
const a = scopeTracker.getDeclarationFromScope('a', '')
const func = scopeTracker.getDeclarationFromScope('func', '')
const foo = scopeTracker.getDeclarationFromScope('foo', '0-0')
const b = scopeTracker.getDeclarationFromScope('b', '0-0')
const c = scopeTracker.getDeclarationFromScope('c', '0-0-0-0')
const func2 = scopeTracker.getDeclarationFromScope('func2', '')
const bar = scopeTracker.getDeclarationFromScope('bar', '1-0')
const d = scopeTracker.getDeclarationFromScope('d', '1-0')
const e = scopeTracker.getDeclarationFromScope('e', '1-0-0-0')
const f = scopeTracker.getDeclarationFromScope('f', '')
assert(a && func && foo && b && c && func2 && bar && d && e && f, 'All declarations should be found')
// identifiers in the same scope should be equal
expect(f.isUnderScope(a.scope)).toBe(false)
expect(func.isUnderScope(a.scope)).toBe(false)
expect(d.isUnderScope(bar.scope)).toBe(false)
// identifiers in deeper scopes should be under the scope of the parent scope
expect(b.isUnderScope(a.scope)).toBe(true)
expect(b.isUnderScope(func.scope)).toBe(true)
expect(c.isUnderScope(a.scope)).toBe(true)
expect(c.isUnderScope(b.scope)).toBe(true)
expect(d.isUnderScope(a.scope)).toBe(true)
expect(d.isUnderScope(func2.scope)).toBe(true)
expect(e.isUnderScope(a.scope)).toBe(true)
expect(e.isUnderScope(d.scope)).toBe(true)
// identifiers in parent scope should not be under the scope of the children
expect(a.isUnderScope(b.scope)).toBe(false)
expect(a.isUnderScope(c.scope)).toBe(false)
expect(a.isUnderScope(d.scope)).toBe(false)
expect(a.isUnderScope(e.scope)).toBe(false)
expect(b.isUnderScope(c.scope)).toBe(false)
// identifiers in parallel scopes should not influence each other
expect(d.isUnderScope(b.scope)).toBe(false)
expect(e.isUnderScope(b.scope)).toBe(false)
expect(b.isUnderScope(d.scope)).toBe(false)
expect(c.isUnderScope(e.scope)).toBe(false)
})
})