Sync view configuration API updates from Gutenberg - #12638
Conversation
Ports the latest iteration of the view configuration API from the Gutenberg update/view-config-api-versioning branch: - Rework the WP_View_Config_Data write API around merge(), replace(), set(), and remove(), all operating on patches of top-level keys and taking the schema version the change was authored against. - Merge lists (view_list, form fields, children) by member identity (id, slug, or field), dropping null members from incoming patches. - Make get_data() private and move filter application into a new apply_filters() method so callbacks cannot read the materialized configuration and become coupled to its shape. - Update the default post type configuration callbacks to use the new single-patch set() signature. - Adapt and expand the unit tests accordingly.
|
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 Core Committers: Use this line as a base for the props when committing in SVN: To understand the WordPress project's expectations around crediting contributors, please review the Contributor Attribution page in the Core Handbook. |
Test using WordPress PlaygroundThe changes in this pull request can previewed and tested using a WordPress Playground instance. WordPress Playground is an experimental project that creates a full WordPress instance entirely within the browser. Some things to be aware of
For more details about these limitations and more, check out the Limitations page in the WordPress Playground documentation. |
There was a problem hiding this comment.
Pull request overview
This pull request syncs WordPress core’s entity view configuration API with the latest iteration from Gutenberg, updating the WP_View_Config_Data write API and adapting core providers and tests to the new patch-based semantics.
Changes:
- Refactors
WP_View_Config_Datawrite operations aroundmerge(),replace(),set(), and introducesremove(), with schema-versioned patches. - Moves filter application behind
WP_View_Config_Data::apply_filters()and keeps the materialized config private. - Updates default post type config providers and expands PHPUnit coverage for the new behaviors.
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| tests/phpunit/tests/view-config.php | Updates API-level tests to use the new patch-based methods and adds coverage for merge/replace/remove behaviors. |
| tests/phpunit/tests/view-config-data.php | Rewrites container unit tests to validate new merge/replace/set/remove semantics; uses reflection to read private state for assertions. |
| tests/phpunit/tests/rest-api/rest-view-config-controller.php | Updates REST controller tests to use merge() with the new view_list patch shape. |
| src/wp-includes/view-config.php | Routes entity view config filtering through WP_View_Config_Data::apply_filters() and updates default post type callbacks to the new set()/merge() signatures. |
| src/wp-includes/class-wp-view-config-data.php | Implements the new patch application engine (apply()), list identity merging, null stripping, and remove() semantics; makes materialized config private. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| * An instance of this class is what `get_entity_view_config_{$kind}_{$name}` | ||
| * filter callbacks receive: a callback changes the configuration by calling | ||
| * methods on the instance and returning it. The configuration has four | ||
| * top-level keys — `default_view`, `default_layouts`, `view_list`, and | ||
| * `form` — and there are two ways to contribute: | ||
| * `form` — and there are three ways to contribute. They form a gradient of how |
| * A change that declares an unsupported schema version is rejected and does | ||
| * not alter anything. Callbacks mutate the container in place, so there is no | ||
| * need to return it; any returned value is ignored. Callbacks must not replace | ||
| * the container with a different value, as later callbacks receive whatever the | ||
| * the previous one returned. |
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 5 out of 5 changed files in this pull request and generated no new comments.
Comments suppressed due to low confidence (2)
src/wp-includes/class-wp-view-config-data.php:187
apply_filters()passes each callback’s return value to the next callback, so the current implementation can break the filter chain if a callback follows the docs and returns nothing (next callback would receivenullinstead ofWP_View_Config_Data). If the intent is “mutate in place and ignore returns”, this should dispatch as an action (or otherwise ensure the container instance is preserved for every callback).
apply_filters(
"get_entity_view_config_{$kind}_{$name}",
$this,
array(
'kind' => $kind,
'name' => $name,
)
);
src/wp-includes/class-wp-view-config-data.php:165
- The docblock says callback return values are ignored, but this method currently uses
apply_filters()(which threads the return value through subsequent callbacks). Also, there’s a duplicated word (“the the”). If the hook is dispatched as an action, update the wording accordingly so it matches runtime behavior.
* A change that declares an unsupported schema version is rejected and does
* not alter anything. Callbacks mutate the container in place, so there is no
* need to return it; any returned value is ignored. Callbacks must not replace
* the container with a different value, as later callbacks receive whatever the
* the previous one returned.
jorgefilipecosta
left a comment
jorgefilipecosta
left a comment
There was a problem hiding this comment.
There are some hedge cases that I would want to discuss further but nothing catastrophic or a blocker. Manually testing things seem to work well (during my smoke tests I did not found issues), there is extensive test coverage, so I think this is ready to ship.
jorgefilipecosta
left a comment
jorgefilipecosta
left a comment
There was a problem hiding this comment.
I found some hedge cases while reviewing rather that going back and forth I proposed a fix for the cases I found at WordPress/gutenberg#80571, #12644. For each case I included a unit test, and left a comment with the output that was happening before the fix e.g: on this PR.
| * | ||
| * A change that declares an unsupported schema version is rejected and does | ||
| * not alter anything. Callbacks mutate the container in place, so there is no | ||
| * need to return it; any returned value is ignored. Callbacks must not replace |
There was a problem hiding this comment.
This comment is wrong we need to return the data. I created a small doc fix PR at #12641.
Trac ticket: https://core.trac.wordpress.org/ticket/65577
Backports: WordPress/gutenberg#80319
Follow-up to: #12391
What
Ports the latest iteration of the view configuration API from the Gutenberg update/view-config-api-versioning branch:
Why
How
WP_View_Config_Datawrite API aroundmerge(),replace(),set(), andremove(), all operating on patches of top-level keys and taking the schema version the change was authored against.get_data()private and move filter application into a new apply_filters() method so callbacks cannot read the materialized configuration and become coupled to its shape.set()signature.Use of AI Tools
AI assistance: Yes
Tool(s): Claude Code, Zed agent
Model(s): Opus, Fabble
Used for: Initial code skeleton and test suggestions; final implementation and tests were reviewed and edited by me.