Skip to content

fix(compiler-vapor): handle cache variable name substring edge cases #13520

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 2 commits into from
Jun 26, 2025
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -113,6 +113,21 @@ export function render(_ctx) {
}"
`;

exports[`cache multiple access > object property name substring cases 1`] = `
"import { setProp as _setProp, renderEffect as _renderEffect, template as _template } from 'vue';
const t0 = _template("<div></div>", true)
export function render(_ctx) {
const n0 = t0()
_renderEffect(() => {
const _p = _ctx.p
const _p_title = _p.title
_setProp(n0, "id", _p_title + _p.titles + _p_title)
})
return n0
}"
`;

exports[`cache multiple access > optional chaining 1`] = `
"import { setProp as _setProp, renderEffect as _renderEffect, template as _template } from 'vue';
const t0 = _template("<div></div>", true)
Expand Down Expand Up @@ -194,6 +209,20 @@ export function render(_ctx) {
}"
`;
exports[`cache multiple access > variable name substring edge cases 1`] = `
"import { setProp as _setProp, renderEffect as _renderEffect, template as _template } from 'vue';
const t0 = _template("<div></div>", true)
export function render(_ctx) {
const n0 = t0()
_renderEffect(() => {
const _title = _ctx.title
_setProp(n0, "id", _title + _ctx.titles + _title)
})
return n0
}"
`;
exports[`compiler v-bind > .attr modifier 1`] = `
"import { setAttr as _setAttr, renderEffect as _renderEffect, template as _template } from 'vue';
const t0 = _template("<div></div>", true)
Expand Down
19 changes: 19 additions & 0 deletions packages/compiler-vapor/__tests__/transforms/vBind.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -785,6 +785,25 @@ describe('cache multiple access', () => {
expect(code).contains('_setProp(n0, "id", _obj[1][_ctx.baz] + _obj.bar)')
})

test('variable name substring edge cases', () => {
const { code } = compileWithVBind(
`<div :id="title + titles + title"></div>`,
)
expect(code).matchSnapshot()
expect(code).contains('const _title = _ctx.title')
expect(code).contains('_setProp(n0, "id", _title + _ctx.titles + _title)')
})

test('object property name substring cases', () => {
const { code } = compileWithVBind(
`<div :id="p.title + p.titles + p.title"></div>`,
)
expect(code).matchSnapshot()
expect(code).contains('const _p = _ctx.p')
expect(code).contains('const _p_title = _p.title')
expect(code).contains('_setProp(n0, "id", _p_title + _p.titles + _p_title)')
})

