Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix(ssr): correctly track scope #10300

Merged
merged 2 commits into from Oct 1, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
61 changes: 60 additions & 1 deletion packages/vite/src/node/ssr/__tests__/ssrTransform.spec.ts
@@ -1,6 +1,5 @@
import { expect, test } from 'vitest'
import { transformWithEsbuild } from '../../plugins/esbuild'
import { traverseHtml } from '../../plugins/html'
import { ssrTransform } from '../ssrTransform'

const ssrTransformSimple = async (code: string, url = '') =>
Expand Down Expand Up @@ -728,3 +727,63 @@ console.log("it can parse the hashbang")`
console.log(\\"it can parse the hashbang\\")"
`)
})

// #10289
test('track scope by class, function, condition blocks', async () => {
const code = `
import { foo, bar } from 'foobar'
if (false) {
const foo = 'foo'
console.log(foo)
} else if (false) {
const [bar] = ['bar']
console.log(bar)
} else {
console.log(foo)
console.log(bar)
}
export class Test {
constructor() {
if (false) {
const foo = 'foo'
console.log(foo)
} else if (false) {
const [bar] = ['bar']
console.log(bar)
} else {
console.log(foo)
console.log(bar)
}
}
};`.trim()

expect(await ssrTransformSimpleCode(code)).toMatchInlineSnapshot(`
"const __vite_ssr_import_0__ = await __vite_ssr_import__(\\"foobar\\");

if (false) {
const foo = 'foo'
console.log(foo)
} else if (false) {
const [bar] = ['bar']
console.log(bar)
} else {
console.log(__vite_ssr_import_0__.foo)
console.log(__vite_ssr_import_0__.bar)
}
class Test {
constructor() {
if (false) {
const foo = 'foo'
console.log(foo)
} else if (false) {
const [bar] = ['bar']
console.log(bar)
} else {
console.log(__vite_ssr_import_0__.foo)
console.log(__vite_ssr_import_0__.bar)
}
}
}
Object.defineProperty(__vite_ssr_exports__, \\"Test\\", { enumerable: true, configurable: true, get(){ return Test }});;"
`)
})
52 changes: 34 additions & 18 deletions packages/vite/src/node/ssr/ssrTransform.ts
Expand Up @@ -322,7 +322,7 @@ function walk(
const scopeMap = new WeakMap<_Node, Set<string>>()
const identifiers: [id: any, stack: Node[]][] = []

const setScope = (node: FunctionNode, name: string) => {
const setScope = (node: _Node, name: string) => {
let scopeIds = scopeMap.get(node)
if (scopeIds && scopeIds.has(name)) {
return
Expand All @@ -337,29 +337,29 @@ function walk(
function isInScope(name: string, parents: Node[]) {
return parents.some((node) => node && scopeMap.get(node)?.has(name))
}
function handlePattern(p: Pattern, parentFunction: FunctionNode) {
function handlePattern(p: Pattern, parentScope: _Node) {
if (p.type === 'Identifier') {
setScope(parentFunction, p.name)
setScope(parentScope, p.name)
} else if (p.type === 'RestElement') {
handlePattern(p.argument, parentFunction)
handlePattern(p.argument, parentScope)
} else if (p.type === 'ObjectPattern') {
p.properties.forEach((property) => {
if (property.type === 'RestElement') {
setScope(parentFunction, (property.argument as Identifier).name)
setScope(parentScope, (property.argument as Identifier).name)
} else {
handlePattern(property.value, parentFunction)
handlePattern(property.value, parentScope)
}
})
} else if (p.type === 'ArrayPattern') {
p.elements.forEach((element) => {
if (element) {
handlePattern(element, parentFunction)
handlePattern(element, parentScope)
}
})
} else if (p.type === 'AssignmentPattern') {
handlePattern(p.left, parentFunction)
handlePattern(p.left, parentScope)
} else {
setScope(parentFunction, (p as any).name)
setScope(parentScope, (p as any).name)
}
}

Expand All @@ -369,7 +369,14 @@ function walk(
return this.skip()
}

parent && parentStack.unshift(parent)
// track parent stack, skip for "else-if"/"else" branches as acorn nests
// the ast within "if" nodes instead of flattening them
if (
parent &&
!(parent.type === 'IfStatement' && node === parent.alternate)
) {
parentStack.unshift(parent)
}

if (node.type === 'MetaProperty' && node.meta.name === 'import') {
onImportMeta(node)
Expand All @@ -389,9 +396,9 @@ function walk(
// If it is a function declaration, it could be shadowing an import
// Add its name to the scope so it won't get replaced
if (node.type === 'FunctionDeclaration') {
const parentFunction = findParentFunction(parentStack)
if (parentFunction) {
setScope(parentFunction, node.id!.name)
const parentScope = findParentScope(parentStack)
if (parentScope) {
setScope(parentScope, node.id!.name)
}
}
// walk function expressions and add its arguments to known identifiers
Expand Down Expand Up @@ -430,15 +437,21 @@ function walk(
// mark property in destructuring pattern
setIsNodeInPattern(node)
} else if (node.type === 'VariableDeclarator') {
const parentFunction = findParentFunction(parentStack)
const parentFunction = findParentScope(parentStack)
if (parentFunction) {
handlePattern(node.id, parentFunction)
}
}
},

leave(node: Node, parent: Node | null) {
parent && parentStack.shift()
// untrack parent stack from above
if (
parent &&
!(parent.type === 'IfStatement' && node === parent.alternate)
) {
parentStack.shift()
}
}
})

Expand Down Expand Up @@ -521,12 +534,15 @@ const isStaticProperty = (node: _Node): node is Property =>
const isStaticPropertyKey = (node: _Node, parent: _Node) =>
isStaticProperty(parent) && parent.key === node

const functionNodeTypeRE = /Function(?:Expression|Declaration)$|Method$/
function isFunction(node: _Node): node is FunctionNode {
return /Function(?:Expression|Declaration)$|Method$/.test(node.type)
return functionNodeTypeRE.test(node.type)
}

function findParentFunction(parentStack: _Node[]): FunctionNode | undefined {
return parentStack.find((i) => isFunction(i)) as FunctionNode
const scopeNodeTypeRE =
/(?:Function|Class)(?:Expression|Declaration)$|Method$|^IfStatement$/
function findParentScope(parentStack: _Node[]): _Node | undefined {
return parentStack.find((i) => scopeNodeTypeRE.test(i.type))
}

function isInDestructuringAssignment(
Expand Down