From 58612294757480974e667652ede5bbcf72b1089d Mon Sep 17 00:00:00 2001 From: Evan You Date: Thu, 15 Aug 2024 09:58:30 +0800 Subject: [PATCH] fix(compiler-core): use ast-based check for function expressions when possible close #11615 --- .../__tests__/transforms/vOn.spec.ts | 15 ++++ .../compiler-core/__tests__/utils.spec.ts | 16 +++-- .../compiler-core/src/transforms/vModel.ts | 5 +- packages/compiler-core/src/transforms/vOn.ts | 9 +-- packages/compiler-core/src/utils.ts | 69 ++++++++++++++++--- 5 files changed, 89 insertions(+), 25 deletions(-) diff --git a/packages/compiler-core/__tests__/transforms/vOn.spec.ts b/packages/compiler-core/__tests__/transforms/vOn.spec.ts index 66097f29f0f..9fda0259585 100644 --- a/packages/compiler-core/__tests__/transforms/vOn.spec.ts +++ b/packages/compiler-core/__tests__/transforms/vOn.spec.ts @@ -285,6 +285,21 @@ describe('compiler: transform v-on', () => { }, ], }) + + const { node: node2 } = parseWithVOn( + `
`, + ) + expect((node2.codegenNode as VNodeCall).props).toMatchObject({ + properties: [ + { + key: { content: `onClick` }, + value: { + type: NodeTypes.SIMPLE_EXPRESSION, + content: `(e: (number | string)[]) => foo(e)`, + }, + }, + ], + }) }) test('should NOT wrap as function if expression is already function expression (async)', () => { diff --git a/packages/compiler-core/__tests__/utils.spec.ts b/packages/compiler-core/__tests__/utils.spec.ts index 506aa86982e..2d377a271ac 100644 --- a/packages/compiler-core/__tests__/utils.spec.ts +++ b/packages/compiler-core/__tests__/utils.spec.ts @@ -1,5 +1,5 @@ -import type { TransformContext } from '../src' -import type { Position } from '../src/ast' +import type { ExpressionNode, TransformContext } from '../src' +import { type Position, createSimpleExpression } from '../src/ast' import { advancePositionWithClone, isMemberExpressionBrowser, @@ -41,7 +41,8 @@ describe('advancePositionWithClone', () => { }) describe('isMemberExpression', () => { - function commonAssertions(fn: (str: string) => boolean) { + function commonAssertions(raw: (exp: ExpressionNode) => boolean) { + const fn = (str: string) => raw(createSimpleExpression(str)) // should work expect(fn('obj.foo')).toBe(true) expect(fn('obj[foo]')).toBe(true) @@ -78,13 +79,16 @@ describe('isMemberExpression', () => { test('browser', () => { commonAssertions(isMemberExpressionBrowser) - expect(isMemberExpressionBrowser('123[a]')).toBe(false) + expect(isMemberExpressionBrowser(createSimpleExpression('123[a]'))).toBe( + false, + ) }) test('node', () => { const ctx = { expressionPlugins: ['typescript'] } as any as TransformContext - const fn = (str: string) => isMemberExpressionNode(str, ctx) - commonAssertions(fn) + const fn = (str: string) => + isMemberExpressionNode(createSimpleExpression(str), ctx) + commonAssertions(exp => isMemberExpressionNode(exp, ctx)) // TS-specific checks expect(fn('foo as string')).toBe(true) diff --git a/packages/compiler-core/src/transforms/vModel.ts b/packages/compiler-core/src/transforms/vModel.ts index 88be6238a4d..60c7dc63bd1 100644 --- a/packages/compiler-core/src/transforms/vModel.ts +++ b/packages/compiler-core/src/transforms/vModel.ts @@ -55,10 +55,7 @@ export const transformModel: DirectiveTransform = (dir, node, context) => { bindingType === BindingTypes.SETUP_REF || bindingType === BindingTypes.SETUP_MAYBE_REF) - if ( - !expString.trim() || - (!isMemberExpression(expString, context) && !maybeRef) - ) { + if (!expString.trim() || (!isMemberExpression(exp, context) && !maybeRef)) { context.onError( createCompilerError(ErrorCodes.X_V_MODEL_MALFORMED_EXPRESSION, exp.loc), ) diff --git a/packages/compiler-core/src/transforms/vOn.ts b/packages/compiler-core/src/transforms/vOn.ts index a1631e10db3..ed809a2d79f 100644 --- a/packages/compiler-core/src/transforms/vOn.ts +++ b/packages/compiler-core/src/transforms/vOn.ts @@ -13,12 +13,9 @@ import { camelize, toHandlerKey } from '@vue/shared' import { ErrorCodes, createCompilerError } from '../errors' import { processExpression } from './transformExpression' import { validateBrowserExpression } from '../validateExpression' -import { hasScopeRef, isMemberExpression } from '../utils' +import { hasScopeRef, isFnExpression, isMemberExpression } from '../utils' import { TO_HANDLER_KEY } from '../runtimeHelpers' -const fnExpRE = - /^\s*(async\s*)?(\([^)]*?\)|[\w$_]+)\s*(:[^=]+)?=>|^\s*(async\s+)?function(?:\s+[\w$]+)?\s*\(/ - export interface VOnDirectiveNode extends DirectiveNode { // v-on without arg is handled directly in ./transformElements.ts due to it affecting // codegen for the entire props object. This transform here is only for v-on @@ -84,8 +81,8 @@ export const transformOn: DirectiveTransform = ( } let shouldCache: boolean = context.cacheHandlers && !exp && !context.inVOnce if (exp) { - const isMemberExp = isMemberExpression(exp.content, context) - const isInlineStatement = !(isMemberExp || fnExpRE.test(exp.content)) + const isMemberExp = isMemberExpression(exp, context) + const isInlineStatement = !(isMemberExp || isFnExpression(exp, context)) const hasMultipleStatements = exp.content.includes(`;`) // process the expression since it's been skipped diff --git a/packages/compiler-core/src/utils.ts b/packages/compiler-core/src/utils.ts index 54a3a845737..5b29a117c14 100644 --- a/packages/compiler-core/src/utils.ts +++ b/packages/compiler-core/src/utils.ts @@ -40,7 +40,7 @@ import { import { NOOP, isObject, isString } from '@vue/shared' import type { PropsExpression } from './transforms/transformElement' import { parseExpression } from '@babel/parser' -import type { Expression } from '@babel/types' +import type { Expression, Node } from '@babel/types' import { unwrapTSNode } from './babelUtils' export const isStaticExp = (p: JSChildNode): p is SimpleExpressionNode => @@ -78,15 +78,20 @@ const validFirstIdentCharRE = /[A-Za-z_$\xA0-\uFFFF]/ const validIdentCharRE = /[\.\?\w$\xA0-\uFFFF]/ const whitespaceRE = /\s+[.[]\s*|\s*[.[]\s+/g +const getExpSource = (exp: ExpressionNode): string => + exp.type === NodeTypes.SIMPLE_EXPRESSION ? exp.content : exp.loc.source + /** * Simple lexer to check if an expression is a member expression. This is * lax and only checks validity at the root level (i.e. does not validate exps * inside square brackets), but it's ok since these are only used on template * expressions and false positives are invalid expressions in the first place. */ -export const isMemberExpressionBrowser = (path: string): boolean => { +export const isMemberExpressionBrowser = (exp: ExpressionNode): boolean => { // remove whitespaces around . or [ first - path = path.trim().replace(whitespaceRE, s => s.trim()) + const path = getExpSource(exp) + .trim() + .replace(whitespaceRE, s => s.trim()) let state = MemberExpLexState.inMemberExp let stateStack: MemberExpLexState[] = [] @@ -154,15 +159,19 @@ export const isMemberExpressionBrowser = (path: string): boolean => { } export const isMemberExpressionNode: ( - path: string, + exp: ExpressionNode, context: TransformContext, ) => boolean = __BROWSER__ ? (NOOP as any) - : (path: string, context: TransformContext): boolean => { + : (exp, context) => { try { - let ret: Expression = parseExpression(path, { - plugins: context.expressionPlugins, - }) + let ret: Node = + exp.ast || + parseExpression(getExpSource(exp), { + plugins: context.expressionPlugins + ? [...context.expressionPlugins, 'typescript'] + : ['typescript'], + }) ret = unwrapTSNode(ret) as Expression return ( ret.type === 'MemberExpression' || @@ -175,10 +184,52 @@ export const isMemberExpressionNode: ( } export const isMemberExpression: ( - path: string, + exp: ExpressionNode, context: TransformContext, ) => boolean = __BROWSER__ ? isMemberExpressionBrowser : isMemberExpressionNode +const fnExpRE = + /^\s*(async\s*)?(\([^)]*?\)|[\w$_]+)\s*(:[^=]+)?=>|^\s*(async\s+)?function(?:\s+[\w$]+)?\s*\(/ + +export const isFnExpressionBrowser: (exp: ExpressionNode) => boolean = exp => + fnExpRE.test(getExpSource(exp)) + +export const isFnExpressionNode: ( + exp: ExpressionNode, + context: TransformContext, +) => boolean = __BROWSER__ + ? (NOOP as any) + : (exp, context) => { + try { + let ret: Node = + exp.ast || + parseExpression(getExpSource(exp), { + plugins: context.expressionPlugins + ? [...context.expressionPlugins, 'typescript'] + : ['typescript'], + }) + // parser may parse the exp as statements when it contains semicolons + if (ret.type === 'Program') { + ret = ret.body[0] + if (ret.type === 'ExpressionStatement') { + ret = ret.expression + } + } + ret = unwrapTSNode(ret) as Expression + return ( + ret.type === 'FunctionExpression' || + ret.type === 'ArrowFunctionExpression' + ) + } catch (e) { + return false + } + } + +export const isFnExpression: ( + exp: ExpressionNode, + context: TransformContext, +) => boolean = __BROWSER__ ? isFnExpressionBrowser : isFnExpressionNode + export function advancePositionWithClone( pos: Position, source: string,