test('cache variable used in both property shorthand and normal binding', () => {
const { code } = compileWithVBind(`
<div :style="{color}" :id="color"/>
Expand Down
104 changes: 86 additions & 18 deletions packages/compiler-vapor/src/generators/expression.ts
Original file line number Diff line number Diff line change
Expand Up @@ -283,14 +283,21 @@ export function processExpressions(
function analyzeExpressions(expressions: SimpleExpressionNode[]) {
const seenVariable: Record<string, number> = Object.create(null)
const variableToExpMap = new Map<string, Set<SimpleExpressionNode>>()
const expToVariableMap = new Map<SimpleExpressionNode, string[]>()
const expToVariableMap = new Map<
SimpleExpressionNode,
Array<{
name: string
loc?: { start: number; end: number }
}>
>()
const seenIdentifier = new Set<string>()
const updatedVariable = new Set<string>()

const registerVariable = (
name: string,
exp: SimpleExpressionNode,
isIdentifier: boolean,
loc?: { start: number; end: number },
parentStack: Node[] = [],
) => {
if (isIdentifier) seenIdentifier.add(name)
Expand All @@ -299,7 +306,11 @@ function analyzeExpressions(expressions: SimpleExpressionNode[]) {
name,
(variableToExpMap.get(name) || new Set()).add(exp),
)
expToVariableMap.set(exp, (expToVariableMap.get(exp) || []).concat(name))

const variables = expToVariableMap.get(exp) || []
variables.push({ name, loc })
expToVariableMap.set(exp, variables)

if (
parentStack.some(
p => p.type === 'UpdateExpression' || p.type === 'AssignmentExpression',
Expand All @@ -317,12 +328,27 @@ function analyzeExpressions(expressions: SimpleExpressionNode[]) {

walkIdentifiers(exp.ast, (currentNode, parent, parentStack) => {
if (parent && isMemberExpression(parent)) {
const memberExp = extractMemberExpression(parent, name => {
registerVariable(name, exp, true)
const memberExp = extractMemberExpression(parent, id => {
registerVariable(id.name, exp, true, {
start: id.start!,
end: id.end!,
})
})
registerVariable(memberExp, exp, false, parentStack)
registerVariable(
memberExp,
exp,
false,
{ start: parent.start!, end: parent.end! },
parentStack,
)
} else if (!parentStack.some(isMemberExpression)) {
registerVariable(currentNode.name, exp, true, parentStack)
registerVariable(
currentNode.name,
exp,
true,
{ start: currentNode.start!, end: currentNode.end! },
parentStack,
)
}
})
}
Expand All @@ -340,11 +366,22 @@ function processRepeatedVariables(
context: CodegenContext,
seenVariable: Record<string, number>,
variableToExpMap: Map<string, Set<SimpleExpressionNode>>,
expToVariableMap: Map<SimpleExpressionNode, string[]>,
expToVariableMap: Map<
SimpleExpressionNode,
Array<{ name: string; loc?: { start: number; end: number } }>
>,
seenIdentifier: Set<string>,
updatedVariable: Set<string>,
): DeclarationValue[] {
const declarations: DeclarationValue[] = []
const expToReplacementMap = new Map<
SimpleExpressionNode,
Array<{
name: string
locs: { start: number; end: number }[]
}>
>()

for (const [name, exps] of variableToExpMap) {
if (updatedVariable.has(name)) continue
if (seenVariable[name] > 1 && exps.size > 0) {
Expand All @@ -356,12 +393,20 @@ function processRepeatedVariables(
// e.g., foo[baz] -> foo_baz.
// for identifiers, we don't need to replace the content - they will be
// replaced during context.withId(..., ids)
const replaceRE = new RegExp(escapeRegExp(name), 'g')
exps.forEach(node => {
if (node.ast) {
node.content = node.content.replace(replaceRE, varName)
// re-parse the expression
node.ast = parseExp(context, node.content)
if (node.ast && varName !== name) {
const replacements = expToReplacementMap.get(node) || []
replacements.push({
name: varName,
locs: expToVariableMap.get(node)!.reduce(
(locs, v) => {
if (v.name === name && v.loc) locs.push(v.loc)
return locs
},
[] as { start: number; end: number }[],
),
})
expToReplacementMap.set(node, replacements)
}
})

Expand All @@ -384,15 +429,35 @@ function processRepeatedVariables(
}
}

for (const [exp, replacements] of expToReplacementMap) {
replacements
.flatMap(({ name, locs }) =>
locs.map(({ start, end }) => ({ start, end, name })),
)
.sort((a, b) => b.end - a.end)
.forEach(({ start, end, name }) => {
exp.content =
exp.content.slice(0, start - 1) + name + exp.content.slice(end - 1)
})

// re-parse the expression
exp.ast = parseExp(context, exp.content)
}

return declarations
}

function shouldDeclareVariable(
name: string,
expToVariableMap: Map<SimpleExpressionNode, string[]>,
expToVariableMap: Map<
SimpleExpressionNode,
Array<{ name: string; loc?: { start: number; end: number } }>
>,
exps: Set<SimpleExpressionNode>,
): boolean {
const vars = Array.from(exps, exp => expToVariableMap.get(exp)!)
const vars = Array.from(exps, exp =>
expToVariableMap.get(exp)!.map(v => v.name),
)
// assume name equals to `foo`
// if each expression only references `foo`, declaration is needed
// to avoid reactivity tracking
Expand Down Expand Up @@ -439,12 +504,15 @@ function processRepeatedExpressions(
expressions: SimpleExpressionNode[],
varDeclarations: DeclarationValue[],
updatedVariable: Set<string>,
expToVariableMap: Map<SimpleExpressionNode, string[]>,
expToVariableMap: Map<
SimpleExpressionNode,
Array<{ name: string; loc?: { start: number; end: number } }>
>,
): DeclarationValue[] {
const declarations: DeclarationValue[] = []
const seenExp = expressions.reduce(
(acc, exp) => {
const variables = expToVariableMap.get(exp)
const variables = expToVariableMap.get(exp)!.map(v => v.name)
// only handle expressions that are not identifiers
if (
exp.ast &&
Expand Down Expand Up @@ -572,12 +640,12 @@ function genVarName(exp: string): string {

function extractMemberExpression(
exp: Node,
onIdentifier: (name: string) => void,
onIdentifier: (id: Identifier) => void,
): string {
if (!exp) return ''
switch (exp.type) {
case 'Identifier': // foo[bar]
onIdentifier(exp.name)
onIdentifier(exp)
return exp.name
case 'StringLiteral': // foo['bar']
return exp.extra ? (exp.extra.raw as string) : exp.value
Expand Down