feat(compiler): Implement constant propagation for template literals #29716
Conversation
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
|
Comparing: d77dd31...cee1f11 Critical size changesIncludes critical production bundles, as well as any change greater than 2%:
Significant size changesIncludes any change greater than 0.2%: Expand to show
|
nikeee
force-pushed
the
template-string-inlining
branch
from
f28ead6 to
8bfecfc
Compare
| } | ||
|
|
||
| const suffix = value.quasis[quasiIndex].cooked; | ||
| resultString += subExprValue.value.toString() + suffix; |
There was a problem hiding this comment.
can you double-check the spec behavior here? is it calling toString() or something else? would be great to link to the appropriate point in the spec for posterity.
There was a problem hiding this comment.
This is a good concern. I've looked it up:
https://262.ecma-international.org/6.0/#sec-template-literals-runtime-semantics-evaluation
It uses string coercion (defined as ToString), wich is defined here: https://262.ecma-international.org/6.0/#sec-tostring
We only deal with string/number/boolean in the case above. So I think the only case that would be non-trivial is ToString(number), which is defined here. This is the definition of Number.prototype.toString(), which - in case of radix = 10 - just returns ToString(number).
If I'm not mistaking, it matches the spec as long as the values are number/string/booleans. It should be a different story when it is null/undefined (which is excluded via the return above).
In the note it says that the coercion basically behaves like when using String.prototype.concat, which is also the way the TS compiler implements this when lowering to ES3/ES5 (they seem to assume that concat operation is not monkey-patched).
Is this fine or should we just handle the cases where it's trivially always a string? More sophisticated stuff can always be added later. What do you think?
compiler/packages/babel-plugin-react-compiler/src/Optimization/ConstantPropagation.ts
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
My apologies, I overlooked this one. Thanks for the PR! This makes sense and seems reasonable to include, see comments just to make sure the behavior is identical to the spec. We should also test more edge cases: where the interpolation is at the beginning or end of the string, where there are multiple interpolations without any intervening text, with odd values like NaN, empty string, etc.
nikeee
force-pushed
the
template-string-inlining
branch
from
8bfecfc to
cee1f11
Compare
I agree, will do! |
Template literals consisting entirely of constant values will be inlined to a string literal,
effectively replacing the backticks with a double quote.
This is done primarily to make the resulting instruction a string literal, so it can be
processed further in constant propatation. So this is now correctly simplified to `true`:
```js
`` === "" // now true
`a${1}` === "a1"
```
If a template literal contains a non-constant value, the entire literal is left as-is.
Transforming a template literal to one that has only partially inlined is something
that can be improved upon in the future.
nikeee
force-pushed
the
template-string-inlining
branch
from
cee1f11 to
6ef9564
Compare
|
This pull request has been automatically marked as stale. If this pull request is still relevant, please leave any comment (for example, "bump"), and we'll keep it open. We are sorry that we haven't been able to prioritize reviewing it yet. Your contribution is very much appreciated. |
github-actions
bot
added
the
Resolution: Stale
|
Closing this pull request after a prolonged period of inactivity. If this issue is still present in the latest release, please ask for this pull request to be reopened. Thank you! |
…33139) New take on #29716 ## Summary Template literals consisting entirely of constant values will be inlined to a string literal, effectively replacing the backticks with a double quote. This is done primarily to make the resulting instruction a string literal, so it can be processed further in constant propatation. So this is now correctly simplified to `true`: ```js `` === "" // now true `a${1}` === "a1" // now true ``` If a template string literal can only partially be comptime-evaluated, it is not that useful for dead code elimination or further constant folding steps and thus, is left as-is in that case. Same is true if the literal contains an array, object, symbol or function. ## How did you test this change? See added tests.
…33139) New take on #29716 ## Summary Template literals consisting entirely of constant values will be inlined to a string literal, effectively replacing the backticks with a double quote. This is done primarily to make the resulting instruction a string literal, so it can be processed further in constant propatation. So this is now correctly simplified to `true`: ```js `` === "" // now true `a${1}` === "a1" // now true ``` If a template string literal can only partially be comptime-evaluated, it is not that useful for dead code elimination or further constant folding steps and thus, is left as-is in that case. Same is true if the literal contains an array, object, symbol or function. ## How did you test this change? See added tests. DiffTrain build for [ac06829](ac06829)
…33139) New take on #29716 ## Summary Template literals consisting entirely of constant values will be inlined to a string literal, effectively replacing the backticks with a double quote. This is done primarily to make the resulting instruction a string literal, so it can be processed further in constant propatation. So this is now correctly simplified to `true`: ```js `` === "" // now true `a${1}` === "a1" // now true ``` If a template string literal can only partially be comptime-evaluated, it is not that useful for dead code elimination or further constant folding steps and thus, is left as-is in that case. Same is true if the literal contains an array, object, symbol or function. ## How did you test this change? See added tests. DiffTrain build for [ac06829](ac06829)
…acebook#33139) New take on facebook#29716 ## Summary Template literals consisting entirely of constant values will be inlined to a string literal, effectively replacing the backticks with a double quote. This is done primarily to make the resulting instruction a string literal, so it can be processed further in constant propatation. So this is now correctly simplified to `true`: ```js `` === "" // now true `a${1}` === "a1" // now true ``` If a template string literal can only partially be comptime-evaluated, it is not that useful for dead code elimination or further constant folding steps and thus, is left as-is in that case. Same is true if the literal contains an array, object, symbol or function. ## How did you test this change? See added tests. DiffTrain build for [ac06829](facebook@ac06829)
…acebook#33139) New take on facebook#29716 ## Summary Template literals consisting entirely of constant values will be inlined to a string literal, effectively replacing the backticks with a double quote. This is done primarily to make the resulting instruction a string literal, so it can be processed further in constant propatation. So this is now correctly simplified to `true`: ```js `` === "" // now true `a${1}` === "a1" // now true ``` If a template string literal can only partially be comptime-evaluated, it is not that useful for dead code elimination or further constant folding steps and thus, is left as-is in that case. Same is true if the literal contains an array, object, symbol or function. ## How did you test this change? See added tests. DiffTrain build for [ac06829](facebook@ac06829)
Template literals consisting entirely of constant values will be inlined to a string literal, effectively replacing the backticks with a double quote.
This is done primarily to make the resulting instruction a string literal, so it can be processed further in constant propatation. So this is now correctly simplified to
true:If a template literal contains a non-constant value, the entire literal is left as-is. Transforming a template literal to one that has only partially inlined is something that can be improved upon in the future.