Make WordPress Core

Opened 5 months ago

Last modified 38 hours ago

#65215 assigned defect (bug)

Connectors: REST API responses on the Connectors screen should be preloaded

Reported by: westonruter Owned by: westonruter
Priority: normal Milestone: 7.2
Component: AI Version: 7.0
Severity: normal Keywords: connectors has-patch has-unit-tests
Cc: Focuses: rest-api, performance

Description (last modified by westonruter)

For screenshots and for what this backports, see โ€‹Gutenberg PR.

What?

The Connectors screen (options-general.php?page=options-connectors-wp-admin) fires several REST API requests after hydration, delaying the Largest Contentful Paint.

Specifically: GET /wp/v2/settings (site entity), OPTIONS /wp/v2/plugins (the canUser( 'create', 'plugin' ) check), a hardcoded GET /wp/v2/plugins/ai/ai used by the AI-plugin callout, and one GET /wp/v2/plugins/<basename> per registered connector โ€” five sequential round-trips with the default connectors.

Why?

These responses are all trivially knowable at render time on the server, so they should be embedded in the initial HTML via createPreloadingMiddleware rather than re-fetched by the client after hydration. This also reduces the number of requests on the server, reducing server load.

Measured impact (median LCP over 10 runs)

Scenario Trunk This PR ฮ”
Fast 4G 1523 ms 1150 ms โˆ’373 ms (โ‰ˆ24.5%)
No throttling 743 ms 518 ms โˆ’225 ms (โ‰ˆ30.3%)

How?

Adds a {page-slug}-wp-admin_preload_paths filter to the wp-build page-wp-admin template so any generated admin page can register page-specific preload paths. Hooks it in the Connectors loader to preload the five paths listed in What? above (the existing hardcoded preload in the template only covers the root site fields, not the settings entity the UI actually resolves, so /wp/v2/settings is added too).

After the change, none of the connector-related REST requests fire on page load โ€” they are served from the preload middleware.

The two remaining requests come from @wordpress/core-abilities, which Gutenberg enqueues on every admin page โ€” out of scope here, but a candidate for similar preloading.


To achieve the following, REST API responses for 404 Not Found need to be served to the preload middleware. This is a follow up to r44123 and r44172.

Change History (24)

This ticket was mentioned in โ€‹PR #11790 on โ€‹WordPress/wordpress-develop by โ€‹@westonruter.


5 months ago
#1

  • Keywords has-patch has-unit-tests added

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

This backports the logic in this Gutenberg PR:

Additionally, improves REST API preloading in a few ways ways:

First of all, support is added to allow a non-OK response for a preloaded request to be sent to the preloading middleware. Without this, all of the plugins on the Connectors screen would cause unconditional 404 REST API responses when accessing the page:

https://github.com/user-attachments/assets/8a94aca9-07da-4193-9acb-d6f1c1233846

The data format for a preloaded request was previously two possibilities:

  1. string: The REST API path to preload.
  2. array{ 0: string, 1?: 'OPTIONS'|'GET' }. A tuple with the first item being the REST API path, and the second item being the method.

This PR modifies the tuple to add a third item for the allowed_statuses:

array{ 0: string, 1?: 'OPTIONS'|'GET', 2?: int<100, 599>|int<100, 599>[] }

This allows us now to do the following:

add_filter(
        'options-connectors-wp-admin_preload_paths',
        static function ( array $preload_paths ): array {
                $preload_paths[] = array( '/wp/v2/plugins/ai/ai?context=edit', 'GET', array( 200, 404 ) );
                return $preload_paths;
        }
);

This allows both 200 OK responses and 404 Not Found responses to be served to the preload middleware. If this 3rd item of the tuple is omitted, then it retains the previous behavior of only allowing 200 OK.

