Skip to content

Commit f77fd2f

Browse files
committed
fix(compiler-vapor): handle variable name substring edge cases
1 parent c86bf7b commit f77fd2f

File tree

3 files changed

+134
-18
lines changed

3 files changed

+134
-18
lines changed

packages/compiler-vapor/__tests__/transforms/__snapshots__/vBind.spec.ts.snap

Lines changed: 29 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -113,6 +113,21 @@ export function render(_ctx) {
113113
}"
114114
`;
115115

116+
exports[`cache multiple access > object property name substring cases 1`] = `
117+
"import { setProp as _setProp, renderEffect as _renderEffect, template as _template } from 'vue';
118+
const t0 = _template("<div></div>", true)
119+
120+
export function render(_ctx) {
121+
const n0 = t0()
122+
_renderEffect(() => {
123+
const _p = _ctx.p
124+
const _p_title = _p.title
125+
_setProp(n0, "id", _p_title + _p.titles + _p_title)
126+
})
127+
return n0
128+
}"
129+
`;
130+
116131
exports[`cache multiple access > repeated expression in expressions 1`] = `
117132
"import { setProp as _setProp, renderEffect as _renderEffect, template as _template } from 'vue';
118133
const t0 = _template("<div></div>")
@@ -180,6 +195,20 @@ export function render(_ctx) {
180195
}"
181196
`;
182197

198+
exports[`cache multiple access > variable name substring edge cases 1`] = `
199+
"import { setProp as _setProp, renderEffect as _renderEffect, template as _template } from 'vue';
200+
const t0 = _template("<div></div>", true)
201+
202+
export function render(_ctx) {
203+
const n0 = t0()
204+
_renderEffect(() => {
205+
const _title = _ctx.title
206+
_setProp(n0, "id", _title + _ctx.titles + _title)
207+
})
208+
return n0
209+
}"
210+
`;
211+
183212
exports[`compiler v-bind > .attr modifier 1`] = `
184213
"import { setAttr as _setAttr, renderEffect as _renderEffect, template as _template } from 'vue';
185214
const t0 = _template("<div></div>", true)

packages/compiler-vapor/__tests__/transforms/vBind.spec.ts

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -785,6 +785,25 @@ describe('cache multiple access', () => {
785785
expect(code).contains('_setProp(n0, "id", _obj[1][_ctx.baz] + _obj.bar)')
786786
})
787787

