Fix unstable references from term-data bindings #73406
Draft
Conversation
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
|
The following accounts have interacted with this PR and/or linked issues. I will continue to update these lists as activity occurs. You can also manually ask me to refresh this list by adding the If you're merging code through a pull request on GitHub, copy and paste the following into the bottom of the merge commit message. To understand the WordPress project's expectations around crediting contributors, please review the Contributor Attribution page in the Core Handbook. |
|
Size Change: +17 B (0%) Total Size: 2.51 MB
ℹ️ View Unchanged
|
Contributor
|
FWIW, I also recently started a refactor of the |
Contributor
Not sure this premise is true though? |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
Drafted by Claude to demonstrate the issue - I think this code is too complex and likely isn't the root bug I wanted to raise the issue to those more familiar with it.
What?
Refactors the
core/term-datablock binding source to remove custom memoization and match thecore/post-datapattern, fixing unstable object references.Why?
The
core/term-databinding source used a custom memoization layer withcreateSelectorfrom rememo. This had several issues:clientIdvalues), the cache was constantly evicteddataFieldsobject viacreateDataFields(), even when the underlying entity data was the samecore/post-databinding source doesn't use custom memoization - it calls core-data selectors directlyThis issue is latent on trunk but becomes visible with timing-sensitive operations (like immediate block selection after link creation).
How?
getTermDataFieldsfunction wrapped increateSelectorgetEntityRecorddirectly ingetValues,canUserEditValue, andgetFieldsListconservativeMapItemto preserve object referencescore/post-dataworksThe fix eliminates the custom caching layer that was causing problems and delegates to core-data's proven memoization strategy.
Testing Instructions
Test 1: Verify Object Reference Stability
Apply this patch to add reference tracking:
Open the editor and create a new post
Insert a navigation block
Add 6+ category or tag links to the navigation menu
Click on different category links and interact with the navigation
Expected: Console shows
✅ STABLE REFERENCEmessages (same object returned for same term)Not expected:
⚠️ UNSTABLE REFERENCEwarningsTest 2: Compare with Old Implementation
getTermDataFieldsfunctionTest 3: Verify Functionality
Test 4: Run Unit Tests
Expected: All 19 tests pass, covering:
getValueswith context and block attributescanUserEditValuepermission checksgetFieldsListfield enumerationTest 5: Performance Check
post-databindingsTesting Instructions for Keyboard
Screenshots or screencast