To further support this enhancement, full PHPStan typing was added to the rest_preload_api_request() function, and any PHPStan rule level 10 errors were addressed:

  2937   Function rest_preload_api_request() has parameter $memo with no value type specified in iterable type array.
         ๐Ÿชช  missingType.iterableValue
         ๐Ÿ’ก  See: https://phpstan.org/blog/solving-phpstan-no-value-type-specified-in-iterable-type
         at src/wp-includes/rest-api.php:2937
  2937   Function rest_preload_api_request() return type has no value type specified in iterable type array.
         ๐Ÿชช  missingType.iterableValue
         ๐Ÿ’ก  See: https://phpstan.org/blog/solving-phpstan-no-value-type-specified-in-iterable-type
         at src/wp-includes/rest-api.php:2937
  2961   Parameter #1 $value of function untrailingslashit expects string, string|false given.
         ๐Ÿชช  argument.type
         at src/wp-includes/rest-api.php:2961
  2979   Offset 'path' might not exist on array{scheme?: string, host?: string, port?: int<0, 65535>, user?: string, pass?: string, path?: string, query?: string, fragment?:
         string}.
         ๐Ÿชช  offsetAccess.notFound
         at src/wp-includes/rest-api.php:2979
  2979   Parameter #1 $method of class WP_REST_Request constructor expects string, string|false given.
         ๐Ÿชช  argument.type
         at src/wp-includes/rest-api.php:2979
  2990   Parameter #1 $embed of function rest_parse_embed_param expects array|string, mixed given.
         ๐Ÿชช  argument.type
         at src/wp-includes/rest-api.php:2990
  2991   Parameter #1 $response of method WP_REST_Server::response_to_data() expects WP_REST_Response, mixed given.
         ๐Ÿชช  argument.type
         at src/wp-includes/rest-api.php:2991
  2994   Cannot access offset string on mixed.
         ๐Ÿชช  offsetAccess.nonOffsetAccessible
         at src/wp-includes/rest-api.php:2994
  2996   Cannot access property $headers on mixed.
         ๐Ÿชช  property.nonObject
         at src/wp-includes/rest-api.php:2996
  3001   Cannot access property $headers on mixed.
         ๐Ÿชช  property.nonObject
         at src/wp-includes/rest-api.php:3001

## Use of AI Tools

AI assistance: Yes
Tool(s): Clause Code
Model(s): Claude Opus 4.7
Used for: Research for PHPStan types.

#2 @westonruter
5 months ago

  • Focuses rest-api added

โ€‹@westonruter commented on โ€‹PR #11790:


5 months ago
#3

@pento As you've been around lately and you introduced rest_preload_api_request() in r44123 and r44172, maybe you'd have review input as well.

#4 @westonruter
5 months ago

cc @chubes4 as this relates to #65206

#5 @westonruter
5 months ago

  • Description modified (diff)

#6 @JeffPaul
5 months ago

  • Keywords connectors added

This ticket was mentioned in โ€‹Slack in #core-test by r1k0. โ€‹View the logs.


5 months ago

#8 @jorgefilipecosta
5 months ago

Thanks so much for working on this enhancement, @westonruter. Given that it's a performance enhancement involving some REST API mechanics changes, and we're close to the final release, I think it may be best to ship this in the next minor.

This ticket was mentioned in โ€‹Slack in #core by audrasjb. โ€‹View the logs.


5 months ago

#10 @audrasjb
5 months ago

  • Milestone 7.0 โ†’ 7.0.1

As per today's bug scrub: Moving to 7.0.1 as we're too late in the release cycle.

#11 @audrasjb
5 months ago

Removing trunk version as this is not going to be shipped with WP 7.0 but in the next releases.

This ticket was mentioned in โ€‹Slack in #core-performance by westonruter. โ€‹View the logs.


4 months ago

#13 @masteradhoc
3 months ago

  • Milestone 7.0.1 โ†’ 7.0.2

As the RC for WP 7.0.1 is expected to land tomorrow we'll have to punt this to 7.0.2.

#14 @jorgefilipecosta
3 months ago

  • Milestone 7.0.2 โ†’ 7.1

โ€‹@jorgefilipecosta commented on โ€‹PR #11790:


3 months ago
#15

Hi @westonruter this would be a very good one to get on 7.1 is it something you can push forward?

It seems to be in a very good shape. The only potential issue I noticed is the 404 preloading. On a live request the an apiFetch promise to a 404 is rejected and with these changes it is not. Which can have unexpected consequences on code. prepareResponse unded โ€‹https://github.com/WordPress/gutenberg/blob/2d6811f860c96953e77e7fc638994dea1179f2bb/packages/api-fetch/src/middlewares/preloading.ts#L112-L149 always returns 200 success.

I see two ways forward we remove the 404 preloading we don't have total preloading but we still have a very good improvement. Or we fix prepareResponse and we get the full preloading in.
Let me know if the is anything I can help with.

โ€‹@extrachill commented on โ€‹PR #11790:


3 months ago
#16

I tested the current Core PR head together with the Gutenberg preload-filter change and can confirm @jorgefilipecostaโ€™s concern deterministically.

