test: check __current_slot_index in snapshot by HuJean · Pull Request #2425 · lynx-family/lynx-stack

HuJean · 2026-04-07T02:23:51Z

Summary by CodeRabbit

Tests
- Added test coverage for insertBefore operations with multi-slot dynamic content insertion and removal scenarios, validating slot index tracking behavior.

Checklist

Tests updated (or not required).
Documentation updated (or not required).
Changeset added, and when a BREAKING CHANGE occurs, it needs to be clearly marked (or not required).

changeset-bot · 2026-04-07T02:23:55Z

🦋 Changeset detected

Latest commit: ad8126d

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 0 packages

When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

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

coderabbitai · 2026-04-07T02:24:05Z

📝 Walkthrough

Walkthrough

This PR introduces slot index tracking for multi-slot dynamic parts in the React snapshot runtime. It adds a __current_slot_index field to SnapshotInstance, updates the insertBefore method to use auto-incrementing slot indexing, and includes a new test case validating the insertion and removal behavior within multi-slot wrapper scenarios.

Changes

Cohort / File(s)	Summary
Changeset `.changeset/few-shirts-laugh.md`	Added new changeset entry with empty YAML frontmatter block.
Runtime Source `packages/react/runtime/src/snapshot/snapshot.ts`	Added `__current_slot_index: number` field to `SnapshotInstance` (initialized to `0`) and modified `insertBefore` to use auto-incrementing slot indexing (`this.__current_slot_index++`) when handling multi-slot dynamic parts.
Runtime Tests `packages/react/runtime/__test__/basic.test.jsx`	Added comprehensive `insertBefore` test case that validates slot index increment behavior during wrapper insertion, child replacement, and removal operations with inline DOM structure snapshots.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Possibly related PRs

test(react/runtime): add multiple slots tests #1372: Focuses on SnapshotInstance multi-slot handling and adds corresponding tests for multiple slots in the same runtime.
fix(react): wrap <wrapper> for JSX with dynamic key #1541: Modifies dynamic JSX slot/wrapper generation and directly complements the slot/dynamic-part mapping logic introduced here.
fix: remove redundant internal-slot for pure text children #1562: Changes how snapshot dynamic slots are represented, affecting the same runtime slot indexing logic.

Suggested labels

framework:React

Suggested reviewers

Yradex
hzy
upupming

Poem

🐰✨ A slot index hops and increments with grace,
Dynamic parts now find their rightful place,
Insertions dance, removals stay serene,
The snapshot knows just what it's seen! 🎯

🚥 Pre-merge checks | ✅ 3

✅ Passed checks (3 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title 'test: check __current_slot_index in snapshot' directly summarizes the main change—a new test validating __current_slot_index behavior in SnapshotInstance, which aligns with the test file additions.
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing Touches

📝 Generate docstrings

Create stacked PR
Commit on current branch

🧪 Generate unit tests (beta)

Create PR with unit tests
Commit unit tests in branch p/test-slot-index

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

codecov · 2026-04-07T02:26:24Z

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ All tests successful. No failed tests found.

📢 Thoughts on this report? Let us know!

coderabbitai

Actionable comments posted: 1

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)

packages/react/runtime/src/snapshot/snapshot.ts (1)

376-377: ⚠️ Potential issue | 🔴 Critical

Guard slot-index overflow before dereferencing slot[index].

At Line 376/377, index is incremented and dereferenced with ! without bounds checks. If insertions exceed available slots (or state drifts), this becomes a runtime crash.

Suggested fix

