fix: avoid crash when spread undefined ref

[upupming]

Description

Fixed a potential crash in @lynx-js/react when spreading props that include an undefined ref. This occurs because applyRef was being called even when the ref value was null or undefined in a spread object.

Added a regression test in packages/react/testing-library.

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).

Summary by CodeRabbit

• Bug Fixes
  • Fixed a crash that occurred when spreading properties containing undefined refs onto React components, improving application stability and reliability.

[changeset-bot]

🦋 Changeset detected

Latest commit: bde489a

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

This PR includes changesets to release 1 package

Name	Type
@lynx-js/react	Patch

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]

📝 Walkthrough

Walkthrough

Adds a patch fix for @lynx-js/react to prevent crashes when spreading undefined refs. The fix guards against applying falsy refs in the removeChild cleanup method and includes a test case validating the behavior.

Changes

Cohort / File(s)	Summary
Changeset metadata .changeset/fix-spread-undefined-ref.md	Documents patch release for @lynx-js/react addressing undefined ref spread crash.
Ref cleanup guard packages/react/runtime/src/backgroundSnapshot.ts	Adds truthy check for value.ref before calling applyRef in removeChild to prevent applying falsy refs.
Test coverage packages/react/testing-library/src/__tests__/ref.test.jsx	Adds test case "spread undefined ref should work" verifying that spreading undefined ref onto a component works without error.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

Possibly related PRs

• refactor(react/runtime): improve cleanup of snapshot instances #1293: Refactors and centralizes ref cleanup logic in backgroundSnapshot.ts's removeChild method.
• fix(react): avoid some unexpected __SetAttribute when attribute value is undefined #1311: Addresses broader handling of undefined values in React runtime's element and ref processing.

Suggested labels

framework:React

Suggested reviewers

• HuJean
• Yradex
• hzy

Poem

🐰 A ref spread undefined? Oh what a fright!
But now with a guard, everything's right!
We check if it's truthy before we apply,
No crashes today—just smooth sailing by! ✨

🚥 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 clearly summarizes the main fix: avoiding a crash when spreading undefined refs, which aligns with the primary code change in backgroundSnapshot.ts and the regression test added.
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 (stacked PR)
• 📝 Generate docstrings (commit on current branch)

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Post copyable unit tests in a comment
• Commit unit tests in branch fix/spread-undefined-ref

📝 Coding Plan

• Generate coding plan for human review comments

---

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

• X
• Mastodon
• Reddit
• LinkedIn

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

[coderabbitai]

🧹 Nitpick comments (1)

packages/react/testing-library/src/__tests__/ref.test.jsx (1)

481-484: Add one post-toggle assertion to prove the unmount path executed.

This keeps the regression test explicit instead of only relying on “no throw”.

✅ Suggested test hardening

   act(() => {
     setShowChild(false);
   });
+  expect(elementTree).toMatchInlineSnapshot(`<page />`);

🤖 Prompt for AI Agents

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

In `@packages/react/testing-library/src/__tests__/ref.test.jsx` around lines 481 -
484, After calling act(() => { setShowChild(false); }) add a concrete assertion
that the unmount path executed (don’t just rely on "no throw"); for example, use
the test utilities used earlier in the file (queryByText/queryByTestId or
container) to assert the child is removed from the DOM (e.g.
expect(queryByTestId('child') or queryByText('child label')).toBeNull() or
expect(container).not.toContainElement(childElement)), or assert any unmount
callback/flag used in the test was called (e.g.
expect(onUnmountMock).toHaveBeenCalled()) so the test explicitly verifies the
unmount path after setShowChild and act.

🤖 Prompt for all review comments with AI agents

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

Nitpick comments:
In `@packages/react/testing-library/src/__tests__/ref.test.jsx`:
- Around line 481-484: After calling act(() => { setShowChild(false); }) add a
concrete assertion that the unmount path executed (don’t just rely on "no
throw"); for example, use the test utilities used earlier in the file
(queryByText/queryByTestId or container) to assert the child is removed from the
DOM (e.g. expect(queryByTestId('child') or queryByText('child
label')).toBeNull() or expect(container).not.toContainElement(childElement)), or
assert any unmount callback/flag used in the test was called (e.g.
expect(onUnmountMock).toHaveBeenCalled()) so the test explicitly verifies the
unmount path after setShowChild and act.

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: 8cf987e8-5c9e-4c64-88e7-e8770dc4b60f

📥 Commits

Reviewing files that changed from the base of the PR and between 045320e and bde489a.

📒 Files selected for processing (3)

• .changeset/fix-spread-undefined-ref.md
• packages/react/runtime/src/backgroundSnapshot.ts
• packages/react/testing-library/src/__tests__/ref.test.jsx

[codecov]

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!

[relativeci]

Web Explorer

#8141 Bundle Size — 384.5KiB (0%).

bde489a(current) vs 045320e main#8139(baseline)

Bundle metrics   1 change

	Current #8141	Baseline #8139
Initial JS	155.59KiB	155.59KiB
Initial CSS	35.1KiB	35.1KiB
Cache Invalidation	0%	0%
Chunks	8	8
Assets	8	8
Modules	238(-0.42%)	239
Duplicate Modules	16	16
Duplicate Code	2.98%	2.98%
Packages	4	4
Duplicate Packages	0	0

Bundle size by type  no changes

	Current #8141	Baseline #8139
JS	253.55KiB	253.55KiB
Other	95.85KiB	95.85KiB
CSS	35.1KiB	35.1KiB

Bundle analysis report Branch fix/spread-undefined-ref Project dashboard

---

^{Generated by RelativeCI Documentation Report issue}

[codspeed-hq]

Merging this PR will degrade performance by 8.37%

⚡ 1 improved benchmark
❌ 1 regressed benchmark
✅ 70 untouched benchmarks
⏩ 3 skipped benchmarks¹

⚠️ Please fix the performance issues or acknowledge them on CodSpeed.

Performance Changes

	Benchmark	BASE	HEAD	Efficiency
❌	basic-performance-nest-level-100	6.8 ms	7.4 ms	-8.37%
⚡	basic-performance-text-200	19 ms	17.7 ms	+7.27%

---

_{Comparing fix/spread-undefined-ref (bde489a) with main (045320e)}

Footnotes

1. 3 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. ↩