Using the same missing-plugin endpoint:

  1. On the baseline, apiFetch() rejects with rest_plugin_not_found and status 404.
  2. With the candidate preload data, the same apiFetch() call resolves with the REST error object as ordinary data.
  3. With parse: false, the reconstructed preloaded response has status 200, ok: true, and status text OK.

The candidate does eliminate the network request, but it changes the observable API contract from rejection to resolution. I think this blocks preloading the 404 responses as currently implemented.

The safe options appear to be:

  • preload only successful responses for now; or
  • preserve the original status through the preload payload and createPreloadingMiddleware()/prepareResponse(), with a browser-level regression test asserting that a preloaded 404 behaves identically to a live 404.

This ticket was mentioned in โ€‹Slack in #core-performance by westonruter. โ€‹View the logs.


2 months ago

#18 @wildworks
2 months ago

  • Milestone 7.1 โ†’ 7.2

The 7.1 RC1 release is imminent, but it appears there are still points to discuss regarding the approach based on the PR. Therefore, I'd like to punt this to 7.2.

โ€‹@ugyensupport commented on โ€‹PR #11790:


7 weeks ago
#19

## Test Report

Ticket: Trac #65215

### Environment

  • WordPress: 7.1 (dev)
  • PHP: 8.5.7 (PHP built-in server via wp server)
  • Database: SQLite
  • Browser: Chromium (Playwright)
  • OS: macOS
  • Theme: Twenty Twenty-Five 1.5
  • Active plugins: All-in-One WP Migration, File Manager Advanced, Gutenberg 23.8.0, WordPress Beta Tester, WP File Manager

### What the PR does
Extends rest_preload_api_request()'s path-tuple format from [path, method] to [path, method, allowed_statuses] so preloading can capture non-200 responses (e.g. 404s). Adds a new options-connectors-wp-admin_preload_paths filter in wp-admin/options-connectors.php, with a callback (_wp_connectors_preload_paths()) that queues /wp/v2/settings, OPTIONS /wp/v2/plugins, the AI plugin's record, and each connector's plugin record (allowing 200/404) for preloading on the Connectors screen.

