perf: inline references to constant primitive <const> tags#3243
Open
DylanPiercey wants to merge 1 commit into
Open
perf: inline references to constant primitive <const> tags#3243DylanPiercey wants to merge 1 commit into
<const> tags#3243DylanPiercey wants to merge 1 commit into
Conversation
🦋 Changeset detectedLatest commit: 46da484 The changes in this PR will be included in the next version bump. This PR includes changesets to release 1 package
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
|
|
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
WalkthroughThe pull request implements a compile-time optimization for 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #3243 +/- ##
==========================================
+ Coverage 94.18% 94.21% +0.03%
==========================================
Files 382 382
Lines 49796 49889 +93
Branches 3698 3728 +30
==========================================
+ Hits 46900 47004 +104
+ Misses 2870 2859 -11
Partials 26 26 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (1)
packages/runtime-tags/src/translator/core/const.ts (1)
71-93: ⚡ Quick winLet primitive aliases fold too.
!upstreamAliaskeeps a primitive alias on the binding/signal path even afterevaluate()has already proven the initializer is inlineable. That leaves the chained-const case from the PR objective unoptimized.♻️ Proposed fix
- if ( - !upstreamAlias && - valueExtra.confident && - t.isIdentifier(node.var) && - canInlineValue(valueExtra.computed) - ) { + if ( + valueExtra.confident && + t.isIdentifier(node.var) && + canInlineValue(valueExtra.computed) + ) {🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/runtime-tags/src/translator/core/const.ts` around lines 71 - 93, The condition `!upstreamAlias &&` in the if statement prevents primitive aliases from being constant folded, even when evaluate() has proven they are inlineable. Remove the `!upstreamAlias &&` check from the if statement condition to allow primitive aliases to participate in constant folding along with the other existing checks (valueExtra.confident, t.isIdentifier(node.var), and canInlineValue(valueExtra.computed)).
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@packages/runtime-tags/src/translator/core/const.ts`:
- Around line 71-93: The condition `!upstreamAlias &&` in the if statement
prevents primitive aliases from being constant folded, even when evaluate() has
proven they are inlineable. Remove the `!upstreamAlias &&` check from the if
statement condition to allow primitive aliases to participate in constant
folding along with the other existing checks (valueExtra.confident,
t.isIdentifier(node.var), and canInlineValue(valueExtra.computed)).
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 356f2f47-b102-42ed-8a47-eacb77513bc3
⛔ Files ignored due to path filters (91)
packages/runtime-tags/src/__tests__/fixtures/attr-class-delimited-shapes/__snapshots__/dom.bundle.debug.jsis excluded by!**/__snapshots__/**and included by**packages/runtime-tags/src/__tests__/fixtures/attr-class-delimited-shapes/__snapshots__/dom.bundle.jsis excluded by!**/__snapshots__/**and included by**packages/runtime-tags/src/__tests__/fixtures/attr-class-delimited-shapes/__snapshots__/html.bundle.debug.jsis excluded by!**/__snapshots__/**and included by**packages/runtime-tags/src/__tests__/fixtures/attr-class-delimited-shapes/__snapshots__/html.bundle.jsis excluded by!**/__snapshots__/**and included by**packages/runtime-tags/src/__tests__/fixtures/attr-class-delimited-shapes/__snapshots__/writes.debug.htmlis excluded by!**/__snapshots__/**and included by**packages/runtime-tags/src/__tests__/fixtures/attr-class-delimited-shapes/__snapshots__/writes.htmlis excluded by!**/__snapshots__/**and included by**packages/runtime-tags/src/__tests__/fixtures/basic-unused-ref/__snapshots__/dom.bundle.debug.jsis excluded by!**/__snapshots__/**and included by**packages/runtime-tags/src/__tests__/fixtures/basic-unused-ref/__snapshots__/html.bundle.debug.jsis excluded by!**/__snapshots__/**and included by**packages/runtime-tags/src/__tests__/fixtures/bound-attr-computed-key/__snapshots__/dom.bundle.debug.jsis excluded by!**/__snapshots__/**and included by**packages/runtime-tags/src/__tests__/fixtures/bound-attr-computed-key/__snapshots__/html.bundle.debug.jsis excluded by!**/__snapshots__/**and included by**packages/runtime-tags/src/__tests__/fixtures/bound-attr-computed-key/__snapshots__/html.bundle.jsis excluded by!**/__snapshots__/**and included by**packages/runtime-tags/src/__tests__/fixtures/const-tag-fold/__snapshots__/dom.bundle.debug.jsis excluded by!**/__snapshots__/**and included by**packages/runtime-tags/src/__tests__/fixtures/const-tag-fold/__snapshots__/dom.bundle.jsis excluded by!**/__snapshots__/**and included by**packages/runtime-tags/src/__tests__/fixtures/const-tag-fold/__snapshots__/html.bundle.debug.jsis excluded by!**/__snapshots__/**and included by**packages/runtime-tags/src/__tests__/fixtures/const-tag-fold/__snapshots__/html.bundle.jsis excluded by!**/__snapshots__/**and included by**packages/runtime-tags/src/__tests__/fixtures/const-tag-fold/__snapshots__/render.debug.mdis excluded by!**/__snapshots__/**and included by**packages/runtime-tags/src/__tests__/fixtures/const-tag-fold/__snapshots__/render.mdis excluded by!**/__snapshots__/**and included by**packages/runtime-tags/src/__tests__/fixtures/const-tag-fold/__snapshots__/writes.debug.htmlis excluded by!**/__snapshots__/**and included by**packages/runtime-tags/src/__tests__/fixtures/const-tag-fold/__snapshots__/writes.htmlis excluded by!**/__snapshots__/**and included by**packages/runtime-tags/src/__tests__/fixtures/const-tag-hoist-error/__snapshots__/html.bundle.debug.jsis excluded by!**/__snapshots__/**and included by**packages/runtime-tags/src/__tests__/fixtures/const-tag/__snapshots__/dom.bundle.debug.jsis excluded by!**/__snapshots__/**and included by**packages/runtime-tags/src/__tests__/fixtures/const-tag/__snapshots__/dom.bundle.jsis excluded by!**/__snapshots__/**and included by**packages/runtime-tags/src/__tests__/fixtures/const-tag/__snapshots__/html.bundle.debug.jsis excluded by!**/__snapshots__/**and included by**packages/runtime-tags/src/__tests__/fixtures/const-tag/__snapshots__/html.bundle.jsis excluded by!**/__snapshots__/**and included by**packages/runtime-tags/src/__tests__/fixtures/const-tag/__snapshots__/writes.htmlis excluded by!**/__snapshots__/**and included by**packages/runtime-tags/src/__tests__/fixtures/controllable-select-dynamic-spread/__snapshots__/dom.bundle.debug.jsis excluded by!**/__snapshots__/**and included by**packages/runtime-tags/src/__tests__/fixtures/controllable-select-dynamic-spread/__snapshots__/dom.bundle.jsis excluded by!**/__snapshots__/**and included by**packages/runtime-tags/src/__tests__/fixtures/controllable-select-dynamic-spread/__snapshots__/html.bundle.debug.jsis excluded by!**/__snapshots__/**and included by**packages/runtime-tags/src/__tests__/fixtures/controllable-select-dynamic-spread/__snapshots__/html.bundle.jsis excluded by!**/__snapshots__/**and included by**packages/runtime-tags/src/__tests__/fixtures/controllable-select-dynamic-spread/__snapshots__/writes.debug.htmlis excluded by!**/__snapshots__/**and included by**packages/runtime-tags/src/__tests__/fixtures/controllable-select-dynamic-spread/__snapshots__/writes.htmlis excluded by!**/__snapshots__/**and included by**packages/runtime-tags/src/__tests__/fixtures/debug-tag/__snapshots__/dom.bundle.debug.jsis excluded by!**/__snapshots__/**and included by**packages/runtime-tags/src/__tests__/fixtures/debug-tag/__snapshots__/dom.bundle.jsis excluded by!**/__snapshots__/**and included by**packages/runtime-tags/src/__tests__/fixtures/debug-tag/__snapshots__/html.bundle.debug.jsis excluded by!**/__snapshots__/**and included by**packages/runtime-tags/src/__tests__/fixtures/debug-tag/__snapshots__/html.bundle.jsis excluded by!**/__snapshots__/**and included by**packages/runtime-tags/src/__tests__/fixtures/declared-alias-closure/__snapshots__/dom.bundle.debug.jsis excluded by!**/__snapshots__/**and included by**packages/runtime-tags/src/__tests__/fixtures/declared-alias-closure/__snapshots__/dom.bundle.jsis excluded by!**/__snapshots__/**and included by**packages/runtime-tags/src/__tests__/fixtures/declared-alias-closure/__snapshots__/html.bundle.debug.jsis excluded by!**/__snapshots__/**and included by**packages/runtime-tags/src/__tests__/fixtures/declared-alias-closure/__snapshots__/html.bundle.jsis excluded by!**/__snapshots__/**and included by**packages/runtime-tags/src/__tests__/fixtures/declared-alias-closure/__snapshots__/writes.debug.htmlis excluded by!**/__snapshots__/**and included by**packages/runtime-tags/src/__tests__/fixtures/declared-alias-closure/__snapshots__/writes.htmlis excluded by!**/__snapshots__/**and included by**packages/runtime-tags/src/__tests__/fixtures/destructure-and-reference-input/__snapshots__/dom.bundle.debug.jsis excluded by!**/__snapshots__/**and included by**packages/runtime-tags/src/__tests__/fixtures/destructure-and-reference-input/__snapshots__/html.bundle.debug.jsis excluded by!**/__snapshots__/**and included by**packages/runtime-tags/src/__tests__/fixtures/destructure-and-reference-input/__snapshots__/html.bundle.jsis excluded by!**/__snapshots__/**and included by**packages/runtime-tags/src/__tests__/fixtures/destructure-and-reference-unprovided-input/__snapshots__/dom.bundle.debug.jsis excluded by!**/__snapshots__/**and included by**packages/runtime-tags/src/__tests__/fixtures/destructure-and-reference-unprovided-input/__snapshots__/html.bundle.debug.jsis excluded by!**/__snapshots__/**and included by**packages/runtime-tags/src/__tests__/fixtures/destructure-and-reference-unprovided-input/__snapshots__/html.bundle.jsis excluded by!**/__snapshots__/**and included by**packages/runtime-tags/src/__tests__/fixtures/destructure-input/__snapshots__/dom.bundle.debug.jsis excluded by!**/__snapshots__/**and included by**packages/runtime-tags/src/__tests__/fixtures/destructure-input/__snapshots__/html.bundle.debug.jsis excluded by!**/__snapshots__/**and included by**packages/runtime-tags/src/__tests__/fixtures/destructure-input/__snapshots__/html.bundle.jsis excluded by!**/__snapshots__/**and included by**packages/runtime-tags/src/__tests__/fixtures/dynamic-closure-with-function/__snapshots__/dom.bundle.debug.jsis excluded by!**/__snapshots__/**and included by**packages/runtime-tags/src/__tests__/fixtures/dynamic-closure-with-function/__snapshots__/html.bundle.debug.jsis excluded by!**/__snapshots__/**and included by**packages/runtime-tags/src/__tests__/fixtures/dynamic-closure-with-function/__snapshots__/html.bundle.jsis excluded by!**/__snapshots__/**and included by**packages/runtime-tags/src/__tests__/fixtures/dynamic-closures/__snapshots__/dom.bundle.debug.jsis excluded by!**/__snapshots__/**and included by**packages/runtime-tags/src/__tests__/fixtures/dynamic-closures/__snapshots__/dom.bundle.jsis excluded by!**/__snapshots__/**and included by**packages/runtime-tags/src/__tests__/fixtures/dynamic-closures/__snapshots__/html.bundle.debug.jsis excluded by!**/__snapshots__/**and included by**packages/runtime-tags/src/__tests__/fixtures/dynamic-closures/__snapshots__/html.bundle.jsis excluded by!**/__snapshots__/**and included by**packages/runtime-tags/src/__tests__/fixtures/dynamic-closures/__snapshots__/writes.debug.htmlis excluded by!**/__snapshots__/**and included by**packages/runtime-tags/src/__tests__/fixtures/dynamic-closures/__snapshots__/writes.htmlis excluded by!**/__snapshots__/**and included by**packages/runtime-tags/src/__tests__/fixtures/dynamic-tag-name/__snapshots__/html.bundle.debug.jsis excluded by!**/__snapshots__/**and included by**packages/runtime-tags/src/__tests__/fixtures/dynamic-tag-name/__snapshots__/html.bundle.jsis excluded by!**/__snapshots__/**and included by**packages/runtime-tags/src/__tests__/fixtures/hoist-only/__snapshots__/dom.bundle.debug.jsis excluded by!**/__snapshots__/**and included by**packages/runtime-tags/src/__tests__/fixtures/hoist-only/__snapshots__/html.bundle.debug.jsis excluded by!**/__snapshots__/**and included by**packages/runtime-tags/src/__tests__/fixtures/import-types/__snapshots__/html.bundle.debug.jsis excluded by!**/__snapshots__/**and included by**packages/runtime-tags/src/__tests__/fixtures/known-define-tag-empty-section-closure/__snapshots__/dom.bundle.debug.jsis excluded by!**/__snapshots__/**and included by**packages/runtime-tags/src/__tests__/fixtures/known-define-tag-empty-section-closure/__snapshots__/dom.bundle.jsis excluded by!**/__snapshots__/**and included by**packages/runtime-tags/src/__tests__/fixtures/known-define-tag-empty-section-closure/__snapshots__/html.bundle.debug.jsis excluded by!**/__snapshots__/**and included by**packages/runtime-tags/src/__tests__/fixtures/known-define-tag-empty-section-closure/__snapshots__/html.bundle.jsis excluded by!**/__snapshots__/**and included by**packages/runtime-tags/src/__tests__/fixtures/known-define-tag-empty-section-closure/__snapshots__/render.debug.mdis excluded by!**/__snapshots__/**and included by**packages/runtime-tags/src/__tests__/fixtures/known-define-tag-empty-section-closure/__snapshots__/render.mdis excluded by!**/__snapshots__/**and included by**packages/runtime-tags/src/__tests__/fixtures/log-tag/__snapshots__/html.bundle.debug.jsis excluded by!**/__snapshots__/**and included by**packages/runtime-tags/src/__tests__/fixtures/log-tag/__snapshots__/render-csr.debug.mdis excluded by!**/__snapshots__/**and included by**packages/runtime-tags/src/__tests__/fixtures/log-tag/__snapshots__/render-csr.mdis excluded by!**/__snapshots__/**and included by**packages/runtime-tags/src/__tests__/fixtures/pure-signal-resume-function/__snapshots__/dom.bundle.debug.jsis excluded by!**/__snapshots__/**and included by**packages/runtime-tags/src/__tests__/fixtures/pure-signal-resume-function/__snapshots__/dom.bundle.jsis excluded by!**/__snapshots__/**and included by**packages/runtime-tags/src/__tests__/fixtures/pure-signal-resume-function/__snapshots__/html.bundle.debug.jsis excluded by!**/__snapshots__/**and included by**packages/runtime-tags/src/__tests__/fixtures/pure-signal-resume-function/__snapshots__/html.bundle.jsis excluded by!**/__snapshots__/**and included by**packages/runtime-tags/src/__tests__/fixtures/pure-signal-resume-function/__snapshots__/writes.debug.htmlis excluded by!**/__snapshots__/**and included by**packages/runtime-tags/src/__tests__/fixtures/pure-signal-resume-function/__snapshots__/writes.htmlis excluded by!**/__snapshots__/**and included by**packages/runtime-tags/src/__tests__/fixtures/return-tag-no-state/__snapshots__/html.bundle.debug.jsis excluded by!**/__snapshots__/**and included by**packages/runtime-tags/src/__tests__/fixtures/return-tag-no-state/__snapshots__/html.bundle.jsis excluded by!**/__snapshots__/**and included by**packages/runtime-tags/src/__tests__/fixtures/tag-param-if-closure/__snapshots__/dom.bundle.debug.jsis excluded by!**/__snapshots__/**and included by**packages/runtime-tags/src/__tests__/fixtures/tag-param-if-closure/__snapshots__/dom.bundle.jsis excluded by!**/__snapshots__/**and included by**packages/runtime-tags/src/__tests__/fixtures/tag-param-if-closure/__snapshots__/html.bundle.debug.jsis excluded by!**/__snapshots__/**and included by**packages/runtime-tags/src/__tests__/fixtures/tag-param-if-closure/__snapshots__/html.bundle.jsis excluded by!**/__snapshots__/**and included by**packages/runtime-tags/src/__tests__/fixtures/tag-param-if-closure/__snapshots__/render.debug.mdis excluded by!**/__snapshots__/**and included by**packages/runtime-tags/src/__tests__/fixtures/tag-param-if-closure/__snapshots__/render.mdis excluded by!**/__snapshots__/**and included by**packages/runtime-tags/src/__tests__/fixtures/tag-param-if-closure/__snapshots__/writes.debug.htmlis excluded by!**/__snapshots__/**and included by**packages/runtime-tags/src/__tests__/fixtures/tag-param-if-closure/__snapshots__/writes.htmlis excluded by!**/__snapshots__/**and included by**packages/runtime-tags/src/__tests__/fixtures/try-tag-closure/__snapshots__/html.bundle.debug.jsis excluded by!**/__snapshots__/**and included by**packages/runtime-tags/src/__tests__/fixtures/try-tag-closure/__snapshots__/html.bundle.jsis excluded by!**/__snapshots__/**and included by**
📒 Files selected for processing (36)
.changeset/const-fold-primitives.mdpackages/runtime-tags/src/__tests__/fixtures-interop/ambiguous-tags-and-components-dir/sizes.jsonpackages/runtime-tags/src/__tests__/fixtures-interop/class/sizes.jsonpackages/runtime-tags/src/__tests__/fixtures-interop/custom-tag-parameters-from-args/sizes.jsonpackages/runtime-tags/src/__tests__/fixtures-interop/interop-basic-class-to-tags/sizes.jsonpackages/runtime-tags/src/__tests__/fixtures-interop/interop-basic-tags-to-class/sizes.jsonpackages/runtime-tags/src/__tests__/fixtures-interop/interop-class-in-components-in-tags-dir/sizes.jsonpackages/runtime-tags/src/__tests__/fixtures-interop/interop-class-to-tags-import/sizes.jsonpackages/runtime-tags/src/__tests__/fixtures-interop/interop-event-handler-render-body-tags-to-class/sizes.jsonpackages/runtime-tags/src/__tests__/fixtures-interop/interop-events-tags-to-class/sizes.jsonpackages/runtime-tags/src/__tests__/fixtures-interop/interop-nested-attr-tags-class-to-tags/sizes.jsonpackages/runtime-tags/src/__tests__/fixtures-interop/interop-nested-class-to-tags/sizes.jsonpackages/runtime-tags/src/__tests__/fixtures-interop/interop-nested-tags-to-class/sizes.jsonpackages/runtime-tags/src/__tests__/fixtures-interop/interop-stateless-tags-to-class/sizes.jsonpackages/runtime-tags/src/__tests__/fixtures-interop/interop-tag-params-class-to-tags/sizes.jsonpackages/runtime-tags/src/__tests__/fixtures-interop/interop-tag-params-tags-to-class/sizes.jsonpackages/runtime-tags/src/__tests__/fixtures-interop/lazy-class-child-event/sizes.jsonpackages/runtime-tags/src/__tests__/fixtures-interop/lazy-class-child-idle/sizes.jsonpackages/runtime-tags/src/__tests__/fixtures-interop/lazy-class-child-ssr-toggle/sizes.jsonpackages/runtime-tags/src/__tests__/fixtures-interop/lazy-class-child-ssr/sizes.jsonpackages/runtime-tags/src/__tests__/fixtures-interop/lazy-class-child-unmount-before-load/sizes.jsonpackages/runtime-tags/src/__tests__/fixtures-interop/lazy-class-child-visible/sizes.jsonpackages/runtime-tags/src/__tests__/fixtures-interop/let/sizes.jsonpackages/runtime-tags/src/__tests__/fixtures/assign-to-owner-closure/sizes.jsonpackages/runtime-tags/src/__tests__/fixtures/attr-class-delimited-shapes/sizes.jsonpackages/runtime-tags/src/__tests__/fixtures/const-tag-fold/sizes.jsonpackages/runtime-tags/src/__tests__/fixtures/const-tag-fold/template.markopackages/runtime-tags/src/__tests__/fixtures/const-tag-fold/test.tspackages/runtime-tags/src/__tests__/fixtures/controllable-select-dynamic-spread/sizes.jsonpackages/runtime-tags/src/__tests__/fixtures/debug-tag/sizes.jsonpackages/runtime-tags/src/__tests__/fixtures/declared-alias-closure/sizes.jsonpackages/runtime-tags/src/__tests__/fixtures/dynamic-closures/sizes.jsonpackages/runtime-tags/src/__tests__/fixtures/known-define-tag-empty-section-closure/sizes.jsonpackages/runtime-tags/src/__tests__/fixtures/pure-signal-resume-function/sizes.jsonpackages/runtime-tags/src/__tests__/fixtures/tag-param-if-closure/sizes.jsonpackages/runtime-tags/src/translator/core/const.ts
DylanPiercey
force-pushed
the
claude/opt-const-folding
branch
2 times, most recently
from
58b466d to
9034e34
Compare
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
packages/runtime-tags/src/translator/util/references.ts (1)
1850-1898:⚠️ Potential issue | 🟠 Major | ⚡ Quick winPreserve non-optional member reads for nullish constants.
Line 1893 can turn a folded
null/undefinedconst read likex.foointonull?.foo/undefined?.foobecause the binding remains nullable, changing a thrown TypeError intoundefined. Gate this nullable-scope rewrite to the non-const replacement path so source optionality is preserved.🐛 Proposed fix
- readBinding.nullable && + !constValueNode && + readBinding.nullable && replaceMember.object.type !== replacement.type🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/runtime-tags/src/translator/util/references.ts` around lines 1850 - 1898, The nullable-scope rewrite that converts a member expression to OptionalMemberExpression (the if (replaceMember) block starting at line 1893) is being applied even when constValueNode is present, which incorrectly transforms const reads like null.foo into null?.foo and changes their semantics from throwing a TypeError to returning undefined. Guard this nullable rewrite block to only execute when constValueNode is not set, ensuring the original optionality of const value replacements is preserved by checking that constValueNode is falsy before entering the if (replaceMember) condition.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@packages/runtime-tags/src/translator/core/const.ts`:
- Around line 148-155: The canInlineValue function currently returns true for
all values of type "number", which includes NaN and Infinity, but numeric
inlining should only apply to finite numbers. Add a check to filter out
non-finite numbers by testing if the value is finite using Number.isFinite()
when the type is "number", so that non-finite values like NaN and Infinity
continue through the runtime path instead of being inlined.
- Around line 100-101: The assignment `binding.constValue =
t.valueToNode(valueExtra.computed)` at line 101 can result in an Identifier node
referencing "undefined" when the computed value is undefined, which may capture
a shadowed undefined variable in the scope. Instead of passing the raw computed
value to t.valueToNode(), check if valueExtra.computed is undefined and pass
void 0 to t.valueToNode() in that case to ensure the const binding uses the void
0 operator instead of an undefined identifier.
---
Outside diff comments:
In `@packages/runtime-tags/src/translator/util/references.ts`:
- Around line 1850-1898: The nullable-scope rewrite that converts a member
expression to OptionalMemberExpression (the if (replaceMember) block starting at
line 1893) is being applied even when constValueNode is present, which
incorrectly transforms const reads like null.foo into null?.foo and changes
their semantics from throwing a TypeError to returning undefined. Guard this
nullable rewrite block to only execute when constValueNode is not set, ensuring
the original optionality of const value replacements is preserved by checking
that constValueNode is falsy before entering the if (replaceMember) condition.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 8746b100-239a-4207-b1a7-ee1b35768135
⛔ Files ignored due to path filters (89)
packages/runtime-tags/src/__tests__/fixtures/attr-class-delimited-shapes/__snapshots__/dom.bundle.debug.jsis excluded by!**/__snapshots__/**and included by**packages/runtime-tags/src/__tests__/fixtures/attr-class-delimited-shapes/__snapshots__/dom.bundle.jsis excluded by!**/__snapshots__/**and included by**packages/runtime-tags/src/__tests__/fixtures/attr-class-delimited-shapes/__snapshots__/html.bundle.debug.jsis excluded by!**/__snapshots__/**and included by**packages/runtime-tags/src/__tests__/fixtures/attr-class-delimited-shapes/__snapshots__/html.bundle.jsis excluded by!**/__snapshots__/**and included by**packages/runtime-tags/src/__tests__/fixtures/attr-class-delimited-shapes/__snapshots__/writes.debug.htmlis excluded by!**/__snapshots__/**and included by**packages/runtime-tags/src/__tests__/fixtures/attr-class-delimited-shapes/__snapshots__/writes.htmlis excluded by!**/__snapshots__/**and included by**packages/runtime-tags/src/__tests__/fixtures/basic-unused-ref/__snapshots__/dom.bundle.debug.jsis excluded by!**/__snapshots__/**and included by**packages/runtime-tags/src/__tests__/fixtures/basic-unused-ref/__snapshots__/html.bundle.debug.jsis excluded by!**/__snapshots__/**and included by**packages/runtime-tags/src/__tests__/fixtures/bound-attr-computed-key/__snapshots__/dom.bundle.debug.jsis excluded by!**/__snapshots__/**and included by**packages/runtime-tags/src/__tests__/fixtures/bound-attr-computed-key/__snapshots__/dom.bundle.jsis excluded by!**/__snapshots__/**and included by**packages/runtime-tags/src/__tests__/fixtures/bound-attr-computed-key/__snapshots__/html.bundle.debug.jsis excluded by!**/__snapshots__/**and included by**packages/runtime-tags/src/__tests__/fixtures/bound-attr-computed-key/__snapshots__/html.bundle.jsis excluded by!**/__snapshots__/**and included by**packages/runtime-tags/src/__tests__/fixtures/const-tag-fold/__snapshots__/dom.bundle.debug.jsis excluded by!**/__snapshots__/**and included by**packages/runtime-tags/src/__tests__/fixtures/const-tag-fold/__snapshots__/dom.bundle.jsis excluded by!**/__snapshots__/**and included by**packages/runtime-tags/src/__tests__/fixtures/const-tag-fold/__snapshots__/html.bundle.debug.jsis excluded by!**/__snapshots__/**and included by**packages/runtime-tags/src/__tests__/fixtures/const-tag-fold/__snapshots__/html.bundle.jsis excluded by!**/__snapshots__/**and included by**packages/runtime-tags/src/__tests__/fixtures/const-tag-fold/__snapshots__/render.debug.mdis excluded by!**/__snapshots__/**and included by**packages/runtime-tags/src/__tests__/fixtures/const-tag-fold/__snapshots__/render.mdis excluded by!**/__snapshots__/**and included by**packages/runtime-tags/src/__tests__/fixtures/const-tag-fold/__snapshots__/writes.debug.htmlis excluded by!**/__snapshots__/**and included by**packages/runtime-tags/src/__tests__/fixtures/const-tag-fold/__snapshots__/writes.htmlis excluded by!**/__snapshots__/**and included by**packages/runtime-tags/src/__tests__/fixtures/const-tag-hoist-error/__snapshots__/html.bundle.debug.jsis excluded by!**/__snapshots__/**and included by**packages/runtime-tags/src/__tests__/fixtures/const-tag/__snapshots__/dom.bundle.debug.jsis excluded by!**/__snapshots__/**and included by**packages/runtime-tags/src/__tests__/fixtures/const-tag/__snapshots__/html.bundle.debug.jsis excluded by!**/__snapshots__/**and included by**packages/runtime-tags/src/__tests__/fixtures/const-tag/__snapshots__/html.bundle.jsis excluded by!**/__snapshots__/**and included by**packages/runtime-tags/src/__tests__/fixtures/controllable-select-dynamic-spread/__snapshots__/dom.bundle.debug.jsis excluded by!**/__snapshots__/**and included by**packages/runtime-tags/src/__tests__/fixtures/controllable-select-dynamic-spread/__snapshots__/dom.bundle.jsis excluded by!**/__snapshots__/**and included by**packages/runtime-tags/src/__tests__/fixtures/controllable-select-dynamic-spread/__snapshots__/html.bundle.debug.jsis excluded by!**/__snapshots__/**and included by**packages/runtime-tags/src/__tests__/fixtures/controllable-select-dynamic-spread/__snapshots__/html.bundle.jsis excluded by!**/__snapshots__/**and included by**packages/runtime-tags/src/__tests__/fixtures/controllable-select-dynamic-spread/__snapshots__/writes.debug.htmlis excluded by!**/__snapshots__/**and included by**packages/runtime-tags/src/__tests__/fixtures/controllable-select-dynamic-spread/__snapshots__/writes.htmlis excluded by!**/__snapshots__/**and included by**packages/runtime-tags/src/__tests__/fixtures/debug-tag/__snapshots__/dom.bundle.debug.jsis excluded by!**/__snapshots__/**and included by**packages/runtime-tags/src/__tests__/fixtures/debug-tag/__snapshots__/dom.bundle.jsis excluded by!**/__snapshots__/**and included by**packages/runtime-tags/src/__tests__/fixtures/debug-tag/__snapshots__/html.bundle.debug.jsis excluded by!**/__snapshots__/**and included by**packages/runtime-tags/src/__tests__/fixtures/debug-tag/__snapshots__/html.bundle.jsis excluded by!**/__snapshots__/**and included by**packages/runtime-tags/src/__tests__/fixtures/destructure-and-reference-input/__snapshots__/dom.bundle.debug.jsis excluded by!**/__snapshots__/**and included by**packages/runtime-tags/src/__tests__/fixtures/destructure-and-reference-input/__snapshots__/html.bundle.debug.jsis excluded by!**/__snapshots__/**and included by**packages/runtime-tags/src/__tests__/fixtures/destructure-and-reference-input/__snapshots__/html.bundle.jsis excluded by!**/__snapshots__/**and included by**packages/runtime-tags/src/__tests__/fixtures/destructure-and-reference-input/__snapshots__/writes.debug.htmlis excluded by!**/__snapshots__/**and included by**packages/runtime-tags/src/__tests__/fixtures/destructure-and-reference-input/__snapshots__/writes.htmlis excluded by!**/__snapshots__/**and included by**packages/runtime-tags/src/__tests__/fixtures/destructure-and-reference-unprovided-input/__snapshots__/dom.bundle.debug.jsis excluded by!**/__snapshots__/**and included by**packages/runtime-tags/src/__tests__/fixtures/destructure-and-reference-unprovided-input/__snapshots__/html.bundle.debug.jsis excluded by!**/__snapshots__/**and included by**packages/runtime-tags/src/__tests__/fixtures/destructure-and-reference-unprovided-input/__snapshots__/html.bundle.jsis excluded by!**/__snapshots__/**and included by**packages/runtime-tags/src/__tests__/fixtures/destructure-and-reference-unprovided-input/__snapshots__/writes.debug.htmlis excluded by!**/__snapshots__/**and included by**packages/runtime-tags/src/__tests__/fixtures/destructure-and-reference-unprovided-input/__snapshots__/writes.htmlis excluded by!**/__snapshots__/**and included by**packages/runtime-tags/src/__tests__/fixtures/destructure-input/__snapshots__/dom.bundle.debug.jsis excluded by!**/__snapshots__/**and included by**packages/runtime-tags/src/__tests__/fixtures/destructure-input/__snapshots__/html.bundle.debug.jsis excluded by!**/__snapshots__/**and included by**packages/runtime-tags/src/__tests__/fixtures/destructure-input/__snapshots__/html.bundle.jsis excluded by!**/__snapshots__/**and included by**packages/runtime-tags/src/__tests__/fixtures/destructure-input/__snapshots__/writes.debug.htmlis excluded by!**/__snapshots__/**and included by**packages/runtime-tags/src/__tests__/fixtures/destructure-input/__snapshots__/writes.htmlis excluded by!**/__snapshots__/**and included by**packages/runtime-tags/src/__tests__/fixtures/dynamic-closure-with-function/__snapshots__/dom.bundle.debug.jsis excluded by!**/__snapshots__/**and included by**packages/runtime-tags/src/__tests__/fixtures/dynamic-closure-with-function/__snapshots__/html.bundle.debug.jsis excluded by!**/__snapshots__/**and included by**packages/runtime-tags/src/__tests__/fixtures/dynamic-closure-with-function/__snapshots__/html.bundle.jsis excluded by!**/__snapshots__/**and included by**packages/runtime-tags/src/__tests__/fixtures/dynamic-closure-with-function/__snapshots__/writes.debug.htmlis excluded by!**/__snapshots__/**and included by**packages/runtime-tags/src/__tests__/fixtures/dynamic-closure-with-function/__snapshots__/writes.htmlis excluded by!**/__snapshots__/**and included by**packages/runtime-tags/src/__tests__/fixtures/dynamic-closures/__snapshots__/dom.bundle.debug.jsis excluded by!**/__snapshots__/**and included by**packages/runtime-tags/src/__tests__/fixtures/dynamic-closures/__snapshots__/html.bundle.debug.jsis excluded by!**/__snapshots__/**and included by**packages/runtime-tags/src/__tests__/fixtures/dynamic-closures/__snapshots__/html.bundle.jsis excluded by!**/__snapshots__/**and included by**packages/runtime-tags/src/__tests__/fixtures/dynamic-tag-name/__snapshots__/html.bundle.debug.jsis excluded by!**/__snapshots__/**and included by**packages/runtime-tags/src/__tests__/fixtures/dynamic-tag-name/__snapshots__/html.bundle.jsis excluded by!**/__snapshots__/**and included by**packages/runtime-tags/src/__tests__/fixtures/dynamic-tag-name/__snapshots__/writes.debug.htmlis excluded by!**/__snapshots__/**and included by**packages/runtime-tags/src/__tests__/fixtures/dynamic-tag-name/__snapshots__/writes.htmlis excluded by!**/__snapshots__/**and included by**packages/runtime-tags/src/__tests__/fixtures/hoist-only/__snapshots__/dom.bundle.debug.jsis excluded by!**/__snapshots__/**and included by**packages/runtime-tags/src/__tests__/fixtures/hoist-only/__snapshots__/html.bundle.debug.jsis excluded by!**/__snapshots__/**and included by**packages/runtime-tags/src/__tests__/fixtures/import-types/__snapshots__/html.bundle.debug.jsis excluded by!**/__snapshots__/**and included by**packages/runtime-tags/src/__tests__/fixtures/known-define-tag-empty-section-closure/__snapshots__/dom.bundle.debug.jsis excluded by!**/__snapshots__/**and included by**packages/runtime-tags/src/__tests__/fixtures/known-define-tag-empty-section-closure/__snapshots__/dom.bundle.jsis excluded by!**/__snapshots__/**and included by**packages/runtime-tags/src/__tests__/fixtures/known-define-tag-empty-section-closure/__snapshots__/html.bundle.debug.jsis excluded by!**/__snapshots__/**and included by**packages/runtime-tags/src/__tests__/fixtures/known-define-tag-empty-section-closure/__snapshots__/html.bundle.jsis excluded by!**/__snapshots__/**and included by**packages/runtime-tags/src/__tests__/fixtures/log-tag/__snapshots__/html.bundle.debug.jsis excluded by!**/__snapshots__/**and included by**packages/runtime-tags/src/__tests__/fixtures/log-tag/__snapshots__/render-csr.debug.mdis excluded by!**/__snapshots__/**and included by**packages/runtime-tags/src/__tests__/fixtures/log-tag/__snapshots__/render-csr.mdis excluded by!**/__snapshots__/**and included by**packages/runtime-tags/src/__tests__/fixtures/pure-signal-resume-function/__snapshots__/dom.bundle.debug.jsis excluded by!**/__snapshots__/**and included by**packages/runtime-tags/src/__tests__/fixtures/pure-signal-resume-function/__snapshots__/dom.bundle.jsis excluded by!**/__snapshots__/**and included by**packages/runtime-tags/src/__tests__/fixtures/pure-signal-resume-function/__snapshots__/html.bundle.debug.jsis excluded by!**/__snapshots__/**and included by**packages/runtime-tags/src/__tests__/fixtures/pure-signal-resume-function/__snapshots__/html.bundle.jsis excluded by!**/__snapshots__/**and included by**packages/runtime-tags/src/__tests__/fixtures/pure-signal-resume-function/__snapshots__/writes.debug.htmlis excluded by!**/__snapshots__/**and included by**packages/runtime-tags/src/__tests__/fixtures/pure-signal-resume-function/__snapshots__/writes.htmlis excluded by!**/__snapshots__/**and included by**packages/runtime-tags/src/__tests__/fixtures/return-tag-no-state/__snapshots__/html.bundle.debug.jsis excluded by!**/__snapshots__/**and included by**packages/runtime-tags/src/__tests__/fixtures/return-tag-no-state/__snapshots__/html.bundle.jsis excluded by!**/__snapshots__/**and included by**packages/runtime-tags/src/__tests__/fixtures/tag-param-if-closure/__snapshots__/dom.bundle.debug.jsis excluded by!**/__snapshots__/**and included by**packages/runtime-tags/src/__tests__/fixtures/tag-param-if-closure/__snapshots__/dom.bundle.jsis excluded by!**/__snapshots__/**and included by**packages/runtime-tags/src/__tests__/fixtures/tag-param-if-closure/__snapshots__/html.bundle.debug.jsis excluded by!**/__snapshots__/**and included by**packages/runtime-tags/src/__tests__/fixtures/tag-param-if-closure/__snapshots__/html.bundle.jsis excluded by!**/__snapshots__/**and included by**packages/runtime-tags/src/__tests__/fixtures/tag-param-if-closure/__snapshots__/writes.debug.htmlis excluded by!**/__snapshots__/**and included by**packages/runtime-tags/src/__tests__/fixtures/tag-param-if-closure/__snapshots__/writes.htmlis excluded by!**/__snapshots__/**and included by**packages/runtime-tags/src/__tests__/fixtures/try-tag-closure/__snapshots__/html.bundle.debug.jsis excluded by!**/__snapshots__/**and included by**packages/runtime-tags/src/__tests__/fixtures/try-tag-closure/__snapshots__/html.bundle.jsis excluded by!**/__snapshots__/**and included by**packages/runtime-tags/src/__tests__/fixtures/try-tag-closure/__snapshots__/writes.debug.htmlis excluded by!**/__snapshots__/**and included by**packages/runtime-tags/src/__tests__/fixtures/try-tag-closure/__snapshots__/writes.htmlis excluded by!**/__snapshots__/**and included by**
📒 Files selected for processing (43)
.changeset/const-fold-primitives.mdpackages/runtime-tags/src/__tests__/fixtures-interop/ambiguous-tags-and-components-dir/sizes.jsonpackages/runtime-tags/src/__tests__/fixtures-interop/class/sizes.jsonpackages/runtime-tags/src/__tests__/fixtures-interop/custom-tag-parameters-from-args/sizes.jsonpackages/runtime-tags/src/__tests__/fixtures-interop/interop-basic-class-to-tags/sizes.jsonpackages/runtime-tags/src/__tests__/fixtures-interop/interop-basic-tags-to-class/sizes.jsonpackages/runtime-tags/src/__tests__/fixtures-interop/interop-class-in-components-in-tags-dir/sizes.jsonpackages/runtime-tags/src/__tests__/fixtures-interop/interop-class-to-tags-import/sizes.jsonpackages/runtime-tags/src/__tests__/fixtures-interop/interop-event-handler-render-body-tags-to-class/sizes.jsonpackages/runtime-tags/src/__tests__/fixtures-interop/interop-events-tags-to-class/sizes.jsonpackages/runtime-tags/src/__tests__/fixtures-interop/interop-nested-attr-tags-class-to-tags/sizes.jsonpackages/runtime-tags/src/__tests__/fixtures-interop/interop-nested-class-to-tags/sizes.jsonpackages/runtime-tags/src/__tests__/fixtures-interop/interop-nested-tags-to-class/sizes.jsonpackages/runtime-tags/src/__tests__/fixtures-interop/interop-stateless-tags-to-class/sizes.jsonpackages/runtime-tags/src/__tests__/fixtures-interop/interop-tag-params-class-to-tags/sizes.jsonpackages/runtime-tags/src/__tests__/fixtures-interop/interop-tag-params-tags-to-class/sizes.jsonpackages/runtime-tags/src/__tests__/fixtures-interop/lazy-class-child-event/sizes.jsonpackages/runtime-tags/src/__tests__/fixtures-interop/lazy-class-child-idle/sizes.jsonpackages/runtime-tags/src/__tests__/fixtures-interop/lazy-class-child-ssr-toggle/sizes.jsonpackages/runtime-tags/src/__tests__/fixtures-interop/lazy-class-child-ssr/sizes.jsonpackages/runtime-tags/src/__tests__/fixtures-interop/lazy-class-child-unmount-before-load/sizes.jsonpackages/runtime-tags/src/__tests__/fixtures-interop/lazy-class-child-visible/sizes.jsonpackages/runtime-tags/src/__tests__/fixtures-interop/let/sizes.jsonpackages/runtime-tags/src/__tests__/fixtures/assign-to-owner-closure/sizes.jsonpackages/runtime-tags/src/__tests__/fixtures/attr-class-delimited-shapes/sizes.jsonpackages/runtime-tags/src/__tests__/fixtures/bound-attr-computed-key/sizes.jsonpackages/runtime-tags/src/__tests__/fixtures/const-tag-fold/sizes.jsonpackages/runtime-tags/src/__tests__/fixtures/const-tag-fold/template.markopackages/runtime-tags/src/__tests__/fixtures/const-tag-fold/test.tspackages/runtime-tags/src/__tests__/fixtures/controllable-select-dynamic-spread/sizes.jsonpackages/runtime-tags/src/__tests__/fixtures/debug-tag/sizes.jsonpackages/runtime-tags/src/__tests__/fixtures/destructure-and-reference-input/sizes.jsonpackages/runtime-tags/src/__tests__/fixtures/destructure-and-reference-unprovided-input/sizes.jsonpackages/runtime-tags/src/__tests__/fixtures/destructure-input/sizes.jsonpackages/runtime-tags/src/__tests__/fixtures/dynamic-closure-with-function/sizes.jsonpackages/runtime-tags/src/__tests__/fixtures/dynamic-tag-name/sizes.jsonpackages/runtime-tags/src/__tests__/fixtures/known-define-tag-empty-section-closure/sizes.jsonpackages/runtime-tags/src/__tests__/fixtures/pure-signal-resume-function/sizes.jsonpackages/runtime-tags/src/__tests__/fixtures/tag-param-if-closure/sizes.jsonpackages/runtime-tags/src/__tests__/fixtures/try-tag-closure/sizes.jsonpackages/runtime-tags/src/translator/core/const.tspackages/runtime-tags/src/translator/util/references.tspackages/runtime-tags/src/translator/util/serialize-reasons.ts
✅ Files skipped from review due to trivial changes (37)
- packages/runtime-tags/src/tests/fixtures-interop/class/sizes.json
- packages/runtime-tags/src/tests/fixtures-interop/interop-basic-tags-to-class/sizes.json
- packages/runtime-tags/src/tests/fixtures-interop/interop-events-tags-to-class/sizes.json
- packages/runtime-tags/src/tests/fixtures-interop/lazy-class-child-event/sizes.json
- packages/runtime-tags/src/tests/fixtures-interop/interop-nested-class-to-tags/sizes.json
- packages/runtime-tags/src/tests/fixtures/bound-attr-computed-key/sizes.json
- packages/runtime-tags/src/tests/fixtures-interop/lazy-class-child-ssr/sizes.json
- packages/runtime-tags/src/tests/fixtures-interop/lazy-class-child-unmount-before-load/sizes.json
- packages/runtime-tags/src/tests/fixtures/controllable-select-dynamic-spread/sizes.json
- packages/runtime-tags/src/tests/fixtures-interop/custom-tag-parameters-from-args/sizes.json
- packages/runtime-tags/src/tests/fixtures/assign-to-owner-closure/sizes.json
- packages/runtime-tags/src/tests/fixtures/tag-param-if-closure/sizes.json
- packages/runtime-tags/src/tests/fixtures-interop/interop-event-handler-render-body-tags-to-class/sizes.json
- packages/runtime-tags/src/tests/fixtures-interop/interop-tag-params-class-to-tags/sizes.json
- packages/runtime-tags/src/tests/fixtures-interop/interop-nested-tags-to-class/sizes.json
- packages/runtime-tags/src/tests/fixtures/const-tag-fold/sizes.json
- .changeset/const-fold-primitives.md
- packages/runtime-tags/src/tests/fixtures/destructure-input/sizes.json
- packages/runtime-tags/src/tests/fixtures-interop/lazy-class-child-visible/sizes.json
- packages/runtime-tags/src/tests/fixtures/dynamic-tag-name/sizes.json
- packages/runtime-tags/src/tests/fixtures/attr-class-delimited-shapes/sizes.json
- packages/runtime-tags/src/tests/fixtures-interop/lazy-class-child-idle/sizes.json
- packages/runtime-tags/src/tests/fixtures-interop/interop-basic-class-to-tags/sizes.json
- packages/runtime-tags/src/tests/fixtures/known-define-tag-empty-section-closure/sizes.json
- packages/runtime-tags/src/tests/fixtures/destructure-and-reference-unprovided-input/sizes.json
- packages/runtime-tags/src/tests/fixtures/try-tag-closure/sizes.json
- packages/runtime-tags/src/tests/fixtures-interop/ambiguous-tags-and-components-dir/sizes.json
- packages/runtime-tags/src/tests/fixtures-interop/lazy-class-child-ssr-toggle/sizes.json
- packages/runtime-tags/src/tests/fixtures/destructure-and-reference-input/sizes.json
- packages/runtime-tags/src/tests/fixtures-interop/let/sizes.json
- packages/runtime-tags/src/tests/fixtures-interop/interop-stateless-tags-to-class/sizes.json
- packages/runtime-tags/src/tests/fixtures-interop/interop-nested-attr-tags-class-to-tags/sizes.json
- packages/runtime-tags/src/tests/fixtures/debug-tag/sizes.json
- packages/runtime-tags/src/tests/fixtures-interop/interop-class-to-tags-import/sizes.json
- packages/runtime-tags/src/tests/fixtures/pure-signal-resume-function/sizes.json
- packages/runtime-tags/src/tests/fixtures-interop/interop-tag-params-tags-to-class/sizes.json
- packages/runtime-tags/src/tests/fixtures-interop/interop-class-in-components-in-tags-dir/sizes.json
🚧 Files skipped from review as they are similar to previous changes (2)
- packages/runtime-tags/src/tests/fixtures/const-tag-fold/test.ts
- packages/runtime-tags/src/tests/fixtures/const-tag-fold/template.marko
Comment on lines
+148
to
+155
| function canInlineValue(value: unknown) { | ||
| switch (typeof value) { | ||
| case "string": | ||
| case "number": | ||
| case "boolean": | ||
| case "bigint": | ||
| case "undefined": | ||
| return true; |
There was a problem hiding this comment.
Restrict numeric folding to finite numbers.
Line 151 admits NaN and ±Infinity, but this optimization is scoped to finite numbers. Keep non-finite values on the existing runtime path.
🐛 Proposed fix
function canInlineValue(value: unknown) {
switch (typeof value) {
+ case "number":
+ return Number.isFinite(value);
case "string":
- case "number":
case "boolean":
case "bigint":
case "undefined":📝 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.
Suggested change
| function canInlineValue(value: unknown) { | |
| switch (typeof value) { | |
| case "string": | |
| case "number": | |
| case "boolean": | |
| case "bigint": | |
| case "undefined": | |
| return true; | |
| function canInlineValue(value: unknown) { | |
| switch (typeof value) { | |
| case "number": | |
| return Number.isFinite(value); | |
| case "string": | |
| case "boolean": | |
| case "bigint": | |
| case "undefined": | |
| return true; |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@packages/runtime-tags/src/translator/core/const.ts` around lines 148 - 155,
The canInlineValue function currently returns true for all values of type
"number", which includes NaN and Infinity, but numeric inlining should only
apply to finite numbers. Add a check to filter out non-finite numbers by testing
if the value is finite using Number.isFinite() when the type is "number", so
that non-finite values like NaN and Infinity continue through the runtime path
instead of being inlined.
fc3fb04 to
de86c97
Compare
Marks a `<const>` whose value is a compile-time primitive as a constant binding carrying the literal value. Bare references fold into the static template output via the consumer's own evaluation, while reactive positions (handlers, expressions) inline the literal through the existing read-replacement walk. Such constants carry no reactive signal, are never serialized, and do not register as closures when read from a nested section. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01G1bgkqJCLUsEz3xsossMCN
DylanPiercey
force-pushed
the
claude/opt-const-folding
branch
from
de86c97 to
46da484
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What
Constant-folds
<const>tags whose value is a statically known primitive (string, finite number, boolean,null,undefined). Each reference to such aconstis replaced with the literal value, and the<const>tag itself is dropped — no binding and no reactive signal are emitted.This is the first in a series of isolated optimization PRs for the
runtime-tagstranslator.Why
Today a
<const x=1/>always produces aderivedbinding and a reactive signal, even though its value can never change. A reference like${x}is rendered through_text/_escapeat setup rather than being baked into the static template.Because folding happens by node substitution, it also composes:
computeNode(e.g.<const total=step + 3/>→5).totaldepends onstep).Object/array/regexp consts are intentionally not folded (identity-sensitive); their inner primitive-const references still fold.
Example
DOM template before/after this change (debug):
Validation
const-tag-foldexercises folding across placeholder, attribute, event-handler and expression positions, composition, a non-foldable object const, and preserved reactivity (count).runtime-tags(5040) andtranslator-interop(266) suites pass; eslint + prettier clean.error-debug-args).🤖 Generated with Claude Code
https://claude.ai/code/session_01G1bgkqJCLUsEz3xsossMCN
Generated by Claude Code