788+
test('variable name substring edge cases', () => {
789+
const { code } = compileWithVBind(
790+
`<div :id="title + titles + title"></div>`,
791+
)
792+
expect(code).matchSnapshot()
793+
expect(code).contains('const _title = _ctx.title')
794+
expect(code).contains('_setProp(n0, "id", _title + _ctx.titles + _title)')
795+
})
796+
797+
test('object property name substring cases', () => {
798+
const { code } = compileWithVBind(
799+
`<div :id="p.title + p.titles + p.title"></div>`,
800+
)
801+
expect(code).matchSnapshot()
802+
expect(code).contains('const _p = _ctx.p')
803+
expect(code).contains('const _p_title = _p.title')
804+
expect(code).contains('_setProp(n0, "id", _p_title + _p.titles + _p_title)')
805+
})
806+
788807
test('cache variable used in both property shorthand and normal binding', () => {
789808
const { code } = compileWithVBind(`
790809
<div :style="{color}" :id="color"/>

packages/compiler-vapor/src/generators/expression.ts

Lines changed: 86 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -283,14 +283,21 @@ export function processExpressions(
283283
function analyzeExpressions(expressions: SimpleExpressionNode[]) {
284284
const seenVariable: Record<string, number> = Object.create(null)
285285
const variableToExpMap = new Map<string, Set<SimpleExpressionNode>>()
286-
const expToVariableMap = new Map<SimpleExpressionNode, string[]>()
286+
const expToVariableMap = new Map<
287+
SimpleExpressionNode,
288+
Array<{
289+
name: string
290+
loc?: { start: number; end: number }
291+
}>
292+
>()
287293
const seenIdentifier = new Set<string>()
288294
const updatedVariable = new Set<string>()
289295

290296
const registerVariable = (
291297
name: string,
292298
exp: SimpleExpressionNode,
293299
isIdentifier: boolean,
300+
loc?: { start: number; end: number },
294301
parentStack: Node[] = [],
295302
) => {
296303
if (isIdentifier) seenIdentifier.add(name)
@@ -299,7 +306,11 @@ function analyzeExpressions(expressions: SimpleExpressionNode[]) {
299306
name,
300307
(variableToExpMap.get(name) || new Set()).add(exp),
301308
)
302-
expToVariableMap.set(exp, (expToVariableMap.get(exp) || []).concat(name))
309+
310+
const variables = expToVariableMap.get(exp) || []
311+
variables.push({ name, loc })
312+
expToVariableMap.set(exp, variables)
313+
303314
if (
304315
parentStack.some(
305316
p => p.type === 'UpdateExpression' || p.type === 'AssignmentExpression',
@@ -317,12 +328,27 @@ function analyzeExpressions(expressions: SimpleExpressionNode[]) {
317328

318329
walkIdentifiers(exp.ast, (currentNode, parent, parentStack) => {
319330
if (parent && isMemberExpression(parent)) {
320-
const memberExp = extractMemberExpression(parent, name => {
321-
registerVariable(name, exp, true)
331+
const memberExp = extractMemberExpression(parent, id => {
332+
registerVariable(id.name, exp, true, {
333+
start: id.start!,
334+
end: id.end!,
335+
})
322336
})
323-
registerVariable(memberExp, exp, false, parentStack)
337+
registerVariable(
338+
memberExp,
339+
exp,
340+
false,
341+
{ start: parent.start!, end: parent.end! },
342+
parentStack,
343+
)
324344
} else if (!parentStack.some(isMemberExpression)) {
325-
registerVariable(currentNode.name, exp, true, parentStack)
345+
registerVariable(
346+
currentNode.name,
347+
exp,
348+
true,
349+
{ start: currentNode.start!, end: currentNode.end! },
350+
parentStack,
351+
)
326352
}
327353
})
328354
}
@@ -340,11 +366,22 @@ function processRepeatedVariables(
340366
context: CodegenContext,
341367
seenVariable: Record<string, number>,
342368
variableToExpMap: Map<string, Set<SimpleExpressionNode>>,
343-
expToVariableMap: Map<SimpleExpressionNode, string[]>,
369+
expToVariableMap: Map<
370+
SimpleExpressionNode,
371+
Array<{ name: string; loc?: { start: number; end: number } }>
372+
>,
344373
seenIdentifier: Set<string>,
345374
updatedVariable: Set<string>,
346375
): DeclarationValue[] {
347376
const declarations: DeclarationValue[] = []
377+
const expToReplacementMap = new Map<
378+
SimpleExpressionNode,
379+
Array<{
380+
name: string
381+
locs: { start: number; end: number }[]
382+
}>
383+
>()
384+
348385
for (const [name, exps] of variableToExpMap) {
349386
if (updatedVariable.has(name)) continue
350387
if (seenVariable[name] > 1 && exps.size > 0) {
@@ -356,12 +393,20 @@ function processRepeatedVariables(
356393
// e.g., foo[baz] -> foo_baz.
357394
// for identifiers, we don't need to replace the content - they will be
358395
// replaced during context.withId(..., ids)
359-
const replaceRE = new RegExp(escapeRegExp(name), 'g')
360396
exps.forEach(node => {
361-
if (node.ast) {
362-
node.content = node.content.replace(replaceRE, varName)
363-
// re-parse the expression
364-
node.ast = parseExp(context, node.content)
397+
if (node.ast && varName !== name) {
398+
const replacements = expToReplacementMap.get(node) || []
399+
replacements.push({
400+
name: varName,
401+
locs: expToVariableMap.get(node)!.reduce(
402+
(locs, v) => {
403+
if (v.name === name && v.loc) locs.push(v.loc)
404+
return locs
405+
},
406+
[] as { start: number; end: number }[],
407+
),
408+
})
409+
expToReplacementMap.set(node, replacements)
365410
}
366411
})
367412

@@ -384,15 +429,35 @@ function processRepeatedVariables(
384429
}
385430
}
386431

432+
for (const [exp, replacements] of expToReplacementMap) {
433+
replacements
434+
.flatMap(({ name, locs }) =>
435+
locs.map(({ start, end }) => ({ start, end, name })),
436+
)
437+
.sort((a, b) => b.end - a.end)
438+
.forEach(({ start, end, name }) => {
439+
exp.content =
440+
exp.content.slice(0, start - 1) + name + exp.content.slice(end - 1)
441+
})
442+
443+
// re-parse the expression
444+
exp.ast = parseExp(context, exp.content)
445+
}
446+
387447
return declarations
388448
}
389449

390450
function shouldDeclareVariable(
391451
name: string,
392-
expToVariableMap: Map<SimpleExpressionNode, string[]>,
452+
expToVariableMap: Map<
453+
SimpleExpressionNode,
454+
Array<{ name: string; loc?: { start: number; end: number } }>
455+
>,
393456
exps: Set<SimpleExpressionNode>,
394457
): boolean {
395-
const vars = Array.from(exps, exp => expToVariableMap.get(exp)!)
458+
const vars = Array.from(exps, exp =>
459+
expToVariableMap.get(exp)!.map(v => v.name),
460+
)
396461
// assume name equals to `foo`
397462
// if each expression only references `foo`, declaration is needed
398463
// to avoid reactivity tracking
@@ -439,12 +504,15 @@ function processRepeatedExpressions(
439504
expressions: SimpleExpressionNode[],
440505
varDeclarations: DeclarationValue[],
441506
updatedVariable: Set<string>,
442-
expToVariableMap: Map<SimpleExpressionNode, string[]>,
507+
expToVariableMap: Map<
508+
SimpleExpressionNode,
509+
Array<{ name: string; loc?: { start: number; end: number } }>
510+
>,
443511
): DeclarationValue[] {
444512
const declarations: DeclarationValue[] = []
445513
const seenExp = expressions.reduce(
446514
(acc, exp) => {
447-
const variables = expToVariableMap.get(exp)
515+
const variables = expToVariableMap.get(exp)!.map(v => v.name)
448516
// only handle expressions that are not identifiers
449517
if (
450518
exp.ast &&
@@ -572,12 +640,12 @@ function genVarName(exp: string): string {
572640

573641
function extractMemberExpression(
574642
exp: Node,
575-
onIdentifier: (name: string) => void,
643+
onIdentifier: (id: Identifier) => void,
576644
): string {
577645
if (!exp) return ''
578646
switch (exp.type) {
579647
case 'Identifier': // foo[bar]
580-
onIdentifier(exp.name)
648+
onIdentifier(exp)
581649
return exp.name
582650
case 'StringLiteral': // foo['bar']
583651
return exp.extra ? (exp.extra.raw as string) : exp.value

0 commit comments

Comments
 (0)