### Steps taken

  1. Applied the patch to src/wp-admin/options-connectors.php, src/wp-includes/rest-api.php, and src/wp-includes/class-wp-http-response.php (the 4th changed file, the PHPUnit test, doesn't apply โ€” no test-suite checkout on this install). php -l clean on all three.
  2. Logged in as a disposable admin and opened Settings โ†’ Connectors (wp-admin/options-connectors.php).
  3. Captured the inline wp.apiFetch.createPreloadingMiddleware(...) payload and the wp-json/* network requests fired by the page.
  4. Reverted just options-connectors.php to the unpatched original, reloaded the Connectors page, and re-captured the same data for comparison.
  5. Diffed the two wp-json/* network traces (patched vs. unpatched).
  6. Directly exercised the new rest_preload_api_request() tuple/status logic via wp eval, reproducing the PR's own PHPUnit fixture (existing post with [path, 'GET', [200]], a missing post with [path, 'GET', [200, 404]], and a missing post with the old 2-element [path, 'GET'] form).
  7. Restored all three files from backup and verified SHA-1 matches the pre-test originals; deleted the disposable user; restored siteurl/home.

Result: PARTIAL โ€” core logic works, but the PR has no observable effect as shipped.

### Actual result

  • โœ… The core logic fix (rest_preload_api_request() accepting a 3rd "allowed statuses" element) works exactly as intended: with ['/wp/v2/posts/<id>', 'GET', [200, 404]], a 404 response for a missing post *is* captured into the preload payload (rest_post_invalid_id, data.status => 404); the old 2-element form still defaults to "200 only" and correctly excludes the 404.
  • โŒ The new options-connectors-wp-admin_preload_paths filter is never applied anywhere, so _wp_connectors_preload_paths() is dead code as shipped. The Connectors screen builds its preload list in wp-includes/build/pages/options-connectors/page-wp-admin.php (wp_options_connectors_wp_admin_preload_data()), which hardcodes its own 2-item $preload_paths array and never calls apply_filters( 'options-connectors-wp-admin_preload_paths', ... ) โ€” confirmed against trunk directly, not just this install's build. That build file isn't touched by this PR.
  • Net effect: applying the patch produces zero observable change on the Connectors screen. The inline preload payload is identical either way, and the browser still fires the same 6 live wp-json requests on mount in both patched and unpatched states โ€” including 4 that log as browser console errors (Failed to load resource: 404) for /wp/v2/plugins/ai/ai and /wp/v2/plugins/ai-provider-for-{anthropic,google,openai}/plugin โ€” precisely the round-trips this PR is meant to eliminate.
  • A Playwright network-trace diff (patched vs. unpatched Connectors page load, wp-json/* requests only) confirmed the request lists are byte-identical apart from incidental static-asset index numbers.

This matches the PR description's own note that it's posted "for code review only" pending discussion of 404-preloading semantics โ€” but concretely, even setting that aside, the PR as written doesn't yet wire the new filter into the page it's meant to speed up, so it ships no user-visible improvement on its own.

### Additional notes

  • WP_HTTP_Response::$headers docblock change (@var array โ†’ @var array<string, string>) is type-annotation only; no behavior to test.
  • The $response instanceof WP_REST_Response guard and the _embed param type-narrowing are defensive/PHPStan-driven; found no case where they change behavior on a normal request โ€” no regression observed.
  • Core files backed up before patching and restored afterward; SHA-1 of all three restored files matched the pre-test backup exactly.

### Support content โ€” core logic verified directly (wp eval)

$existing = wp_insert_post( array( 'post_title' => 'Preload Test', 'post_status' => 'publish' ) );
$missing  = 999999;
$preload_paths = array(
    '/wp/v2/types',
    array( "/wp/v2/posts/$existing", 'GET', array( 200 ) ),
    array( "/wp/v2/posts/$missing", 'GET', array( 200, 404 ) ),
    array( "/wp/v2/posts/$missing", 'GET' ),          // old 2-element form, defaults to [200]
    array( '/wp/v2/media', 'OPTIONS' ),
);
$data = array_reduce( $preload_paths, 'rest_preload_api_request', array() );

Result:

Keys: /wp/v2/types, /wp/v2/posts/<existing>, /wp/v2/posts/999999, OPTIONS
existing status body.id: <existing>
missing (200,404 allowed) present: YES code=rest_post_invalid_id

โ†’ the 404-allowed tuple *was* preloaded; the old 2-element form (default 200-only) correctly was *not* โ€” matches expected backward-compatible behavior.

### Support content โ€” filter never consumed (confirmed on trunk)

curl -s https://raw.githubusercontent.com/WordPress/wordpress-develop/trunk/src/wp-includes/build/pages/options-connectors/page-wp-admin.php \
  | grep -n "apply_filters\|preload_paths"

โ†’ only shows the hardcoded $preload_paths array and the unrelated options-connectors-wp-admin_boot_dependencies filter; no options-connectors-wp-admin_preload_paths anywhere.

---
๐Ÿค– Test performed with โ€‹Claude Code

โ€‹@ugyensupport commented on โ€‹PR #11790:


7 weeks ago
#20

## Follow-up: two fixes needed before this is safe to wire up

Beyond the โ€‹previous test report, dug a bit further into what would actually be needed to make this land cleanly.

### 1. The filter-wiring gap โ€” blocking, one-line fix

wp_options_connectors_wp_admin_preload_data() in wp-includes/build/pages/options-connectors/page-wp-admin.php never calls apply_filters( 'options-connectors-wp-admin_preload_paths', $preload_paths ). It hardcodes its own 2-item $preload_paths array. Without adding that apply_filters() call before the array_reduce(), none of this PR's new preload paths (_wp_connectors_preload_paths()) are ever consumed โ€” confirmed on trunk, not just a local build.

### 2. A real correctness bug this gap is currently masking โ€” createPreloadingMiddleware always fakes HTTP 200

Looked at the client side that actually consumes preloaded data: createPreloadingMiddleware in packages/api-fetch/src/middlewares/preloading.ts (Gutenberg trunk). Its prepareResponse() helper:

function prepareResponse( responseData, parse ) {
        if ( parse ) {
                return Promise.resolve( responseData.body );
        }
        // ...
        return Promise.resolve(
                new window.Response( JSON.stringify( responseData.body ), {
                        status: 200,
                        statusText: 'OK',
                        headers: responseData.headers,
                } )
        );
}
  • With parse: true (the default for essentially all core-data calls) it resolves successfully with responseData.body โ€” no status info survives at all.
  • With parse: false it hardcodes status: 200 on the reconstructed Response, regardless of what status was actually recorded.

Compare that to a live (non-preloaded) request. api-fetch's default handler (packages/api-fetch/src/index.ts) does:

if ( ! response.ok ) {
        return parseAndThrowError( response, parse );
}
return parseResponseAndNormalizeError( response, parse );

i.e. a real 404 rejects the promise, and callers (core-data resolvers, .catch() handlers, etc.) branch on that rejection.

So even after fixing #1, a preloaded 404 โ€” e.g. the "is the AI plugin installed" check this PR adds โ€” would resolve successfully instead of rejecting, the opposite of what the equivalent live request does today. Concretely: AiPluginCallout's getEntityRecord( 'root', 'plugin', 'ai' ) lookup, which currently correctly treats a missing AI plugin as "not found" via a rejected fetch, would instead receive a fake-success response carrying the WP_Error body ({code: 'rest_...', data: { status: 404 }}) as if it were valid plugin data. That's plausibly a broken "AI plugin found" state shown to users instead of the intended graceful fallback โ€” not just a hypothetical, since the shape of a WP REST error body doesn't get special-cased anywhere in the resolved-success path.

This is exactly what's flagged in the PR description itself ("further discussion regarding 404 preloading behavior and its interaction with JavaScript fetch promises") โ€” worth confirming it's a concrete, verifiable issue and not just caution.

### Suggested order of fixes

  1. Fix createPreloadingMiddleware (Gutenberg-side) to propagate the recorded status โ€” reject / normalize-and-throw for non-2xx preloaded entries the same way the live default handler does.
  2. Then wire the apply_filters( 'options-connectors-wp-admin_preload_paths', ... ) call into wp_options_connectors_wp_admin_preload_data().
  3. Add a test (PHP and/or JS) asserting a preloaded 404 rejects the same way a live 404 does โ€” the coverage gap that would have caught this.

---
๐Ÿค– Generated with โ€‹Claude Code

โ€‹@wildworks commented on โ€‹PR #11790:


10 days ago
#21

Fixed a PHPStan CI error in 6236869.

โ€‹@wildworks commented on โ€‹PR #11790:


10 days ago
#22

Pushed an additional 71701fd86bfdaec282f2f4883039f5de30ec4ce2 based on โ€‹https://github.com/WordPress/gutenberg/pull/77353#discussion_r4131889985

โ€‹@wildworks commented on โ€‹PR #11790:


7 days ago
#23

030f827001ce6e65d3a9b8263a0b1b45e1f1f3aa / 2d55126acc: Resolved merge conflicts with trunk, and applied the feedback from the Gutenberg side to this branch as well.

#24 @logicrays
38 hours ago

Hello @westonruter - the LCP numbers make a strong case for preloading these Connectors REST requests.

Looking at โ€‹https://github.com/WordPress/wordpress-develop/pull/11790 together with โ€‹https://github.com/WordPress/gutenberg/pull/77353, I think there are still two blockers before this is safe to ship:

  1. Filter wiring _wp_connectors_preload_paths() hooks wp_options_connectors_wp_admin_preload_paths, but trunk's generated page-wp-admin.php still hardcodes $preload_paths and does not call apply_filters(). Until Gutenberg #77353 (or an equivalent Core build update) lands, the Connectors-specific paths never get consumed, so this Core PR alone has no user-visible effect on the screen.
  1. 404 preloading vs apiFetch contract Agree with @jorgefilipecosta / @chubes4 / @dugyen: preloading non-OK responses is currently unsafe because createPreloadingMiddleware/prepareResponse() treat preloaded data as success (and hardcode HTTP 200 when parse: false). A live missing-plugin request rejects; a preloaded 404 resolves. That can break โ€œplugin not installedโ€ handling (e.g. the AI plugin callout).

Also noticing a strategy mismatch:

  • Core #11790 preloads plugin records with [path, 'GET', [200, 404]]
  • Gutenberg #77353 currently registers those paths without allowing 404

I'd suggest either:

  • A (safer / ship sooner): preload only known-OK paths for now (/wp/v2/settings, OPTIONS /wp/v2/plugins, and plugin records only when present), and leave missing-plugin 404s on the network; or
  • B (full fix): first fix Gutenberg preloading middleware to preserve status and reject non-2xx the same as a live request, with a JS regression test, then land 404-allowed preloads.

Happy to help re-test Connectors network waterfall + AI callout once those pieces are aligned.

Note: See TracTickets for help on using tickets.