-
-
Notifications
You must be signed in to change notification settings - Fork 8.8k
feat(compiler): evaluate static interpolations at compile time #13617
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
base: main
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -7,6 +7,7 @@ import { | |
type CommentNode, | ||
type CompoundExpressionNode, | ||
type ConditionalExpression, | ||
ConstantTypes, | ||
type ExpressionNode, | ||
type FunctionExpression, | ||
type IfStatement, | ||
|
@@ -32,6 +33,7 @@ import { SourceMapGenerator } from 'source-map-js' | |
import { | ||
advancePositionWithMutation, | ||
assert, | ||
evaluateConstant, | ||
isSimpleIdentifier, | ||
toValidAssetId, | ||
} from './utils' | ||
|
@@ -41,6 +43,7 @@ import { | |
isArray, | ||
isString, | ||
isSymbol, | ||
toDisplayString, | ||
} from '@vue/shared' | ||
import { | ||
CREATE_COMMENT, | ||
|
@@ -760,6 +763,20 @@ function genExpression(node: SimpleExpressionNode, context: CodegenContext) { | |
|
||
function genInterpolation(node: InterpolationNode, context: CodegenContext) { | ||
const { push, helper, pure } = context | ||
|
||
if ( | ||
node.content.type === NodeTypes.SIMPLE_EXPRESSION && | ||
node.content.constType === ConstantTypes.CAN_STRINGIFY | ||
) { | ||
if (node.content.content) { | ||
push(JSON.stringify(toDisplayString(evaluateConstant(node.content)))) | ||
} else { | ||
push(`""`) | ||
} | ||
Comment on lines
+771
to
+775
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. We can't call |
||
|
||
return | ||
} | ||
|
||
if (pure) push(PURE_ANNOTATION) | ||
push(`${helper(TO_DISPLAY_STRING)}(`) | ||
genNode(node.content, context) | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -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. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
|
||
) | ||
|
||
export const transformExpression: NodeTransform = (node, context) => { | ||
if (node.type === NodeTypes.INTERPOLATION) { | ||
|
@@ -119,7 +121,14 @@ export function processExpression( | |
return node | ||
} | ||
|
||
if (!context.prefixIdentifiers || !node.content.trim()) { | ||
if (!node.content.trim()) { | ||
// This allows stringification to continue in the presence of empty | ||
// interpolations. | ||
node.constType = ConstantTypes.CAN_STRINGIFY | ||
return node | ||
} | ||
|
||
if (!context.prefixIdentifiers) { | ||
return node | ||
} | ||
|
||
|
Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
@@ -37,7 +37,13 @@ import { | |||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
TO_HANDLERS, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
WITH_MEMO, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
} from './runtimeHelpers' | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
import { NOOP, isObject, isString } from '@vue/shared' | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
import { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
NOOP, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
isObject, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
isString, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
isSymbol, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
toDisplayString, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
} from '@vue/shared' | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
import type { PropsExpression } from './transforms/transformElement' | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
import { parseExpression } from '@babel/parser' | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
import type { Expression, Node } from '@babel/types' | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
@@ -564,3 +570,32 @@ export function getMemoedVNodeCall( | |||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
} | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
export const forAliasRE: RegExp = /([\s\S]*?)\s+(?:in|of)\s+(\S[\s\S]*)/ | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
// __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 { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Moved - unchanged |
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
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 | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
} | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
} | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
Comment on lines
+574
to
+601
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🛠️ Refactor suggestion Add error handling and improve type safety. The function implementation looks good overall but has a few areas for improvement:
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
Suggested change
🤖 Prompt for AI Agents
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 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. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @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. |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -7,16 +7,17 @@ import { | |
ConstantTypes, | ||
type ElementNode, | ||
ElementTypes, | ||
type ExpressionNode, | ||
type HoistTransform, | ||
Namespaces, | ||
NodeTypes, | ||
type PlainElementNode, | ||
type SimpleExpressionNode, | ||
TO_DISPLAY_STRING, | ||
type TemplateChildNode, | ||
type TextCallNode, | ||
type TransformContext, | ||
createCallExpression, | ||
evaluateConstant, | ||
isStaticArgOf, | ||
} from '@vue/compiler-core' | ||
import { | ||
|
@@ -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. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This isn't strictly necessary (see above comment about unused imports), but doesn't hurt to cleanup. |
||
|
||
if ( | ||
node.content.type === NodeTypes.SIMPLE_EXPRESSION && | ||
!node.content.content | ||
) { | ||
return '' | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Again, can't call |
||
} | ||
|
||
return escapeHtml(toDisplayString(evaluateConstant(node.content))) | ||
case NodeTypes.COMPOUND_EXPRESSION: | ||
return escapeHtml(evaluateConstant(node)) | ||
|
@@ -386,32 +398,3 @@ function stringifyElement( | |
} | ||
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) | ||
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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.