Skip to content

Commit

Permalink
fix(ssr): correctly track scope (#10300)
Browse files Browse the repository at this point in the history
  • Loading branch information
bluwy authored and antfu committed Oct 3, 2022
1 parent dc140af commit effb7c3
Show file tree
Hide file tree
Showing 2 changed files with 94 additions and 19 deletions.
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

0 comments on commit effb7c3

Please sign in to comment.