-      const index = this.__current_slot_index++;
-      const [s, elementIndex] = __snapshot_def.slot[index]!;
+      const index = this.__current_slot_index;
+      const slotEntry = __snapshot_def.slot[index];
+      if (!slotEntry) {
+        throw new Error(
+          `Slot index overflow: index=${index}, slotCount=${__snapshot_def.slot.length}, type=${this.type}`,
+        );
+      }
+      this.__current_slot_index = index + 1;
+      const [s, elementIndex] = slotEntry;

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@packages/react/runtime/src/snapshot/snapshot.ts` around lines 376 - 377,
Guard the slot access by checking that the computed index
(this.__current_slot_index++) is strictly less than the available slots array
length before dereferencing __snapshot_def.slot[index]; if it is out of range,
either expand/allocate a new slot entry or throw a clear error (including index
and __snapshot_def.slot.length) instead of using the non-null assertion (!) on
slot[index]; replace the direct use of slot[index]! with a safe read that
handles undefined and update the logic around __current_slot_index and
__snapshot_def.slot to prevent silent runtime crashes.

🤖 Prompt for all review comments with AI agents

Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@packages/react/runtime/src/snapshot/snapshot.ts`:
- Around line 83-87: The cloned snapshot can miss initialization of the new
field __current_slot_index causing undefined++ in insertBefore; update the clone
path used by takeElements so that it either copies the source's
__current_slot_index or initializes it to 0 when creating/cloning an instance.
Specifically, in the takeElements/cloning logic ensure you assign
this.__current_slot_index = source.__current_slot_index ?? 0 (or set to 0 when
no source) so insertBefore and subsequent slot lookups never operate on
undefined.

---

Outside diff comments:
In `@packages/react/runtime/src/snapshot/snapshot.ts`:
- Around line 376-377: Guard the slot access by checking that the computed index
(this.__current_slot_index++) is strictly less than the available slots array
length before dereferencing __snapshot_def.slot[index]; if it is out of range,
either expand/allocate a new slot entry or throw a clear error (including index
and __snapshot_def.slot.length) instead of using the non-null assertion (!) on
slot[index]; replace the direct use of slot[index]! with a safe read that
handles undefined and update the logic around __current_slot_index and
__snapshot_def.slot to prevent silent runtime crashes.

🪄 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: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: 410252af-9f3e-4ffb-980c-826f0da8a96e

📥 Commits

Reviewing files that changed from the base of the PR and between 9df4844 and ad8126d.

📒 Files selected for processing (3)

.changeset/few-shirts-laugh.md
packages/react/runtime/__test__/basic.test.jsx
packages/react/runtime/src/snapshot/snapshot.ts

relativeci · 2026-04-07T02:38:40Z

React External

#200 Bundle Size — 591.44KiB (0%).

ad8126d(current) vs 9df4844 main#197(baseline)

Bundle metrics no changes

	Current #200	Baseline #197
Initial JS	`0B`	`0B`
Initial CSS	`0B`	`0B`
Cache Invalidation	`0%`	`0%`
Chunks	`0`	`0`
Assets	`3`	`3`
Modules	`17`	`17`
Duplicate Modules	`5`	`5`
Duplicate Code	`8.59%`	`8.59%`
Packages	`0`	`0`
Duplicate Packages	`0`	`0`

Bundle size by type no changes

	Current #200	Baseline #197
Other	`591.44KiB`	`591.44KiB`

Bundle analysis report Branch p/test-slot-index Project dashboard

^{Generated by RelativeCI Documentation Report issue}

codspeed-hq · 2026-04-07T02:38:40Z

Merging this PR will not alter performance

✅ 72 untouched benchmarks
⏩ 21 skipped benchmarks¹

_{Comparing p/test-slot-index (ad8126d) with main (9df4844)}

21 benchmarks were skipped, so the baseline results were used instead. If they were deleted from the codebase, click here and archive them to remove them from the performance reports. ↩

relativeci · 2026-04-07T02:38:42Z

Web Explorer

#8656 Bundle Size — 728.84KiB (0%).

ad8126d(current) vs 9df4844 main#8653(baseline)

Bundle metrics no changes

	Current #8656	Baseline #8653
Initial JS	`43.31KiB`	`43.31KiB`
Initial CSS	`2.16KiB`	`2.16KiB`
Cache Invalidation	`0%`	`0%`
Chunks	`8`	`8`
Assets	`10`	`10`
Modules	`148`	`148`
Duplicate Modules	`11`	`11`
Duplicate Code	`34.69%`	`34.69%`
Packages	`3`	`3`
Duplicate Packages	`0`	`0`

Bundle size by type no changes

	Current #8656	Baseline #8653
Other	`384.62KiB`	`384.62KiB`
JS	`342.07KiB`	`342.07KiB`
CSS	`2.16KiB`	`2.16KiB`

Bundle analysis report Branch p/test-slot-index Project dashboard

^{Generated by RelativeCI Documentation Report issue}

relativeci · 2026-04-07T02:38:44Z

React Example

#7081 Bundle Size — 237.81KiB (0%).

