Skip to content

Commit 236cac3

Browse files
committed
fix(compiler-core): use ast-based check for function expressions when possible
close #11615
1 parent b27d900 commit 236cac3

File tree

5 files changed

+96
-28
lines changed

5 files changed

+96
-28
lines changed

packages/compiler-core/__tests__/transforms/vOn.spec.ts

+15
Original file line numberDiff line numberDiff line change
@@ -285,6 +285,21 @@ describe('compiler: transform v-on', () => {
285285
},
286286
],
287287
})
288+
289+
const { node: node2 } = parseWithVOn(
290+
`<div @click="(e: (number | string)[]) => foo(e)"/>`,
291+
)
292+
expect((node2.codegenNode as VNodeCall).props).toMatchObject({
293+
properties: [
294+
{
295+
key: { content: `onClick` },
296+
value: {
297+
type: NodeTypes.SIMPLE_EXPRESSION,
298+
content: `(e: (number | string)[]) => foo(e)`,
299+
},
300+
},
301+
],
302+
})
288303
})
289304

290305
test('should NOT wrap as function if expression is already function expression (async)', () => {

packages/compiler-core/__tests__/utils.spec.ts

+10-6
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
1-
import type { TransformContext } from '../src'
2-
import type { Position } from '../src/ast'
1+
import type { ExpressionNode, TransformContext } from '../src'
2+
import { type Position, createSimpleExpression } from '../src/ast'
33
import {
44
advancePositionWithClone,
55
isMemberExpressionBrowser,
@@ -41,7 +41,8 @@ describe('advancePositionWithClone', () => {
4141
})
4242

4343
describe('isMemberExpression', () => {
44-
function commonAssertions(fn: (str: string) => boolean) {
44+
function commonAssertions(raw: (exp: ExpressionNode) => boolean) {
45+
const fn = (str: string) => raw(createSimpleExpression(str))
4546
// should work
4647
expect(fn('obj.foo')).toBe(true)
4748
expect(fn('obj[foo]')).toBe(true)
@@ -78,13 +79,16 @@ describe('isMemberExpression', () => {
7879

7980
test('browser', () => {
8081
commonAssertions(isMemberExpressionBrowser)
81-
expect(isMemberExpressionBrowser('123[a]')).toBe(false)
82+
expect(isMemberExpressionBrowser(createSimpleExpression('123[a]'))).toBe(
83+
false,
84+
)
8285
})
8386

8487
test('node', () => {
8588
const ctx = { expressionPlugins: ['typescript'] } as any as TransformContext
86-
const fn = (str: string) => isMemberExpressionNode(str, ctx)
87-
commonAssertions(fn)
89+
const fn = (str: string) =>
90+
isMemberExpressionNode(createSimpleExpression(str), ctx)
91+
commonAssertions(exp => isMemberExpressionNode(exp, ctx))
8892

8993
// TS-specific checks
9094
expect(fn('foo as string')).toBe(true)

packages/compiler-core/src/transforms/vModel.ts

+1-4
Original file line numberDiff line numberDiff line change
@@ -55,10 +55,7 @@ export const transformModel: DirectiveTransform = (dir, node, context) => {
5555
bindingType === BindingTypes.SETUP_REF ||
5656
bindingType === BindingTypes.SETUP_MAYBE_REF)
5757

58-
if (
59-
!expString.trim() ||
60-
(!isMemberExpression(expString, context) && !maybeRef)
61-
) {
58+
if (!expString.trim() || (!isMemberExpression(exp, context) && !maybeRef)) {
6259
context.onError(
6360
createCompilerError(ErrorCodes.X_V_MODEL_MALFORMED_EXPRESSION, exp.loc),
6461
)

packages/compiler-core/src/transforms/vOn.ts

+3-6
Original file line numberDiff line numberDiff line change
@@ -13,12 +13,9 @@ import { camelize, toHandlerKey } from '@vue/shared'
1313
import { ErrorCodes, createCompilerError } from '../errors'
1414
import { processExpression } from './transformExpression'
1515
import { validateBrowserExpression } from '../validateExpression'
16-
import { hasScopeRef, isMemberExpression } from '../utils'
16+
import { hasScopeRef, isFnExpression, isMemberExpression } from '../utils'
1717
import { TO_HANDLER_KEY } from '../runtimeHelpers'
1818

19-
const fnExpRE =
20-
/^\s*(async\s*)?(\([^)]*?\)|[\w$_]+)\s*(:[^=]+)?=>|^\s*(async\s+)?function(?:\s+[\w$]+)?\s*\(/
21-
2219
export interface VOnDirectiveNode extends DirectiveNode {
2320
// v-on without arg is handled directly in ./transformElements.ts due to it affecting
2421
// codegen for the entire props object. This transform here is only for v-on
@@ -84,8 +81,8 @@ export const transformOn: DirectiveTransform = (
8481
}
8582
let shouldCache: boolean = context.cacheHandlers && !exp && !context.inVOnce
8683
if (exp) {
87-
const isMemberExp = isMemberExpression(exp.content, context)
88-
const isInlineStatement = !(isMemberExp || fnExpRE.test(exp.content))
84+
const isMemberExp = isMemberExpression(exp, context)
85+
const isInlineStatement = !(isMemberExp || isFnExpression(exp, context))
8986
const hasMultipleStatements = exp.content.includes(`;`)
9087

9188
// process the expression since it's been skipped

packages/compiler-core/src/utils.ts

+67-12
Original file line numberDiff line numberDiff line change
@@ -39,7 +39,7 @@ import {
3939
import { NOOP, isObject, isString } from '@vue/shared'
4040
import type { PropsExpression } from './transforms/transformElement'
4141
import { parseExpression } from '@babel/parser'
42-
import type { Expression } from '@babel/types'
42+
import type { Expression, Node } from '@babel/types'
4343
import { unwrapTSNode } from './babelUtils'
4444

4545
export const isStaticExp = (p: JSChildNode): p is SimpleExpressionNode =>
@@ -77,15 +77,20 @@ const validFirstIdentCharRE = /[A-Za-z_$\xA0-\uFFFF]/
7777
const validIdentCharRE = /[\.\?\w$\xA0-\uFFFF]/
7878
const whitespaceRE = /\s+[.[]\s*|\s*[.[]\s+/g
7979

80+
const getExpSource = (exp: ExpressionNode): string =>
81+
exp.type === NodeTypes.SIMPLE_EXPRESSION ? exp.content : exp.loc.source
82+
8083
/**
8184
* Simple lexer to check if an expression is a member expression. This is
8285
* lax and only checks validity at the root level (i.e. does not validate exps
8386
* inside square brackets), but it's ok since these are only used on template
8487
* expressions and false positives are invalid expressions in the first place.
8588
*/
86-
export const isMemberExpressionBrowser = (path: string): boolean => {
89+
export const isMemberExpressionBrowser = (exp: ExpressionNode): boolean => {
8790
// remove whitespaces around . or [ first
88-
path = path.trim().replace(whitespaceRE, s => s.trim())
91+
const path = getExpSource(exp)
92+
.trim()
93+
.replace(whitespaceRE, s => s.trim())
8994

9095
let state = MemberExpLexState.inMemberExp
9196
let stateStack: MemberExpLexState[] = []
@@ -152,13 +157,20 @@ export const isMemberExpressionBrowser = (path: string): boolean => {
152157
return !currentOpenBracketCount && !currentOpenParensCount
153158
}
154159

155-
export const isMemberExpressionNode = __BROWSER__
156-
? (NOOP as any as (path: string, context: TransformContext) => boolean)
157-
: (path: string, context: TransformContext): boolean => {
160+
export const isMemberExpressionNode: (
161+
exp: ExpressionNode,
162+
context: TransformContext,
163+
) => boolean = __BROWSER__
164+
? (NOOP as any)
165+
: (exp, context) => {
158166
try {
159-
let ret: Expression = parseExpression(path, {
160-
plugins: context.expressionPlugins,
161-
})
167+
let ret: Node =
168+
exp.ast ||
169+
parseExpression(getExpSource(exp), {
170+
plugins: context.expressionPlugins
171+
? [...context.expressionPlugins, 'typescript']
172+
: ['typescript'],
173+
})
162174
ret = unwrapTSNode(ret) as Expression
163175
return (
164176
ret.type === 'MemberExpression' ||
@@ -170,9 +182,52 @@ export const isMemberExpressionNode = __BROWSER__
170182
}
171183
}
172184

173-
export const isMemberExpression = __BROWSER__
174-
? isMemberExpressionBrowser
175-
: isMemberExpressionNode
185+
export const isMemberExpression: (
186+
exp: ExpressionNode,
187+
context: TransformContext,
188+
) => boolean = __BROWSER__ ? isMemberExpressionBrowser : isMemberExpressionNode
189+
190+
const fnExpRE =
191+
/^\s*(async\s*)?(\([^)]*?\)|[\w$_]+)\s*(:[^=]+)?=>|^\s*(async\s+)?function(?:\s+[\w$]+)?\s*\(/
192+
193+
export const isFnExpressionBrowser: (exp: ExpressionNode) => boolean = exp =>
194+
fnExpRE.test(getExpSource(exp))
195+
196+
export const isFnExpressionNode: (
197+
exp: ExpressionNode,
198+
context: TransformContext,
199+
) => boolean = __BROWSER__
200+
? (NOOP as any)
201+
: (exp, context) => {
202+
try {
203+
let ret: Node =
204+
exp.ast ||
205+
parseExpression(getExpSource(exp), {
206+
plugins: context.expressionPlugins
207+
? [...context.expressionPlugins, 'typescript']
208+
: ['typescript'],
209+
})
210+
// parser may parse the exp as statements when it contains semicolons
211+
if (ret.type === 'Program') {
212+
ret = ret.body[0]
213+
if (ret.type === 'ExpressionStatement') {
214+
ret = ret.expression
215+
}
216+
}
217+
ret = unwrapTSNode(ret) as Expression
218+
return (
219+
ret.type === 'FunctionExpression' ||
220+
ret.type === 'ArrowFunctionExpression'
221+
)
222+
} catch (e) {
223+
return false
224+
}
225+
}
226+
227+
export const isFnExpression: (
228+
exp: ExpressionNode,
229+
context: TransformContext,
230+
) => boolean = __BROWSER__ ? isFnExpressionBrowser : isFnExpressionNode
176231

177232
export function advancePositionWithClone(
178233
pos: Position,

0 commit comments

Comments
 (0)