Skip to content

Trac63946/rest lazy load namespaces#10080

Open
prettyboymp wants to merge 16 commits intoWordPress:trunkfrom
prettyboymp:trac63946/rest-lazy-load-namespaces
Open

Trac63946/rest lazy load namespaces#10080
prettyboymp wants to merge 16 commits intoWordPress:trunkfrom
prettyboymp:trac63946/rest-lazy-load-namespaces

Conversation

@prettyboymp
Copy link

Adds support for registering namespaces to the REST API Server for the purposes of lazy loading. The server will trigger a action specific to that namespace when corresponding routes would need to be loaded. This provides a performance improvement when adding new namespaces and routes by reducing the number of routes that need to be checked for matches and avoiding unnecessary translation processing.

Trac ticket: https://core.trac.wordpress.org/ticket/63946


This Pull Request is for code review only. Please keep all other discussion in the Trac ticket. Do not merge this Pull Request. See GitHub Pull Requests for Code Review in the Core Handbook for more details.

@github-actions
Copy link

Hi @prettyboymp! 👋

Thank you for your contribution to WordPress! 💖

It looks like this is your first pull request to wordpress-develop. Here are a few things to be aware of that may help you out!

No one monitors this repository for new pull requests. Pull requests must be attached to a Trac ticket to be considered for inclusion in WordPress Core. To attach a pull request to a Trac ticket, please include the ticket's full URL in your pull request description.

Pull requests are never merged on GitHub. The WordPress codebase continues to be managed through the SVN repository that this GitHub repository mirrors. Please feel free to open pull requests to work on any contribution you are making.

More information about how GitHub pull requests can be used to contribute to WordPress can be found in the Core Handbook.

Please include automated tests. Including tests in your pull request is one way to help your patch be considered faster. To learn about WordPress' test suites, visit the Automated Testing page in the handbook.

If you have not had a chance, please review the Contribute with Code page in the WordPress Core Handbook.

The Developer Hub also documents the various coding standards that are followed:

Thank you,
The WordPress Project

@github-actions
Copy link

github-actions bot commented Sep 29, 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 props-bot label.

Core Committers: Use this line as a base for the props when committing in SVN:

Props prettyboymp, swissspidy, timothyblynjacobs, kraftbj.

To understand the WordPress project's expectations around crediting contributors, please review the Contributor Attribution page in the Core Handbook.

@github-actions
Copy link

Test using WordPress Playground

The 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

  • The Plugin and Theme Directories cannot be accessed within Playground.
  • All changes will be lost when closing a tab with a Playground instance.
  • All changes will be lost when refreshing the page.
  • A fresh instance is created each time the link below is clicked.
  • Every time this pull request is updated, a new ZIP file containing all changes is created. If changes are not reflected in the Playground instance,
    it's possible that the most recent build failed, or has not completed. Check the list of workflow runs to be sure.

For more details about these limitations and more, check out the Limitations page in the WordPress Playground documentation.

Test this pull request with WordPress Playground.

* ) );
* } );
*/
do_action( 'rest_lazy_load_namespace_' . $route_namespace );
Copy link
Author

@prettyboymp prettyboymp Oct 2, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This somewhat breaks from convention around only using underscores for hooks since by their own convention namespaces are almost always going to contain dashes and slashes. Would it be better to convert these for the hook for the sake of convention or leave it as is?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This also isn't a recommended pattern anymore. The namespace should just be it's own parameter.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This also isn't a recommended pattern anymore. The namespace should just be it's own parameter.

@TimothyBJacobs, do you happen to have a reference to some relevant discussion around this? There seem to still be (relatively) recent hooks added with this pattern.

I would hate for every callback created for lazy route registration to have to start with if( '/my-plugin-namespace' === $route_namespace )....

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Usually WP provides hooks for both. e.g. do_action( 'rest_lazy_load_namespace_' . $route_namespace, $route_namespace ); and do_action( 'rest_lazy_load_namespace', $route_namespace );

@kraftbj
Copy link

kraftbj commented Feb 10, 2026

I rebased against trunk and performed a Claude review. There may be some issues that need to be addressed so sharing them here before looking deeper and/or asking for more eyes.


Review Notes

Positives

  • Clear performance win for sites with many REST namespaces. Avoids loading/translating routes that are never hit.
  • Good test coverage — 12+ new test cases covering registration, dispatch, idempotent loading, mixed lazy/regular, double registration, namespace index, root index, and "no unnecessary loading."
  • Non-breaking — existing register_rest_route() continues to work identically. Lazy loading is opt-in.
  • Clean dispatch integration — match_request_to_handler() checks both $this->namespaces and $this->lazy_namespaces when matching, and only loads the specific namespace needed.

Issues & Concerns

1. get_index() doesn't include lazy namespaces in the namespaces array (Possible Bug)

After calling $this->load_all_lazy_namespaces(), the index still only reports:

'namespaces' => array_keys( $this->namespaces ),