ad8126d(current) vs 9df4844 main#7078(baseline)

Bundle metrics no changes

	Current #7081	Baseline #7078
Initial JS	`0B`	`0B`
Initial CSS	`0B`	`0B`
Cache Invalidation	`0%`	`0%`
Chunks	`0`	`0`
Assets	`4`	`4`
Modules	`180`	`180`
Duplicate Modules	`71`	`71`
Duplicate Code	`46.39%`	`46.39%`
Packages	`2`	`2`
Duplicate Packages	`0`	`0`

Bundle size by type no changes

	Current #7081	Baseline #7078
IMG	`145.76KiB`	`145.76KiB`
Other	`92.05KiB`	`92.05KiB`

Bundle analysis report Branch p/test-slot-index Project dashboard

^{Generated by RelativeCI Documentation Report issue}

relativeci · 2026-04-07T02:38:44Z

React MTF Example

#214 Bundle Size — 207.38KiB (0%).

ad8126d(current) vs 9df4844 main#211(baseline)

Bundle metrics no changes

	Current #214	Baseline #211
Initial JS	`0B`	`0B`
Initial CSS	`0B`	`0B`
Cache Invalidation	`0%`	`0%`
Chunks	`0`	`0`
Assets	`3`	`3`
Modules	`174`	`174`
Duplicate Modules	`68`	`68`
Duplicate Code	`46.08%`	`46.08%`
Packages	`2`	`2`
Duplicate Packages	`0`	`0`

Bundle size by type no changes

	Current #214	Baseline #211
IMG	`111.23KiB`	`111.23KiB`
Other	`96.15KiB`	`96.15KiB`

Bundle analysis report Branch p/test-slot-index Project dashboard

^{Generated by RelativeCI Documentation Report issue}

test: check __current_slot_index in snapshot

ad8126d

HuJean requested review from Yradex and hzy as code owners April 7, 2026 02:23

coderabbitai Bot added the framework:React label Apr 7, 2026

coderabbitai Bot reviewed Apr 7, 2026

View reviewed changes

Comment thread packages/react/runtime/src/snapshot/snapshot.ts

HuJean enabled auto-merge (squash) April 7, 2026 02:55

Yradex approved these changes Apr 7, 2026

View reviewed changes

HuJean merged commit 55602e2 into main Apr 7, 2026
77 of 80 checks passed

HuJean deleted the p/test-slot-index branch April 7, 2026 03:00

This was referenced Apr 21, 2026

feat: reduce slotIndex to save json serialization time #2486

Merged

chore(react): bump @lynx-js/internal-preact + fix SlotV2 cross-wrapper InsertBefore #2512

Merged

Conversation

HuJean commented Apr 7, 2026 • edited by coderabbitai Bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary by CodeRabbit

Checklist

Uh oh!

changeset-bot Bot commented Apr 7, 2026

🦋 Changeset detected

Uh oh!

coderabbitai Bot commented Apr 7, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Walkthrough

Changes

Estimated code review effort

Possibly related PRs

Suggested labels

Suggested reviewers

Poem

Uh oh!

codecov Bot commented Apr 7, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

coderabbitai Bot left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

relativeci Bot commented Apr 7, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

React External

#200 Bundle Size — 591.44KiB (0%).

Uh oh!

codspeed-hq Bot commented Apr 7, 2026

Merging this PR will not alter performance

Footnotes

Uh oh!

relativeci Bot commented Apr 7, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Web Explorer

#8656 Bundle Size — 728.84KiB (0%).

Uh oh!

relativeci Bot commented Apr 7, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

React Example

#7081 Bundle Size — 237.81KiB (0%).

Uh oh!

relativeci Bot commented Apr 7, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

React MTF Example

#214 Bundle Size — 207.38KiB (0%).

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

HuJean commented Apr 7, 2026 •

edited by coderabbitai Bot

Loading

coderabbitai Bot commented Apr 7, 2026 •

edited

Loading

codecov Bot commented Apr 7, 2026 •

edited

Loading

relativeci Bot commented Apr 7, 2026 •

edited

Loading

relativeci Bot commented Apr 7, 2026 •

edited

Loading

relativeci Bot commented Apr 7, 2026 •

edited

Loading

relativeci Bot commented Apr 7, 2026 •

edited

Loading