Skip serializing <let> values that still equal a static primitive default - #3351
Skip serializing <let> values that still equal a static primitive default#3351DylanPiercey wants to merge 1 commit into
<let> values that still equal a static primitive default#3351Conversation
🦋 Changeset detectedLatest commit: ac2e7bd 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 |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #3351 +/- ##
=======================================
Coverage 94.29% 94.30%
=======================================
Files 388 388
Lines 53085 53175 +90
Branches 4237 4252 +15
=======================================
+ Hits 50058 50148 +90
- Misses 2998 2999 +1
+ Partials 29 28 -1 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
WalkthroughThe change tracks eligible static primitive defaults on 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 9
🧹 Nitpick comments (1)
packages/runtime-tags/src/__tests__/fixtures/let-static-default-resume/test.ts (1)
3-6: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winPut the exported
configbefore the helper.The TypeScript guideline requires public exports first and helpers last. Convert
clickto a function declaration, move it belowconfig, and rely on declaration hoisting.Proposed refactor
-const click = (selector: string) => (container: Element) => - container.querySelector<HTMLElement>(selector)!.click(); - export const config: TestConfig = { steps: [ { @@ ], }; + +function click(selector: string) { + return (container: Element) => + container.querySelector<HTMLElement>(selector)!.click(); +}🤖 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/__tests__/fixtures/let-static-default-resume/test.ts` around lines 3 - 6, Move the exported config declaration before the click helper, then convert click from an arrow function to a function declaration placed after config. Preserve its selector and container behavior, relying on function declaration hoisting where needed.Source: Coding guidelines
🤖 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/__tests__/fixtures/define-tag-object/sizes.json`:
- Around line 4-10: Revert the manually edited values in the sizes.json fixture
and regenerate it using the repository’s fixture-generation workflow. Do not
edit the fixture directly; use the relevant generation command or test workflow
for define-tag-object so the min and brotli outputs are reproducible.
In
`@packages/runtime-tags/src/__tests__/fixtures/define-tag-render-args/sizes.json`:
- Around line 4-10: The fixture size values were edited manually; revert those
changes and regenerate sizes.json using the repository’s fixture-generation
workflow, following the relevant generation script or command instead of editing
the file directly.
In
`@packages/runtime-tags/src/__tests__/fixtures/define-tag-render-attr-signal/sizes.json`:
- Around line 4-6: Revert the manual baseline changes in the fixture sizes.json
and regenerate the outputs using the repository’s prescribed fixture-generation
workflow; do not edit sizes.json directly. Apply this to both affected ranges,
including the entries referenced by the fixture generation process.
In
`@packages/runtime-tags/src/__tests__/fixtures/define-tag-render-conditional/sizes.json`:
- Around line 4-5: Revert the manually edited values in the sizes.json fixture
and regenerate it using the repository’s standard fixture-generation workflow;
do not edit fixture sizes.json files directly.
In
`@packages/runtime-tags/src/__tests__/fixtures/define-tag-render-stateful/sizes.json`:
- Around line 4-10: Revert the manual edits in the fixture sizes and regenerate
the outputs using the repository’s fixture-generation workflow; do not modify
sizes.json directly. Locate the relevant generation script or test for
define-tag-render-stateful and run it to update the generated baseline.
In `@packages/runtime-tags/src/__tests__/fixtures/shadow-same-scope/sizes.json`:
- Around line 4-10: Revert the manual changes in the sizes.json fixture and
regenerate it using the repository’s standard fixture-generation workflow; do
not edit the file directly.
In `@packages/runtime-tags/src/__tests__/fixtures/show-tag-empty-body/sizes.json`:
- Around line 4-10: Revert the manual edits to the fixture baseline in
sizes.json and regenerate it using the repository’s fixture-generation workflow,
following the instructions for show-tag-empty-body rather than editing the JSON
directly.
In `@packages/runtime-tags/src/__tests__/fixtures/show-tag-hidden/sizes.json`:
- Around line 4-10: The fixture sizes were edited manually; revert the changes
to sizes.json and regenerate it using the repository’s fixture-generation
workflow, following the path instructions and avoiding direct edits.
In `@packages/runtime-tags/src/translator/core/let.ts`:
- Line 205: In the relevant value-validation logic, replace the self-comparison
`value === value` with `!Number.isNaN(value)` while retaining the existing
negative-zero check, so the expression remains functionally equivalent and
satisfies Biome’s noSelfCompare rule.
---
Nitpick comments:
In
`@packages/runtime-tags/src/__tests__/fixtures/let-static-default-resume/test.ts`:
- Around line 3-6: Move the exported config declaration before the click helper,
then convert click from an arrow function to a function declaration placed after
config. Preserve its selector and container behavior, relying on function
declaration hoisting where needed.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
| "min": 2687, | ||
| "brotli": 1356 | ||
| } | ||
| }, | ||
| "html": { | ||
| "min": 393, | ||
| "brotli": 269 | ||
| "min": 380, | ||
| "brotli": 259 |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
Regenerate this fixture instead of editing sizes.json directly.
Revert these manual baseline changes and regenerate the fixture outputs through the repository workflow so the min/brotli values remain reproducible and aligned with the generated artifacts. As per path instructions, “Never hand-edit fixture sizes.json files; regenerate them instead.”
🤖 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/__tests__/fixtures/define-tag-object/sizes.json`
around lines 4 - 10, Revert the manually edited values in the sizes.json fixture
and regenerate it using the repository’s fixture-generation workflow. Do not
edit the fixture directly; use the relevant generation command or test workflow
for define-tag-object so the min and brotli outputs are reproducible.
Source: Path instructions
| "min": 2647, | ||
| "brotli": 1341 | ||
| } | ||
| }, | ||
| "html": { | ||
| "min": 395, | ||
| "brotli": 266 | ||
| "min": 391, | ||
| "brotli": 262 |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
Regenerate this fixture instead of editing sizes.json directly.
Revert the hand-edited size values and regenerate this fixture through the repository workflow. As per path instructions, “Never hand-edit fixture sizes.json files; regenerate them instead.”
🤖 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/__tests__/fixtures/define-tag-render-args/sizes.json`
around lines 4 - 10, The fixture size values were edited manually; revert those
changes and regenerate sizes.json using the repository’s fixture-generation
workflow, following the relevant generation script or command instead of editing
the file directly.
Source: Path instructions
| "min": 2647, | ||
| "brotli": 1339 | ||
| } |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
Regenerate this fixture instead of editing sizes.json directly.
Revert both manual baseline edits and regenerate the fixture outputs through the repository workflow. As per path instructions, “Never hand-edit fixture sizes.json files; regenerate them instead.”
Also applies to: 8-10
🤖 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/__tests__/fixtures/define-tag-render-attr-signal/sizes.json`
around lines 4 - 6, Revert the manual baseline changes in the fixture sizes.json
and regenerate the outputs using the repository’s prescribed fixture-generation
workflow; do not edit sizes.json directly. Apply this to both affected ranges,
including the entries referenced by the fixture generation process.
Source: Path instructions
| "min": 6291, | ||
| "brotli": 2882 |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
Regenerate this fixture instead of editing sizes.json directly.
Revert these manual size changes and regenerate the fixture output through the repository workflow. As per path instructions, “Never hand-edit fixture sizes.json files; regenerate them instead.”
🤖 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/__tests__/fixtures/define-tag-render-conditional/sizes.json`
around lines 4 - 5, Revert the manually edited values in the sizes.json fixture
and regenerate it using the repository’s standard fixture-generation workflow;
do not edit fixture sizes.json files directly.
Source: Path instructions
| "min": 2647, | ||
| "brotli": 1341 | ||
| } | ||
| }, | ||
| "html": { | ||
| "min": 398, | ||
| "brotli": 268 | ||
| "min": 394, | ||
| "brotli": 265 |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
Regenerate this fixture instead of editing sizes.json directly.
Revert the manual baseline changes and regenerate the fixture outputs through the repository workflow. As per path instructions, “Never hand-edit fixture sizes.json files; regenerate them instead.”
🤖 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/__tests__/fixtures/define-tag-render-stateful/sizes.json`
around lines 4 - 10, Revert the manual edits in the fixture sizes and regenerate
the outputs using the repository’s fixture-generation workflow; do not modify
sizes.json directly. Locate the relevant generation script or test for
define-tag-render-stateful and run it to update the generated baseline.
Source: Path instructions
| "min": 2833, | ||
| "brotli": 1382 | ||
| } | ||
| }, | ||
| "html": { | ||
| "min": 540, | ||
| "brotli": 275 | ||
| "min": 515, | ||
| "brotli": 259 |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
Regenerate this fixture instead of editing sizes.json directly.
Revert these manual size edits and regenerate the fixture through the repository workflow. As per path instructions, “Never hand-edit fixture sizes.json files; regenerate them instead.”
🤖 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/__tests__/fixtures/shadow-same-scope/sizes.json`
around lines 4 - 10, Revert the manual changes in the sizes.json fixture and
regenerate it using the repository’s standard fixture-generation workflow; do
not edit the file directly.
Source: Path instructions
| "min": 4282, | ||
| "brotli": 2066 | ||
| } | ||
| }, | ||
| "html": { | ||
| "min": 374, | ||
| "brotli": 265 | ||
| "min": 360, | ||
| "brotli": 250 |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
Regenerate this fixture instead of editing sizes.json directly.
Revert the manual baseline changes and regenerate the fixture outputs through the repository workflow. As per path instructions, “Never hand-edit fixture sizes.json files; regenerate them instead.”
🤖 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/__tests__/fixtures/show-tag-empty-body/sizes.json`
around lines 4 - 10, Revert the manual edits to the fixture baseline in
sizes.json and regenerate it using the repository’s fixture-generation workflow,
following the instructions for show-tag-empty-body rather than editing the JSON
directly.
Source: Path instructions
| "min": 4282, | ||
| "brotli": 2065 | ||
| } | ||
| }, | ||
| "html": { | ||
| "min": 391, | ||
| "brotli": 269 | ||
| "min": 377, | ||
| "brotli": 258 |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
Regenerate this fixture instead of editing sizes.json directly.
Revert these manual size edits and regenerate the fixture through the repository workflow. As per path instructions, “Never hand-edit fixture sizes.json files; regenerate them instead.”
🤖 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/__tests__/fixtures/show-tag-hidden/sizes.json`
around lines 4 - 10, The fixture sizes were edited manually; revert the changes
to sizes.json and regenerate it using the repository’s fixture-generation
workflow, following the path instructions and avoiding direct edits.
Source: Path instructions
| case "boolean": | ||
| return true; | ||
| case "number": | ||
| return value === value && !Object.is(value, -0); |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟠 Major | ⚡ Quick win
Replace the self-comparison so Biome passes.
value === value triggers Biome’s noSelfCompare error, blocking lint. Use !Number.isNaN(value) instead.
Proposed fix
- return value === value && !Object.is(value, -0);
+ return !Number.isNaN(value) && !Object.is(value, -0);📝 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.
| return value === value && !Object.is(value, -0); | |
| return !Number.isNaN(value) && !Object.is(value, -0); |
🧰 Tools
🪛 Biome (2.5.1)
[error] 205-205: This comparison uses the same expression on both sides.
(lint/suspicious/noSelfCompare)
🤖 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/let.ts` at line 205, In the
relevant value-validation logic, replace the self-comparison `value === value`
with `!Number.isNaN(value)` while retaining the existing negative-zero check, so
the expression remains functionally equivalent and satisfies Biome’s
noSelfCompare rule.
Source: Linters/SAST tools
a1a6df8 to
ac2e7bd
Compare
1 participant
Description
When a
<let>default computes to a primitive that===can test (string, number, boolean,null— excludingNaN/-0), the client can rebuild it from the compiled output, so SSR skips serializing values that still equal it:value === DEFAULT ? void 0 : value; undefined props already drop from the wire.$scope.x ??= DEFAULT;, so every read stays a plain scope access — generated read sites are byte-identical to today, and the bundle cost is one??=per elided let rather than anything per read.<script>that reads the value, which can run before another effect's materializer — keep serializing.One caveat from using
??=: a value mutated tonullduring the server render (only reachable via an immediately-invoked function inside a scriptlet, since plain scriptlet assignments to tag variables are compile errors) would resume as the default instead ofnull. Client-sidenull/undefinedassignments are unaffected — the materializer has already run by then and never re-runs.This removes the resume payload for untouched state — e.g.
basic-counter's fill goes from[_ => [1, {c: 0}], "a0 1"]to["a0 1"], and per-row<let/open=false/>style flags in loops serialize nothing per iteration.Checklist: