-
Notifications
You must be signed in to change notification settings - Fork 111
fix(react): avoid some unexpected __SetAttribute when attribute value is undefined
#1311
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
🦋 Changeset detectedLatest commit: a96b7de 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 |
WalkthroughThe changes update the serialization format of the Changes
Sequence Diagram(s)sequenceDiagram
participant TestSuite
participant SnapshotInstance
participant SerializedSnapshot
TestSuite->>SnapshotInstance: Call toJSON()
SnapshotInstance->>SnapshotInstance: Convert __values array to object with string keys
SnapshotInstance-->>SerializedSnapshot: Return object with values as Record<string, unknown>
SerializedSnapshot-->>TestSuite: Used in test snapshot comparison
Suggested labels
Poem
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 ESLint
ERROR Cannot resolve version $@rspack/core in overrides. The direct dependencies don't have dependency "@rspack/core". 📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (7)
✅ Files skipped from review due to trivial changes (3)
🚧 Files skipped from review as they are similar to previous changes (4)
✨ Finishing Touches
🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This PR fixes an issue where undefined attribute values in React components were causing unexpected __SetAttribute operations during serialization. The change modifies how snapshot values are serialized by converting arrays to objects, which ensures undefined values are preserved during JSON serialization and prevents unnecessary attribute operations.
Key changes:
- Changed
valuestype fromany[]toRecord<string, unknown>in SerializedSnapshotInstance interface - Modified serialization logic to convert array values to objects using
Object.assign - Added test case to verify
undefinedvalues don't cause patches
Reviewed Changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| packages/react/runtime/src/snapshot.ts | Updates type definition and serialization logic to convert array values to objects |
| packages/react/runtime/test/snapshot/ref.test.jsx | Updates test expectations to reflect new object-based values format |
| packages/react/runtime/test/snapshot/event.test.jsx | Updates test expectations to reflect new object-based values format |
| packages/react/runtime/test/preact.test.jsx | Adds new test case for undefined attribute handling |
| packages/react/runtime/test/lifecycle/reload.test.jsx | Updates test expectations to reflect new object-based values format |
| .changeset/calm-lies-dig.md | Documents the fix for undefined attribute handling |
CodSpeed Performance ReportMerging #1311 will not alter performanceComparing Summary
|
…ue is `undefined`
React Example#3093 Bundle Size — 234.46KiB (~+0.01%).124a125(current) vs ec7228f main#3063(baseline) Bundle metrics
Bundle size by type
Bundle analysis report Branch hzy:p/hzy/perf-0718 Project dashboard Generated by RelativeCI Documentation Report issue |
Web Explorer#3083 Bundle Size — 304.59KiB (0%).124a125(current) vs ec7228f main#3054(baseline) Bundle metrics
Bundle size by type
|
| Current #3083 |
Baseline #3054 |
|
|---|---|---|
221.88KiB |
221.88KiB |
|
50.89KiB |
50.89KiB |
|
31.83KiB |
31.83KiB |
Bundle analysis report Branch hzy:p/hzy/perf-0718 Project dashboard
Generated by RelativeCI Documentation Report issue
…bute value is `undefined`" (#1317) Reverts #1311, as it causes the performance of `JSON.stringify(__root)` to degrade by about ~10%. <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **Bug Fixes** * Improved handling of undefined attribute values to prevent unintended behavior in intrinsic elements. * **Refactor** * Updated the serialization format of snapshot data, changing the representation of values from objects with numeric keys to arrays for improved consistency. * **Tests** * Modified multiple test snapshots to reflect the new array-based format for values. * Removed a test case related to patch emission for undefined values. <!-- end of auto-generated comment: release notes by coderabbit.ai -->
Summary
Checklist
Summary by CodeRabbit
Bug Fixes
New Features
Refactor
Tests