fix: generate code should convert quote on vue template#1464
fix: generate code should convert quote on vue template#1464hexqi merged 4 commits intoopentiny:developfrom
Conversation
WalkthroughRefines Vue SFC attribute generation: double quotes in string attributes are HTML-encoded ( Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
e96d673 to
f45f72c
Compare
…nd unique state key generation 1. 引号转义优化:将 handleExpressionAttrHook 和 handlePrimitiveAttributeHook 中的引号处理逻辑由替换为单引号改为替换为 HTML 实体 "。这确保了在生成的 Vue 模板中,属性值内的双引号能够被正确转义,避免模板解析错误。 2. 增强鲁棒性:在 generateAttribute.js 的循环中增加了 retryCount 限制(100次),防止在生成唯一的 stateKey 时出现潜在的死循环。
f45f72c to
8b4b104
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
packages/vue-generator/src/generator/vue/sfc/generateAttribute.js (1)
80-91:⚠️ Potential issue | 🟠 MajorThe new quote fix still misses
v-modelandt(...)bindings.
:${key}now escapes"correctly, but these two non-JSX branches still interpolate raw double quotes into a double-quoted attribute. Cases likeform["name"]ort('k', {"name":"x"})will still generate invalid SFC template code. Please route them through the same escape step instead of keeping separate formatting paths.Possible fix
+const escapeTemplateAttrValue = (value = '') => `${value}`.replaceAll(/"/g, '"') + const handleJSExpressionBinding = (key, value, isJSX) => { const expressValue = value.value.replace(isJSX ? thisRegexp : thisPropsBindRe, '') if (isJSX) { return `${key}={${expressValue}}` } // 支持带参数的 v-model if (value.model) { const modelArgs = value.model?.prop ? `:${value.model.prop}` : '' - return `v-model${modelArgs}="${expressValue}"` + return `v-model${modelArgs}="${escapeTemplateAttrValue(expressValue)}"` } // expression 使用 v-bind 绑定 - return `:${key}="${expressValue.replaceAll(/"/g, '"')}"` + return `:${key}="${escapeTemplateAttrValue(expressValue)}"` } const handleBindI18n = (key, value, isJSX) => { const tArguments = [`'${value.key}'`] // TODO: 拿到场景用例 const i18nParams = JSON.stringify(value.params) @@ if (isJSX) { return `${key}={t(${tArguments.join(',')})}` } - return `:${key}="t(${tArguments.join(',')})"` + const bindingExpr = `t(${tArguments.join(',')})` + return `:${key}="${escapeTemplateAttrValue(bindingExpr)}"` }Also applies to: 94-107
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/vue-generator/src/generator/vue/sfc/generateAttribute.js` around lines 80 - 91, The v-model branch and other non-JSX branches (including t(...) bindings) are returning raw expressValue with unescaped double quotes; compute a single escapedExpressValue = expressValue.replaceAll(/"/g, '"') and use that in the v-model return (when value.model is true) and any other template-attribute returns (the `:${key}` path and the t(...) handling around lines 94-107) instead of expressValue so form["name"] and t('k', {"name":"x"}) produce valid quoted attributes; reference the existing symbols expressValue, value.model, and key when making the change.
🧹 Nitpick comments (1)
packages/vue-generator/test/testcases/sfc/templateQuote/templateQuote.test.js (1)
11-49: Missing regression case for the retry-limit guard path.This suite covers quote rendering well, but it doesn’t appear to validate the new state-key retry cap behavior from this PR. Please add one case that forces repeated key collisions and asserts generation completes deterministically (no hang, stable output).
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/vue-generator/test/testcases/sfc/templateQuote/templateQuote.test.js` around lines 11 - 49, Add a new test that triggers the state-key retry-limit guard by forcing repeated key collisions and asserting generation completes deterministically; use genSFCWithDefaultPlugin to generate from a specially-crafted schema that reuses the same state keys repeatedly (or mocks the key allocator to always collide) and then call formatCode(res, 'vue') and expect the formatted output toMatchFileSnapshot (or assert non-hanging/stable string output) so the retry-cap branch in the key allocation logic is exercised; name the test clearly (e.g., "should cap retries and complete deterministically on state-key collisions") and reuse the existing test scaffolding (genSFCWithDefaultPlugin, formatCode, toMatchFileSnapshot) so it integrates with the suite.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@packages/vue-generator/src/generator/vue/sfc/generateAttribute.js`:
- Around line 582-592: The code currently always pushes
attributes.push(`:${key}="state.${stateKey}"`) even if globalHooks.addState
never succeeded; update the block in the shouldBindToState && !isJSX branch
(variables: stateKey, addSuccess, retryCount, globalHooks.addState) to check
addSuccess after the retry loop and only emit the bound attribute when
addSuccess is true; if addSuccess is false after 100 attempts, emit a safe
fallback (e.g., the original non-bound attribute or a literal value derived from
res) or skip adding the binding entirely so the generated code does not
reference an unregistered state key. Ensure any fallback preserves original
behavior when binding fails.
---
Outside diff comments:
In `@packages/vue-generator/src/generator/vue/sfc/generateAttribute.js`:
- Around line 80-91: The v-model branch and other non-JSX branches (including
t(...) bindings) are returning raw expressValue with unescaped double quotes;
compute a single escapedExpressValue = expressValue.replaceAll(/"/g, '"')
and use that in the v-model return (when value.model is true) and any other
template-attribute returns (the `:${key}` path and the t(...) handling around
lines 94-107) instead of expressValue so form["name"] and t('k', {"name":"x"})
produce valid quoted attributes; reference the existing symbols expressValue,
value.model, and key when making the change.
---
Nitpick comments:
In
`@packages/vue-generator/test/testcases/sfc/templateQuote/templateQuote.test.js`:
- Around line 11-49: Add a new test that triggers the state-key retry-limit
guard by forcing repeated key collisions and asserting generation completes
deterministically; use genSFCWithDefaultPlugin to generate from a
specially-crafted schema that reuses the same state keys repeatedly (or mocks
the key allocator to always collide) and then call formatCode(res, 'vue') and
expect the formatted output toMatchFileSnapshot (or assert non-hanging/stable
string output) so the retry-cap branch in the key allocation logic is exercised;
name the test clearly (e.g., "should cap retries and complete deterministically
on state-key collisions") and reuse the existing test scaffolding
(genSFCWithDefaultPlugin, formatCode, toMatchFileSnapshot) so it integrates with
the suite.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: c312a6c7-449c-4012-9f90-b9be73ebe1d2
📒 Files selected for processing (17)
packages/vue-generator/src/generator/vue/sfc/generateAttribute.jspackages/vue-generator/test/testcases/sfc/accessor/expected/Accessor.vuepackages/vue-generator/test/testcases/sfc/accessor/schema.jsonpackages/vue-generator/test/testcases/sfc/case01/expected/FormTable.vuepackages/vue-generator/test/testcases/sfc/templateQuote/components-map.jsonpackages/vue-generator/test/testcases/sfc/templateQuote/expected/jsxQuote.vuepackages/vue-generator/test/testcases/sfc/templateQuote/expected/multiline.vuepackages/vue-generator/test/testcases/sfc/templateQuote/expected/primitiveQuote.vuepackages/vue-generator/test/testcases/sfc/templateQuote/expected/quoteScope.vuepackages/vue-generator/test/testcases/sfc/templateQuote/expected/templateQuote.vuepackages/vue-generator/test/testcases/sfc/templateQuote/jsxQuote.components-map.jsonpackages/vue-generator/test/testcases/sfc/templateQuote/jsxQuote.schema.jsonpackages/vue-generator/test/testcases/sfc/templateQuote/multiline.schema.jsonpackages/vue-generator/test/testcases/sfc/templateQuote/page.schema.jsonpackages/vue-generator/test/testcases/sfc/templateQuote/primitiveQuote.schema.jsonpackages/vue-generator/test/testcases/sfc/templateQuote/scope.schema.jsonpackages/vue-generator/test/testcases/sfc/templateQuote/templateQuote.test.js
packages/vue-generator/src/generator/vue/sfc/generateAttribute.js
Outdated
Show resolved
Hide resolved
|
@coderabbitai full review |
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
packages/vue-generator/src/generator/vue/sfc/generateAttribute.js (1)
524-529:⚠️ Potential issue | 🟠 MajorEscape object keys with special characters to prevent generated code syntax errors.
The code generates JavaScript object literals in
{ ${resStr.join(',')} }. Currently, non-identifier keys use simple string interpolation'${key}':, which breaks when keys contain quotes or backslashes. Keys like"foo'bar"produce invalid syntax:{ 'foo'bar': 1 }. While string values are properly escaped (line 459–465), object keys lack this protection.Use
JSON.stringify(key)to safely escape all special characters:Suggested fix
let renderKey = shouldRenderKey ? // 如果 key 是合法的变量名,则直接使用 key: 的形式,否则需要添加引号,防止不合法的 key 导致语法错误 normalKeyRegexp.test(key) ? `${key}: ` - : `'${key}': ` + : `${JSON.stringify(key)}: ` : ''🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/vue-generator/src/generator/vue/sfc/generateAttribute.js` around lines 524 - 529, The object-key rendering can produce invalid JS when keys contain quotes or backslashes; update the branch in the loop that builds keys (the code using normalKeyRegexp, shouldRenderKey and the obj iteration that pushes into resStr) to escape keys with JSON.stringify instead of raw `'${key}'`: when the key is not a valid identifier use `${JSON.stringify(key)}: ` so keys like "foo'bar" become safe string-literal keys; keep the existing normalKeyRegexp.test(key) path for identifier-like keys.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@packages/vue-generator/src/generator/vue/sfc/generateAttribute.js`:
- Around line 88-91: The template attribute escaping only replaces " with "
and fails to escape & first, causing entity decoding issues; update all binding
paths (handleJSExpressionBinding, the v-for source binding, the state binding
fallback, the object binding fallback, and the primitive string attribute
handling) to first replaceAll('&' , '&') and then replaceAll('"', '"')
(i.e., escape & before ") for any expressValue used in attribute generation so
v-bind expressions, v-for sources and primitive attributes are safe from HTML
entity decoding changes.
---
Outside diff comments:
In `@packages/vue-generator/src/generator/vue/sfc/generateAttribute.js`:
- Around line 524-529: The object-key rendering can produce invalid JS when keys
contain quotes or backslashes; update the branch in the loop that builds keys
(the code using normalKeyRegexp, shouldRenderKey and the obj iteration that
pushes into resStr) to escape keys with JSON.stringify instead of raw
`'${key}'`: when the key is not a valid identifier use `${JSON.stringify(key)}:
` so keys like "foo'bar" become safe string-literal keys; keep the existing
normalKeyRegexp.test(key) path for identifier-like keys.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: 85a6cab1-5ebc-4e8a-adec-0c21c9246fd9
📒 Files selected for processing (1)
packages/vue-generator/src/generator/vue/sfc/generateAttribute.js
English | 简体中文
PR
PR Checklist
Please check if your PR fulfills the following requirements:
PR Type
What kind of change does this PR introduce?
【问题描述】Background and solution
vue-generator 在生成 Vue 模板属性时,通过
"→'的朴素替换来避免双引号与 HTML 属性分隔符冲突。当属性值同时包含单引号和双引号(或在 v-bind 上下文中包含双引号)时,这种策略会生成无法运行的代码。问题 1:v-bind 对象绑定生成无效 JS
transformObjValue先将值内的"转为',handleObjBindAttrHook再将外层包裹的"也转为',导致内层未转义的'与外层定界符'冲突。输入 schema:
props.theme = { defaultValue: '{"class": "test-class", "id": "test-id"}' }
transformObjValue 第一步 → 值内 " 变 ':
defaultValue: "{'class': 'test-class', 'id': 'test-id'}"
handleObjBindAttrHook 第二步 → 外层 " 也变 ':
defaultValue: '{'class': 'test-class', 'id': 'test-id'}'
↑ 内层 ' 没有转义,与外层 ' 冲突
最终生成:
:theme="{ defaultValue: '{'class': 'test-class', 'id': 'test-id'}' }"
↑ Vue 执行这段 JS 表达式 → 语法错误 ❌
如果值同时含
"和'(如{"class": "te'st'-class"}),问题更严重——转义后的\'和未转义的'混杂,产生不可预测的解析结果。问题 2:JSExpression 中的双引号截断 HTML 属性
handleJSExpressionBinding旧代码对表达式值不做任何引号处理,表达式中的"直接截断 HTML 属性结构。输入 schema:
props.data = { type: "JSExpression", value: '[{ "name": "test" }]' }
旧代码直接拼接:
:data="[{ "name": "test" }]"
↑ " 截断了 HTML 属性 → 属性值变成 [{ ,后续内容变成无效属性 ❌
问题 3:v-for 循环源含引号时的脆弱转换
handleLoopAttrHook非表达式路径使用JSON.stringify→'转义 →"替换的三步链式处理。虽然JSON.stringify的\"转义让单纯场景意外可用,但当数据同时包含'和"时,多层替换的交互可能产生异常转义序列。
【解决方案】What is the current behavior?
统一使用
"HTML 实体编码替代"→'替换,从根本上避免引号冲突:"在 HTML 属性中被浏览器/Vue 模板编译器自动解码为",不会与属性分隔符冲突"原文)具体修改:
handlePrimitiveAttributeHook"→'"→"handleJSExpressionBinding"→"handleObjBindAttrHook内联属性"→'"→"handleLoopAttrHook非表达式路径JSON.stringify+"→'JSON.stringify+"→"transformObjValue字符串转义'→\'+"→'\→\\、\n→\\n、\r→\\r、"→\"handleObjBindAttrHook状态注册循环while (!addSuccess)无上限retryCount < 100防御修复后的效果
输入: props.theme = { defaultValue: '{"class": "test-class"}' }
transformObjValue → 正确转义:
defaultValue: "{"class": "test-class"}"
handleObjBindAttrHook → HTML 实体编码:
:theme="{ defaultValue: "{"class": "test-class"}" }"
HTML 解码后 Vue 收到的 JS 表达式:
{ defaultValue: "{"class": "test-class"}" } ← 合法 JS ✅
Prettier 格式化最终输出:
:theme="{ defaultValue: '{"class": "test-class"}' }" ✅
Test coverage
新增
templateQuote测试套件(5 个测试用例),覆盖:templateQuoteprimitiveQuotequoteScope"和'的 filter 表达式multiline\n换行的多行字符串在 v-bind 对象绑定中的转义jsxQuote")Issue Number: N/A
What is the new behavior?
Does this PR introduce a breaking change?
Other information
Summary by CodeRabbit
Bug Fixes
Tests