From 7663a79a295800da7c889fcd2e8e1e6d775263db Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E4=B8=89=E5=92=B2=E6=99=BA=E5=AD=90=20Kevin=20Deng?= Date: Wed, 9 Nov 2022 09:26:43 +0800 Subject: [PATCH] fix(reactivity-transform): respect user defined symbols that conflict with macros (#6840) closes #6838 --- .../reactivityTransform.spec.ts.snap | 21 ++++ .../__tests__/reactivityTransform.spec.ts | 29 +++++ .../src/reactivityTransform.ts | 100 +++++++++++++----- 3 files changed, 121 insertions(+), 29 deletions(-) diff --git a/packages/reactivity-transform/__tests__/__snapshots__/reactivityTransform.spec.ts.snap b/packages/reactivity-transform/__tests__/__snapshots__/reactivityTransform.spec.ts.snap index 9d55945a381..ea6641578b8 100644 --- a/packages/reactivity-transform/__tests__/__snapshots__/reactivityTransform.spec.ts.snap +++ b/packages/reactivity-transform/__tests__/__snapshots__/reactivityTransform.spec.ts.snap @@ -199,6 +199,27 @@ exports[`object destructure w/ mid-path default values 1`] = ` " `; +exports[`should not overwrite current scope 1`] = ` +" + const fn = () => { + const $ = () => 'foo' + const $ref = () => 'bar' + const $$ = () => 'baz' + console.log($()) + console.log($ref()) + console.log($$()) + } + " +`; + +exports[`should not overwrite importing 1`] = ` +" + import { $, $$ } from './foo' + $('foo') + $$('bar') + " +`; + exports[`should not rewrite scope variable 1`] = ` "import { ref as _ref } from 'vue' diff --git a/packages/reactivity-transform/__tests__/reactivityTransform.spec.ts b/packages/reactivity-transform/__tests__/reactivityTransform.spec.ts index 7471615cae2..f1c5d4eb0cb 100644 --- a/packages/reactivity-transform/__tests__/reactivityTransform.spec.ts +++ b/packages/reactivity-transform/__tests__/reactivityTransform.spec.ts @@ -416,6 +416,35 @@ test('macro import alias and removal', () => { assertCode(code) }) +// #6838 +test('should not overwrite importing', () => { + const { code } = transform( + ` + import { $, $$ } from './foo' + $('foo') + $$('bar') + ` + ) + assertCode(code) +}) + +// #6838 +test('should not overwrite current scope', () => { + const { code } = transform( + ` + const fn = () => { + const $ = () => 'foo' + const $ref = () => 'bar' + const $$ = () => 'baz' + console.log($()) + console.log($ref()) + console.log($$()) + } + ` + ) + assertCode(code) +}) + describe('errors', () => { test('$ref w/ destructure', () => { expect(() => transform(`let { a } = $ref(1)`)).toThrow( diff --git a/packages/reactivity-transform/src/reactivityTransform.ts b/packages/reactivity-transform/src/reactivityTransform.ts index 9d47416f43c..f35be8b2e1d 100644 --- a/packages/reactivity-transform/src/reactivityTransform.ts +++ b/packages/reactivity-transform/src/reactivityTransform.ts @@ -8,7 +8,11 @@ import { Program, VariableDeclarator, Expression, - VariableDeclaration + VariableDeclaration, + ImportDeclaration, + ImportSpecifier, + ImportDefaultSpecifier, + ImportNamespaceSpecifier } from '@babel/types' import MagicString, { SourceMap } from 'magic-string' import { walk } from 'estree-walker' @@ -25,6 +29,7 @@ import { hasOwn, isArray, isString, genPropsAccessExp } from '@vue/shared' const CONVERT_SYMBOL = '$' const ESCAPE_SYMBOL = '$$' +const IMPORT_SOURCE = 'vue/macros' const shorthands = ['ref', 'computed', 'shallowRef', 'toRef', 'customRef'] const transformCheckRE = /[^\w]\$(?:\$|ref|computed|shallowRef)?\s*(\(|\<)/ @@ -48,6 +53,13 @@ export interface RefTransformResults { importedHelpers: string[] } +export interface ImportBinding { + local: string + imported: string + source: string + specifier: ImportSpecifier | ImportDefaultSpecifier | ImportNamespaceSpecifier +} + export function transform( src: string, { @@ -115,39 +127,40 @@ export function transformAST( // TODO remove when out of experimental warnExperimental() - let convertSymbol = CONVERT_SYMBOL - let escapeSymbol = ESCAPE_SYMBOL + const userImports: Record = Object.create(null) + for (const node of ast.body) { + if (node.type !== 'ImportDeclaration') continue + walkImportDeclaration(node) + } // macro import handling - for (const node of ast.body) { - if ( - node.type === 'ImportDeclaration' && - node.source.value === 'vue/macros' - ) { - // remove macro imports - s.remove(node.start! + offset, node.end! + offset) - // check aliasing - for (const specifier of node.specifiers) { - if (specifier.type === 'ImportSpecifier') { - const imported = (specifier.imported as Identifier).name - const local = specifier.local.name - if (local !== imported) { - if (imported === ESCAPE_SYMBOL) { - escapeSymbol = local - } else if (imported === CONVERT_SYMBOL) { - convertSymbol = local - } else { - error( - `macro imports for ref-creating methods do not support aliasing.`, - specifier - ) - } - } - } + let convertSymbol: string | undefined + let escapeSymbol: string | undefined + for (const { local, imported, source, specifier } of Object.values( + userImports + )) { + if (source === IMPORT_SOURCE) { + if (imported === ESCAPE_SYMBOL) { + escapeSymbol = local + } else if (imported === CONVERT_SYMBOL) { + convertSymbol = local + } else if (imported !== local) { + error( + `macro imports for ref-creating methods do not support aliasing.`, + specifier + ) } } } + // default symbol + if (!convertSymbol && !userImports[CONVERT_SYMBOL]) { + convertSymbol = CONVERT_SYMBOL + } + if (!escapeSymbol && !userImports[ESCAPE_SYMBOL]) { + escapeSymbol = ESCAPE_SYMBOL + } + const importedHelpers = new Set() const rootScope: Scope = {} const scopeStack: Scope[] = [rootScope] @@ -170,7 +183,32 @@ export function transformAST( } } + function walkImportDeclaration(node: ImportDeclaration) { + const source = node.source.value + if (source === IMPORT_SOURCE) { + s.remove(node.start! + offset, node.end! + offset) + } + + for (const specifier of node.specifiers) { + const local = specifier.local.name + const imported = + (specifier.type === 'ImportSpecifier' && + specifier.imported.type === 'Identifier' && + specifier.imported.name) || + 'default' + userImports[local] = { + source, + local, + imported, + specifier + } + } + } + function isRefCreationCall(callee: string): string | false { + if (!convertSymbol || currentScope[convertSymbol] !== undefined) { + return false + } if (callee === convertSymbol) { return convertSymbol } @@ -628,7 +666,11 @@ export function transformAST( ) } - if (callee === escapeSymbol) { + if ( + escapeSymbol && + currentScope[escapeSymbol] === undefined && + callee === escapeSymbol + ) { s.remove(node.callee.start! + offset, node.callee.end! + offset) escapeScope = node }