Core Data: Cleanup edits matching persisted record on undo/redo#77100
Draft
Core Data: Cleanup edits matching persisted record on undo/redo#77100
Conversation
|
Size Change: +102 B (0%) Total Size: 7.75 MB 📦 View Changed
ℹ️ View Unchanged
|
4e413bb to
9014b87
Compare
Mamaduka
commented
Apr 7, 2026
Comment on lines
+165
to
+176
| // If the value matches the persisted record, clear | ||
| // the edit so the entity is no longer dirty for | ||
| // this property. | ||
| const persisted = | ||
| persistedRecord?.[ key ]?.raw ?? | ||
| persistedRecord?.[ key ]; | ||
| return [ | ||
| key, | ||
| fastDeepEqual( editValue, persisted ) | ||
| ? undefined | ||
| : editValue, | ||
| ]; |
Member
Author
There was a problem hiding this comment.
@youknowriad, this is still a draft, but do you recall whether a similar cleanup was intentionally omitted during the UndoManager refactoring?
Contributor
There was a problem hiding this comment.
It doesn't ring a bell. I'd say it was probably not mandatory, but I'm not entirely clear about implications.
Member
Author
Member
There was a problem hiding this comment.
Hmm, the fix makes sense to me - editEntityRecord already does this exact comparison, and it was missing in withMultiEntityRecordEdits.
One thing worth doublechecking: here the persisted value is read directly from state?.queriedData?.items?.default?.[ recordId ] with a .raw fallback, while editEntityRecord uses the getRawEntityRecord selector. Worth checking that the selector doesn't do additional normalization beyond .raw unwrapping - just in case. We might want to do the same - for consistency, if for nothing else.
Member
Author
There was a problem hiding this comment.
I recently worked on getRawEntityRecord refactoring, and it just unwraps raw values or bails early. This is just a simplified version of it with fewer internal deps.
Initially, I had some doubts about using the default context (state?.queriedData?.items?.default?.[ recordId ]), but both getRawEntityRecord and getEditedEntityRecord grab a record from it.
Mamaduka
commented
Apr 8, 2026
Comment on lines
+197
to
+224
| // Clear edits that match the persisted record | ||
| // so the entity is no longer dirty after undo. | ||
| const persistedRecord = select.getEntityRecord( | ||
| kind, | ||
| name, | ||
| key | ||
| ); | ||
| if ( persistedRecord ) { | ||
| edits = Object.fromEntries( | ||
| Object.entries( edits ).map( | ||
| ( [ editKey, editValue ] ) => { | ||
| const persisted = | ||
| persistedRecord[ editKey ] | ||
| ?.raw ?? | ||
| persistedRecord[ editKey ]; | ||
| return [ | ||
| editKey, | ||
| fastDeepEqual( | ||
| editValue, | ||
| persisted | ||
| ) | ||
| ? undefined | ||
| : editValue, | ||
| ]; | ||
| } | ||
| ) | ||
| ); | ||
| } |
Member
Author
There was a problem hiding this comment.
Trying out something for the RTC fix. @chriszarate, does this look correct to you?
Contributor
There was a problem hiding this comment.
I tested this locally with RTC and it works well. I don't see any issues with it.
Member
Author
There was a problem hiding this comment.
Thanks for the confirmation, @alecgeatches!
The failing e2e might be related, will try to debug it tomorrow.
When undoing all changes, `withMultiEntityRecordEdits` replayed original values as edits without comparing them to the persisted record. This left the entity marked dirty even though no actual changes remained. Compare each undo/redo value against the persisted record in `withMultiEntityRecordEdits` and set matching edits to `undefined` so the edits reducer clears them.
1923c46 to
1d33546
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
4 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What?
🚧 Just runnign the e2e tests 🚧
Closes #24679.
When undoing all changes,
withMultiEntityRecordEditsreplayed original values as edits without comparing them to the persisted record. This left the entity marked dirty even though no actual changes remained.Compare each undo/redo value against the persisted record in
withMultiEntityRecordEditsand set matching edits toundefinedso the edits reducer clears them.Todo
Why?
How?
Testing Instructions
Testing Instructions for Keyboard
Screenshots or screencast
Use of AI Tools