From fb79d1a4eea8c6c904c8d5ef9f4325a0696bee6b Mon Sep 17 00:00:00 2001 From: bluwy Date: Sat, 1 Oct 2022 00:22:48 +0800 Subject: [PATCH 1/2] fix(ssr): correctly track scope --- .../node/ssr/__tests__/ssrTransform.spec.ts | 60 ++++++++++++++++++- packages/vite/src/node/ssr/ssrTransform.ts | 52 ++++++++++------ 2 files changed, 93 insertions(+), 19 deletions(-) diff --git a/packages/vite/src/node/ssr/__tests__/ssrTransform.spec.ts b/packages/vite/src/node/ssr/__tests__/ssrTransform.spec.ts index e70a3e30c948cc..5a597d1f78b205 100644 --- a/packages/vite/src/node/ssr/__tests__/ssrTransform.spec.ts +++ b/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 = '') => @@ -728,3 +727,62 @@ console.log("it can parse the hashbang")` console.log(\\"it can parse the hashbang\\")" `) }) + +test('import name and variable scope shadowing', 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 }});;" + `) +}) diff --git a/packages/vite/src/node/ssr/ssrTransform.ts b/packages/vite/src/node/ssr/ssrTransform.ts index 0798d674547fb3..fe24dddac2e79a 100644 --- a/packages/vite/src/node/ssr/ssrTransform.ts +++ b/packages/vite/src/node/ssr/ssrTransform.ts @@ -322,7 +322,7 @@ function walk( const scopeMap = new WeakMap<_Node, Set>() 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 @@ -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) } } @@ -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) @@ -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 @@ -430,7 +437,7 @@ 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) } @@ -438,7 +445,13 @@ function walk( }, leave(node: Node, parent: Node | null) { - parent && parentStack.shift() + // untrack parent stack from above + if ( + parent && + !(parent.type === 'IfStatement' && node === parent.alternate) + ) { + parentStack.shift() + } } }) @@ -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( From 1a25f5c45f8f1adde355d9eb4dc40aba53a453ee Mon Sep 17 00:00:00 2001 From: bluwy Date: Sat, 1 Oct 2022 00:28:10 +0800 Subject: [PATCH 2/2] chore: update test name --- packages/vite/src/node/ssr/__tests__/ssrTransform.spec.ts | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/packages/vite/src/node/ssr/__tests__/ssrTransform.spec.ts b/packages/vite/src/node/ssr/__tests__/ssrTransform.spec.ts index 5a597d1f78b205..a5ca20c9be3d09 100644 --- a/packages/vite/src/node/ssr/__tests__/ssrTransform.spec.ts +++ b/packages/vite/src/node/ssr/__tests__/ssrTransform.spec.ts @@ -728,7 +728,8 @@ console.log("it can parse the hashbang")` `) }) -test('import name and variable scope shadowing', async () => { +// #10289 +test('track scope by class, function, condition blocks', async () => { const code = ` import { foo, bar } from 'foobar' if (false) {