feat(compiler): evaluate static interpolations at compile time #13617
Conversation
WalkthroughThe updates introduce optimizations for static interpolation handling in the compiler. New tests verify that constant and empty interpolations are stringified at compile time. The code generator now inlines constant interpolation values, and a utility for evaluating constant expressions is added and reused. Related stringification logic and tests are updated accordingly. Changes
Sequence Diagram(s)sequenceDiagram
participant Parser
participant Transformer
participant Codegen
participant Utils
Parser->>Transformer: Parse template with interpolation
Transformer->>Transformer: Process expression, mark as constant if applicable
Transformer->>Codegen: Pass InterpolationNode
Codegen->>Utils: evaluateConstant(expression)
Utils-->>Codegen: Stringified constant value
Codegen->>Codegen: Inline string literal if constant
Codegen-->>Output: Generated code with inlined value
Suggested labels
Suggested reviewers
Poem
✨ Finishing Touches
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
| @@ -148,7 +148,7 @@ return function render(_ctx, _cache) { | |||
| const { toDisplayString: _toDisplayString, createElementVNode: _createElementVNode, openBlock: _openBlock, createElementBlock: _createElementBlock } = _Vue | |||
There was a problem hiding this comment.
We can't remove the toDisplayString here since the optimisation for this test is performed during codegen, after we've emitted the import.
This shouldn't matter since any unused imports will be tree-shaken by most build tools.
| if (node.content.content) { | ||
| push(JSON.stringify(toDisplayString(evaluateConstant(node.content)))) | ||
| } else { | ||
| push(`""`) | ||
| } |
There was a problem hiding this comment.
We can't call evaluateConstant when node.content is undefined, so we just emit an empty string (which is the behaviour of `toDisplayString(undefined)"
| @@ -44,7 +44,9 @@ import { parseExpression } from '@babel/parser' | |||
| import { IS_REF, UNREF } from '../runtimeHelpers' | |||
| import { BindingTypes } from '../options' | |||
|
|
|||
| const isLiteralWhitelisted = /*@__PURE__*/ makeMap('true,false,null,this') | |||
| const isLiteralWhitelisted = /*@__PURE__*/ makeMap( | |||
| 'true,false,null,undefined,this', | |||
There was a problem hiding this comment.
undefined can be used as a variable name, but we check if it's a variable everywhere we use isLiteralWhitelisted, so I believe this change is sound.
| // in addition, constant exps bail on presence of parens so you can't even | ||
| // run JSFuck in here. But we mark it unsafe for security review purposes. | ||
| // (see compiler-core/src/transforms/transformExpression) | ||
| export function evaluateConstant(exp: ExpressionNode): string { |
| @@ -304,6 +305,17 @@ function stringifyNode( | |||
| case NodeTypes.COMMENT: | |||
| return `<!--${escapeHtml(node.content)}-->` | |||
| case NodeTypes.INTERPOLATION: | |||
| // We add TO_DISPLAY_STRING for every interpolation, so we need to | |||
| // decrease its usage count whenever we remove an interpolation. | |||
| context.removeHelper(TO_DISPLAY_STRING) | |||
There was a problem hiding this comment.
This isn't strictly necessary (see above comment about unused imports), but doesn't hurt to cleanup.
| node.content.type === NodeTypes.SIMPLE_EXPRESSION && | ||
| !node.content.content | ||
| ) { | ||
| return '' |
There was a problem hiding this comment.
Again, can't call evaluateConstant(undefined)
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (3)
packages/compiler-core/__tests__/__snapshots__/codegen.spec.ts.snapis excluded by!**/*.snappackages/compiler-core/__tests__/transforms/__snapshots__/cacheStatic.spec.ts.snapis excluded by!**/*.snappackages/compiler-dom/__tests__/transforms/__snapshots__/stringifyStatic.spec.ts.snapis excluded by!**/*.snap
📒 Files selected for processing (6)
packages/compiler-core/__tests__/codegen.spec.ts(2 hunks)packages/compiler-core/src/codegen.ts(4 hunks)packages/compiler-core/src/transforms/transformExpression.ts(2 hunks)packages/compiler-core/src/utils.ts(2 hunks)packages/compiler-dom/__tests__/transforms/stringifyStatic.spec.ts(1 hunks)packages/compiler-dom/src/transforms/stringifyStatic.ts(2 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (5)
packages/compiler-core/__tests__/codegen.spec.ts (3)
packages/compiler-core/src/ast.ts (3)
InterpolationNode(249-252)locStub(585-589)createSimpleExpression(685-698)packages/compiler-core/src/codegen.ts (1)
generate(284-407)packages/compiler-core/src/index.ts (1)
generate(25-25)
packages/compiler-core/src/utils.ts (4)
packages/compiler-core/src/ast.ts (1)
ExpressionNode(91-91)packages/shared/src/general.ts (2)
isString(51-51)isSymbol(52-52)packages/runtime-core/src/index.ts (1)
toDisplayString(377-377)packages/shared/src/toDisplayString.ts (1)
toDisplayString(24-36)
packages/compiler-core/src/codegen.ts (3)
packages/runtime-core/src/index.ts (1)
toDisplayString(377-377)packages/shared/src/toDisplayString.ts (1)
toDisplayString(24-36)packages/compiler-core/src/utils.ts (1)
evaluateConstant(581-601)
packages/compiler-core/src/transforms/transformExpression.ts (1)
packages/shared/src/index.ts (1)
makeMap(1-1)
packages/compiler-dom/src/transforms/stringifyStatic.ts (1)
packages/compiler-core/src/runtimeHelpers.ts (1)
TO_DISPLAY_STRING(44-46)
🔇 Additional comments (12)
packages/compiler-core/__tests__/codegen.spec.ts (3)
6-6: LGTM! Import addition supports new test cases.The
InterpolationNodetype import is correctly added to support the new test cases that directly construct interpolation nodes.
196-213: Excellent test coverage for static interpolation optimization.The test effectively verifies that complex static expressions are evaluated at compile time and inlined as string literals. The test expression includes diverse literal types (strings, numbers, booleans, null, undefined, template literals) which provides comprehensive coverage of the optimization.
The expected output
"hello1falseundefinednullhi"correctly represents the JavaScript string conversion behavior.
215-228: Good coverage for empty interpolation edge case.This test ensures that empty interpolations are handled gracefully by outputting an empty string literal rather than generating a runtime helper call. This is an important optimization for templates with empty interpolations.
packages/compiler-core/src/codegen.ts (2)
10-10: Appropriate imports for the interpolation optimization.The imports are correctly added to support the new optimization:
ConstantTypesfor checking if expressions can be stringifiedevaluateConstantfor evaluating constant expressions at compile timetoDisplayStringfor consistent string conversion behaviorAlso applies to: 36-36, 46-46
767-779: Well-implemented compile-time interpolation optimization.The optimization correctly:
- Checks if the interpolation content is a simple expression marked as
CAN_STRINGIFY- Handles empty content by outputting an empty string literal
- Uses
evaluateConstantandtoDisplayStringfor consistent evaluation matching runtime behavior- Falls back to the original runtime helper call for non-constant interpolations
The logic preserves the original behavior while adding the optimization, ensuring backward compatibility.
packages/compiler-dom/__tests__/transforms/stringifyStatic.spec.ts (1)
529-561: Comprehensive test for static interpolation stringification.This test effectively validates that static interpolations are stringified equivalently to their static text counterparts. The test design is excellent:
- Comprehensive coverage: Tests various literal types (
"1",`1`,1,false,undefined,null, empty string)- Threshold calculation: Dynamically calculates repetitions needed to trigger stringification
- Equivalence verification: Ensures interpolated and static versions produce identical output
- Snapshot consistency: Maintains output stability
The test correctly maps interpolated expressions to their expected string representations, ensuring the optimization produces correct results.
packages/compiler-dom/src/transforms/stringifyStatic.ts (2)
15-15: Good refactoring to use centralized utilities.The addition of
TO_DISPLAY_STRINGimport andevaluateConstantimport aligns with the broader refactoring to use centralized utilities across the compiler. This improves maintainability and consistency.Also applies to: 20-20
308-318: Proper helper management and empty content handling.The enhanced interpolation handling correctly:
- Helper accounting: Calls
context.removeHelper(TO_DISPLAY_STRING)to decrement the helper usage count when an interpolation is removed during stringification- Empty content guard: Returns empty string for simple expressions with empty content, preventing
evaluateConstantfrom being called on undefined content- Consistent evaluation: Uses the centralized
evaluateConstantutility for reliable constant evaluationThe logic properly handles edge cases while maintaining the optimization's effectiveness.
packages/compiler-core/src/transforms/transformExpression.ts (2)
47-49: Appropriate expansion of literal whitelist.Adding
"undefined"to the literal whitelist is correct and enables the compiler to recognizeundefinedas a stringifiable constant. This supports the static interpolation optimization by allowingundefinedexpressions to be evaluated at compile time.The change is consistent with JavaScript's treatment of
undefinedas a primitive value that can be safely stringified.
124-133: Well-structured early return logic for empty expressions.The reorganization of early return conditions is well-designed:
- Empty content handling: First checks for empty content and marks it as
CAN_STRINGIFYbefore returning- Prefix identifier check: Then checks
context.prefixIdentifiersand returns if false- Optimization enablement: Ensures empty expressions can be optimized even when prefix identifiers is disabled
This change enables the static interpolation optimization to work correctly with empty expressions while maintaining the existing logic flow.
packages/compiler-core/src/utils.ts (2)
40-46: LGTM: Imports for new utility function.The imports are correctly added to support the new
evaluateConstantfunction implementation.
581-581: Inconsistent with past review comment.The past review comment indicates this line was "Moved - unchanged", but this appears to be a new function addition rather than a move operation.
Likely an incorrect or invalid review comment.
| // __UNSAFE__ | ||
| // Reason: eval. | ||
| // It's technically safe to eval because only constant expressions are possible | ||
| // here, e.g. `{{ 1 }}` or `{{ 'foo' }}` | ||
| // in addition, constant exps bail on presence of parens so you can't even | ||
| // run JSFuck in here. But we mark it unsafe for security review purposes. | ||
| // (see compiler-core/src/transforms/transformExpression) | ||
| export function evaluateConstant(exp: ExpressionNode): string { | ||
| if (exp.type === NodeTypes.SIMPLE_EXPRESSION) { | ||
| return new Function(`return (${exp.content})`)() | ||
| } else { | ||
| // compound | ||
| let res = `` | ||
| exp.children.forEach(c => { | ||
| if (isString(c) || isSymbol(c)) { | ||
| return | ||
| } | ||
| if (c.type === NodeTypes.TEXT) { | ||
| res += c.content | ||
| } else if (c.type === NodeTypes.INTERPOLATION) { | ||
| res += toDisplayString(evaluateConstant(c.content)) | ||
| } else { | ||
| res += evaluateConstant(c as ExpressionNode) | ||
| } | ||
| }) | ||
| return res | ||
| } | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Add error handling and improve type safety.
The function implementation looks good overall but has a few areas for improvement:
- Missing error handling: The
new Function()call should be wrapped in try-catch to handle malformed expressions gracefully. - Unsafe type casting: Line 596 uses
c as ExpressionNodewithout type checking, which could lead to runtime errors.
Apply this diff to improve error handling and type safety:
export function evaluateConstant(exp: ExpressionNode): string {
if (exp.type === NodeTypes.SIMPLE_EXPRESSION) {
- return new Function(`return (${exp.content})`)()
+ try {
+ return new Function(`return (${exp.content})`)()
+ } catch (e) {
+ // Fallback for malformed expressions
+ return exp.content
+ }
} else {
// compound
let res = ``
exp.children.forEach(c => {
if (isString(c) || isSymbol(c)) {
return
}
if (c.type === NodeTypes.TEXT) {
res += c.content
} else if (c.type === NodeTypes.INTERPOLATION) {
res += toDisplayString(evaluateConstant(c.content))
} else {
- res += evaluateConstant(c as ExpressionNode)
+ if (isObject(c) && 'type' in c) {
+ res += evaluateConstant(c as ExpressionNode)
+ }
}
})
return res
}
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| // __UNSAFE__ | |
| // Reason: eval. | |
| // It's technically safe to eval because only constant expressions are possible | |
| // here, e.g. `{{ 1 }}` or `{{ 'foo' }}` | |
| // in addition, constant exps bail on presence of parens so you can't even | |
| // run JSFuck in here. But we mark it unsafe for security review purposes. | |
| // (see compiler-core/src/transforms/transformExpression) | |
| export function evaluateConstant(exp: ExpressionNode): string { | |
| if (exp.type === NodeTypes.SIMPLE_EXPRESSION) { | |
| return new Function(`return (${exp.content})`)() | |
| } else { | |
| // compound | |
| let res = `` | |
| exp.children.forEach(c => { | |
| if (isString(c) || isSymbol(c)) { | |
| return | |
| } | |
| if (c.type === NodeTypes.TEXT) { | |
| res += c.content | |
| } else if (c.type === NodeTypes.INTERPOLATION) { | |
| res += toDisplayString(evaluateConstant(c.content)) | |
| } else { | |
| res += evaluateConstant(c as ExpressionNode) | |
| } | |
| }) | |
| return res | |
| } | |
| } | |
| // __UNSAFE__ | |
| // Reason: eval. | |
| // It's technically safe to eval because only constant expressions are possible | |
| // here, e.g. `{{ 1 }}` or `{{ 'foo' }}` | |
| // in addition, constant exps bail on presence of parens so you can't even | |
| // run JSFuck in here. But we mark it unsafe for security review purposes. | |
| // (see compiler-core/src/transforms/transformExpression) | |
| export function evaluateConstant(exp: ExpressionNode): string { | |
| if (exp.type === NodeTypes.SIMPLE_EXPRESSION) { | |
| try { | |
| return new Function(`return (${exp.content})`)() | |
| } catch (e) { | |
| // Fallback for malformed expressions | |
| return exp.content | |
| } | |
| } else { | |
| // compound | |
| let res = `` | |
| exp.children.forEach(c => { | |
| if (isString(c) || isSymbol(c)) { | |
| return | |
| } | |
| if (c.type === NodeTypes.TEXT) { | |
| res += c.content | |
| } else if (c.type === NodeTypes.INTERPOLATION) { | |
| res += toDisplayString(evaluateConstant(c.content)) | |
| } else { | |
| if (isObject(c) && 'type' in c) { | |
| res += evaluateConstant(c as ExpressionNode) | |
| } | |
| } | |
| }) | |
| return res | |
| } | |
| } |
🤖 Prompt for AI Agents
In packages/compiler-core/src/utils.ts between lines 574 and 601, the
evaluateConstant function lacks error handling around the new Function call and
uses unsafe type casting on line 596. To fix this, wrap the new Function
invocation in a try-catch block to gracefully handle any errors from malformed
expressions, returning a safe fallback value or rethrowing as needed.
Additionally, before casting c to ExpressionNode on line 596, add a type check
to ensure c is indeed an ExpressionNode to prevent runtime errors.
There was a problem hiding this comment.
I feel like this util was written in a specific way, for it's specific use-case, so changing it is out-of-scope, but happy to look into it if the human reviewer would like me to
There was a problem hiding this comment.
@CamWass That's a perfectly reasonable approach! You're right that this function was designed for its specific use case within the compiler optimization, and the current implementation aligns with that purpose. The safety measures you've mentioned in the comments (constant expressions only, no parens) do provide reasonable protection for the intended use case.
If a human reviewer feels the error handling should be added, we can always revisit it. For now, keeping the focus on the PR's core objective of compile-time static interpolation evaluation makes sense.
This PR allows the compiler to evaluate static interpolations (and calls to
toDisplayString) at compile time. 'Static' here means that the value is marked asConstantTypes.CAN_STRINGIFY.I found calls to
toDisplayStringwith literal arguments in a real-world, large Vue project. These can be optimised at compile time with little effort, removing a function call from the generated code and improving performance + code-size slightly.This PR:
undefined, as static and handles them when evaluating at compile time. This allows them to be stringified.toDisplayStringduring codegen. This catches static calls that couldn't be strinngified (most).Example input:
Output before this PR (plaground link):
Output after this PR:
Summary by CodeRabbit
New Features
Bug Fixes
Refactor