If a lazy-loaded namespace's action registers routes via register_rest_route(), those routes will populate $this->namespaces as a side effect. But if the lazy load action fails or registers nothing, the namespace won't appear in the index. This should probably be:

'namespaces' => array_keys( $this->namespaces + $this->lazy_namespaces ),

2. get_namespaces() doesn't include lazy namespaces

get_namespaces() still returns only array_keys( $this->namespaces ). Callers outside the class who use this public method won't see lazy-registered namespaces, even after they've been loaded. The test test_lazy_namespace_registration explicitly asserts this as expected behavior, but it seems like a design gap — once a lazy namespace has been loaded, shouldn't it appear in get_namespaces()? There's a todo comment in the test acknowledging this open question.

3. Namespace matching in match_request_to_handler() has a prefix collision issue

if ( str_starts_with( trailingslashit( ltrim( $path, '/' ) ), $namespace ) ) {

This checks if the path starts with the namespace string, but without a trailing slash on $namespace. So a request to /test/v1-extended/foo would match namespace test/v1 because "test/v1-extended/foo/" starts with "test/v1". This could cause unnecessary lazy-loading of the wrong namespace. The fix would be:

if ( str_starts_with( trailingslashit( ltrim( $path, '/' ) ), trailingslashit( $namespace ) ) ) {

(Note: this bug likely pre-exists in trunk's match_request_to_handler, but lazy loading makes it more impactful since it triggers side effects.)

4. Validation order in register_rest_namespace() is suboptimal

The empty() check runs before the is_string() check. A non-string value like an array would hit the empty() branch first and try to use it in a sprintf, which could produce warnings. The is_string() check should come first.

5. Typo in comment

"namespases" should be "namespaces" (line 1276 area).

6. Missing @since tags on several new methods

register_lazy_loaded_namespace(), load_lazy_namespace(), and load_all_lazy_namespaces() are all missing @since tags in their docblocks, while other new additions (constants, properties) have @since X.X.0.

7. rest_lazy_load_namespace generic action docblock is sparse

The generic rest_lazy_load_namespace action has a minimal docblock compared to the namespace-specific one. It should document the $route_namespace parameter that's passed.

8. No test for register_rest_namespace() validation paths

The global register_rest_namespace() function has validation for empty namespace, non-string namespace, leading/trailing slashes, and calling before rest_api_init. None of these validation branches are tested.

9. Route priority tests seem unrelated to lazy loading

test_route_priority_registration_order and test_route_priority_reverse_registration_order test existing WP_REST_Server behavior (route matching order), not lazy loading. They're tagged with ticket 63946 but don't use lazy namespaces. They may be here to document assumptions the lazy loading depends on, but that should be clarified in the test docblocks.

- Fix namespace prefix collision in match_request_to_handler() by
  adding trailingslashit() to namespace comparison.
- Reorder validation in register_rest_namespace() so is_string()
  check runs before empty() to avoid array-to-string warnings.
- Fix "namespases" typo.
- Add missing @SInCE tags to new methods.
- Flesh out rest_lazy_load_namespace action docblock with description
  and @param.
- Clarify route priority test docblocks to explain their relevance
  to lazy loading changes.
- Resolve @todo in test: lazy namespaces intentionally excluded from
  get_namespaces() until loaded.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
@prettyboymp
Copy link
Author

@kraftbj, thanks for the reviewing. I addressed several items in 9233b0f.

1. get_index() doesn't include lazy namespaces in the namespaces array - By design. The contract of register_rest_namespace() is that routes will be registered when the lazy load hook fires. After load_all_lazy_namespaces(), any correctly implemented lazy namespace will have called register_rest_route(), which populates $this->namespaces. A namespace that registers nothing is a misconfiguration and correctly omitted from the index.

2. get_namespaces() doesn't include lazy namespaces - Intentional for backward compatibility. get_namespaces() returns namespaces that have registered routes. Including unloaded lazy namespaces would either require triggering all lazy loads as a side effect (defeating the purpose) or returning namespaces with no routes (misleading to callers). Any existing namespace migrating to lazy loading takes on this trade-off explicitly. Resolved the @todo in the test to clarify this is expected behavior.

3. Namespace prefix collision — Fixed. Added trailingslashit( $namespace ) to the str_starts_with check so test/v1 no longer falsely matches test/v1-extended/... paths. This is safe - actual route matching is still done by regex, so no backward compatibility concern.

4. Validation order - Fixed. is_string() check now runs before empty().

5. Typo — Fixed.

6. Missing @since tags - Fixed. Added @since X.X.0 to all three new methods.

7. Generic action docblock - Fixed. Added full description and @param documentation.

8. Validation path tests - Skipping for now, but happy to add if we think its helpful. There's no existing precedent for testing _doing_it_wrong validation on global REST API functions in the test suite.

9. Route priority test docblocks - Fixed. Added clarifying text that these document existing route priority behavior that the lazy loading changes to match_request_to_handler() must preserve.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants