-
Notifications
You must be signed in to change notification settings - Fork 111
fix(react): remove and insert list-items those item-key have been changed
#1299
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
fix(react): remove and insert list-items those item-key have been changed
#1299
Conversation
🦋 Changeset detectedLatest commit: 7b6eb7a 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 |
WalkthroughThis change updates the logic for handling attribute changes in list items within the Changes
Sequence Diagram(s)sequenceDiagram
participant ReactLynx
participant ListUpdateInfoRecording
participant LynxEngine
ReactLynx->>ListUpdateInfoRecording: onSetAttribute(child, attr, oldAttr)
alt oldAttr missing or attr["item-key"] == oldAttr["item-key"]
ListUpdateInfoRecording->>ListUpdateInfoRecording: Update platformInfoUpdate map
else attr["item-key"] != oldAttr["item-key"]
ListUpdateInfoRecording->>ListUpdateInfoRecording: Call onInsertBefore(child, nextSibling)
ListUpdateInfoRecording->>LynxEngine: Schedule removal and insertion
end
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 (2)
🚧 Files skipped from review as they are similar to previous changes (1)
🔇 Additional comments (1)
✨ 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 React list items with changed item-key attributes were not being properly removed and reinserted. The fix ensures that when a list item's item-key changes (even if the React key stays the same), the item is treated as a removal and insertion operation rather than an update, which aligns with the Lynx Engine's expectations.
- Modified list update logic to detect
item-keychanges and trigger remove/insert operations - Updated type signatures to use proper
PlatformInfotypes instead ofany - Fixed null handling in platform info extraction to prevent errors when
oldValueis undefined
Reviewed Changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| packages/react/runtime/src/snapshot/spread.ts | Fixed null handling for oldValue and reordered assignment for proper platform info extraction |
| packages/react/runtime/src/snapshot/platformInfo.ts | Added type cast to ensure oldValue is treated as PlatformInfo |
| packages/react/runtime/src/listUpdateInfo.ts | Enhanced onSetAttribute to detect item-key changes and trigger remove/insert operations |
| .changeset/shaky-swans-end.md | Added changelog entry documenting the fix |
CodSpeed Performance ReportMerging #1299 will not alter performanceComparing Summary
|
React Example#3083 Bundle Size — 234.49KiB (+0.02%).7b6eb7a(current) vs ec7228f main#3063(baseline) Bundle metrics
Bundle size by type
Bundle analysis report Branch hzy:p/hzy/remove-insert-when-ite... Project dashboard Generated by RelativeCI Documentation Report issue |
Web Explorer#3073 Bundle Size — 304.59KiB (0%).7b6eb7a(current) vs ec7228f main#3054(baseline) Bundle metrics
Bundle size by type
|
| Current #3073 |
Baseline #3054 |
|
|---|---|---|
221.88KiB |
221.88KiB |
|
50.89KiB |
50.89KiB |
|
31.83KiB |
31.83KiB |
Bundle analysis report Branch hzy:p/hzy/remove-insert-when-ite... Project dashboard
Generated by RelativeCI Documentation Report issue
0eabade to
835cc64
Compare
Co-authored-by: Copilot <[email protected]> Signed-off-by: Zhiyuan Hong <[email protected]>
835cc64 to
7b6eb7a
Compare
…ave been changed" (#1353) Reverts #1299 <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **Refactor** * Simplified internal handling of attribute updates for list items, streamlining how changes to item attributes are processed. * Improved logic for managing old attribute values, ensuring more consistent and predictable updates. * **Tests** * Removed specific test cases related to list item attribute changes to align with updated logic. <!-- end of auto-generated comment: release notes by coderabbit.ai -->
Summary
Checklist
Summary by CodeRabbit
item-keychanges, ensuring consistent behavior across components.item-keyare handled and to illustrate expected behavior with examples.