Skip to content

Commit 5861229

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

File tree

5 files changed

+89
-25
lines changed

5 files changed

+89
-25
lines changed

Diff for: 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)', () => {

Diff for: 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)

Diff for: 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
)

Diff for: 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

Diff for: packages/compiler-core/src/utils.ts

+60-9
Original file line numberDiff line numberDiff line change
@@ -40,7 +40,7 @@ import {
4040
import { NOOP, isObject, isString } from '@vue/shared'
4141
import type { PropsExpression } from './transforms/transformElement'
4242
import { parseExpression } from '@babel/parser'
43-
import type { Expression } from '@babel/types'
43+
import type { Expression, Node } from '@babel/types'
4444
import { unwrapTSNode } from './babelUtils'
4545

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

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

9196
let state = MemberExpLexState.inMemberExp
9297
let stateStack: MemberExpLexState[] = []
@@ -154,15 +159,19 @@ export const isMemberExpressionBrowser = (path: string): boolean => {
154159
}
155160

156161
export const isMemberExpressionNode: (
157-
path: string,
162+
exp: ExpressionNode,
158163
context: TransformContext,
159164
) => boolean = __BROWSER__
160165
? (NOOP as any)
161-
: (path: string, context: TransformContext): boolean => {
166+
: (exp, context) => {
162167
try {
163-
let ret: Expression = parseExpression(path, {
164-
plugins: context.expressionPlugins,
165-
})
168+
let ret: Node =
169+
exp.ast ||
170+
parseExpression(getExpSource(exp), {
171+
plugins: context.expressionPlugins
172+
? [...context.expressionPlugins, 'typescript']
173+
: ['typescript'],
174+
})
166175
ret = unwrapTSNode(ret) as Expression
167176
return (
168177
ret.type === 'MemberExpression' ||
@@ -175,10 +184,52 @@ export const isMemberExpressionNode: (
175184
}
176185

177186
export const isMemberExpression: (
178-
path: string,
187+
exp: ExpressionNode,
179188
context: TransformContext,
180189
) => boolean = __BROWSER__ ? isMemberExpressionBrowser : isMemberExpressionNode
181190

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

0 commit comments

Comments
 (0)