Block Bindings: Move computation of sources into dedicated useSelect
#72974
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
ockham
added
[Type] Bug
ockham
commented
Nov 4, 2025
| export const getBlockBindingsSourcesForBlock = createRegistrySelector( | ||
| ( select ) => | ||
| createSelector( | ||
| ( state, blockName, blockContext ) => { |
Contributor
Author
There was a problem hiding this comment.
The selector doesn't currently need the blockName; however, since we're memoizing it, we need it to ensure that we get a fresh list of sources if the user selects a block of a different type.
I'm planning to add logic to only return sources and source items that are compatible with the block's attributes, at which point we'll need blockName anyway.
2 tasks
|
Size Change: -51 B (0%) Total Size: 2.49 MB
ℹ️ View Unchanged
|
Contributor
Author
|
e2e tests are failing because some sources aren't present in the dropdown. I've debugged a bit, and it seems like Sooo we'll need to find a way to make sure that we wait for resolvers to return before reading selectors inside of a source's Fun. |
Contributor
Author
Oh, maybe it's not all that bad. 8f876bc seems to fix the issue. I hope that losing that layer of caching isn't too bad for performance. OTOH, e2e tests in |
Contributor
Author
|
I've filed a PR that removes the workaround from the e2e tests, to demonstrate that they fail in CI without the workaround: #73001 |
This was referenced Nov 5, 2025
|
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. |
sirreal
force-pushed
the
add/block-bindings-selector-for-sources
branch
from
6118f88 to
89d4334
Compare
This comment was marked as outdated.
This comment was marked as outdated.
ockham
added
the
Backport to WP 6.9 Beta/RC
github-project-automation
bot
moved this to 🔎 Needs Review
in WordPress 6.9 Editor Tasks
ellatrix
moved this from 🔎 Needs Review
to 🏗️ In Progress
in WordPress 6.9 Editor Tasks
ellatrix
moved this from 🏗️ In Progress
to 🔎 Needs Review
in WordPress 6.9 Editor Tasks
ockham
removed
the
Backport to WP 6.9 Beta/RC
ockham
moved this from 🔎 Needs Review
to 🐛 Punted to 6.9.1
in WordPress 6.9 Editor Tasks
ockham
force-pushed
the
add/block-bindings-selector-for-sources
branch
from
ed5af86 to
72b8d56
Compare
ockham
force-pushed
the
add/block-bindings-selector-for-sources
branch
from
379d683 to
837336f
Compare
There was a problem hiding this comment.
Pull Request Overview
This PR refactors the block bindings sources computation by moving it into a dedicated useSelect hook to fix race conditions and eliminate referential equality hacks. The changes remove workarounds from e2e tests that were previously needed to mask timing issues.
- Separates sources computation from other settings into its own
useSelecthook - Restructures the
sourcesdata structure to map source names directly to field items instead of wrapping them in objects - Removes test workarounds that manually created bindings through the UI
Reviewed Changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
test/e2e/specs/editor/various/block-bindings/post-meta.spec.js |
Removes UI interaction workarounds and directly inserts blocks with bindings configured |
packages/block-editor/src/hooks/block-bindings.js |
Refactors sources computation into dedicated useSelect, simplifies data structure, and updates references throughout |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Co-authored-by: Jon Surrell <[email protected]>
sirreal
approved these changes
Nov 17, 2025
Member
sirreal
left a comment
sirreal
left a comment
There was a problem hiding this comment.
This seems to improve the problems it sets out to and I don't see cause for concern. Looks good.
I had problems the other day with tests, but it was due to an older WordPress version running in my environment.
Co-authored-by: Copilot <[email protected]>
github-project-automation
bot
moved this from 🐛 Punted to 6.9.1
to ✅ Done
in WordPress 6.9 Editor Tasks
Contributor
Author
|
We tried to get this into RC1 last week but didn't quite make it in time since we still had to sort out some problems. |
ockham
added
the
Backport to WP 6.9 Beta/RC
github-actions
bot
removed
the
Backport to WP 6.9 Beta/RC
gutenbergplugin
pushed a commit
that referenced
this pull request
Nov 17, 2025
#72974) There are a few hacks and workarounds in Block Bindings to make them work. One example is the definition of `const _sources` outside of the following `useSelect` statement, which was done to avoid a console warning ("The `useSelect` hook returns different values when called with the same state and parameters"). Additionally, e2e tests had workarounds to make them pass in CI. This commit moves `sources` computation into a dedicated `useSelect`, applies some general refactoring, and removes e2e workarounds. Co-authored-by: ockham <[email protected]> Co-authored-by: gziolo <[email protected]> Co-authored-by: sirreal <[email protected]>
github-actions
bot
added
the
Backported to WP Core
|
I just cherry-picked this PR to the wp/6.9 branch to get it included in the next release: 7b1dc7f |
Member
|
What does this fix other than removing hacks? Is this a regression in 6.9? RC is meant to be quite strict so changes don't introduce new bugs. |
Contributor
Author
|
This fixes a race condition that was visible when running e2e tests locally: When first opening the dropdown of available sources for a given bindable attribute, the The responsible code has probably been around longer than 6.9; however, we made some other changes during the 6.9 cycle which might have made the issue apparent. With this change, e2e tests now also pass locally (and with the workaround removed, we ensure that they would also fail in CI if the previous race condition reappeared). Overall, I think that that test coverage provides some confidence that the change is working fine; however, I realize that it's a fairly big change for RC2. I'd like to leave the ultimate decision with you; I'm fine if we revert the commit on the |
Contributor
Author
|
Per some additional conversation via DM, I agree that it's probably better to err on the side of caution and revert (on the 6.9 branch). I'll do that now. |
Contributor
Author
|
Reverted on the |
priethor
removed
the
Backported to WP Core
This was referenced Nov 20, 2025
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.
What?
There are a few hacks and workarounds in Block Bindings to make them work. One example is the definition of
const _sourcesoutside of the followinguseSelectstatement:gutenberg/packages/block-editor/src/hooks/block-bindings.js
Lines 300 to 305 in 2b6c500
This was done to avoid the following console warning:
However, this papered over an underlying issue (through forced referential equality).
Additionally, e2e tests had workarounds to make them pass in CI:
gutenberg/test/e2e/specs/editor/various/block-bindings/post-meta.spec.js
Lines 147 to 155 in 2b6c500
gutenberg/test/e2e/specs/editor/various/block-bindings/post-meta.spec.js
Lines 183 to 191 in 2b6c500
However, those tests (and the five in the
Fields list dropdowngroup) never passed when run locally. In other words, the workaround papered over a race condition in CI (but couldn't prevent tests from failing locally).See #65604 for a longer discussion of this issue, and #72367 (comment) for the rationale behind the present approach.
See #67885 and #72109 for further previous attempts at fixing the underlying issue.
Why?
To improve stability, remove hacks, and ensure that e2e tests actually guard against regressions.
How?
By moving
sourcescomputation into a dedicateduseSelect, and some general refactoring.Testing Instructions
See CI, but also try some smoke testing. (Refer to other Block Bindings related PRs for some ideas.)
Additionally, verify that all tests from the
post-meta.spec.jsare now passing when run locally.Follow-up
In a future iteration, we could consider introducing more granular selectors, but that can be done separately from this bugfix:
getBlockBindingsSourcesForBlockAttribute( state, blockName, blockContext, blockAttribute )selector.