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 )
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
โ@westonruter commented on โPR #11790:
5 months ago
#3
This ticket was mentioned in โSlack in #core-test by r1k0. โView the logs.
5 months ago
#8
@
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
@
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
@
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
@
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.
โ@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:
- On the baseline,
apiFetch()rejects withrest_plugin_not_foundand status404. - With the candidate preload data, the same
apiFetch()call resolves with the REST error object as ordinary data. - With
parse: false, the reconstructed preloaded response has status200,ok: true, and status textOK.
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 preloaded404behaves identically to a live404.
This ticket was mentioned in โSlack in #core-performance by westonruter. โView the logs.
2 months ago
#18
@
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
- Applied the patch to
src/wp-admin/options-connectors.php,src/wp-includes/rest-api.php, andsrc/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 -lclean on all three. - Logged in as a disposable admin and opened Settings โ Connectors (
wp-admin/options-connectors.php). - Captured the inline
wp.apiFetch.createPreloadingMiddleware(...)payload and thewp-json/*network requests fired by the page. - Reverted just
options-connectors.phpto the unpatched original, reloaded the Connectors page, and re-captured the same data for comparison. - Diffed the two
wp-json/*network traces (patched vs. unpatched). - Directly exercised the new
rest_preload_api_request()tuple/status logic viawp 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). - 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_pathsfilter is never applied anywhere, so_wp_connectors_preload_paths()is dead code as shipped. The Connectors screen builds its preload list inwp-includes/build/pages/options-connectors/page-wp-admin.php(wp_options_connectors_wp_admin_preload_data()), which hardcodes its own 2-item$preload_pathsarray and never callsapply_filters( 'options-connectors-wp-admin_preload_paths', ... )โ confirmed againsttrunkdirectly, 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-jsonrequests 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/aiand/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::$headersdocblock change (@var arrayโ@var array<string, string>) is type-annotation only; no behavior to test.- The
$response instanceof WP_REST_Responseguard and the_embedparam 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 allcore-datacalls) it resolves successfully withresponseData.bodyโ no status info survives at all. - With
parse: falseit hardcodesstatus: 200on the reconstructedResponse, 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
- 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. - Then wire the
apply_filters( 'options-connectors-wp-admin_preload_paths', ... )call intowp_options_connectors_wp_admin_preload_data(). - 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
@
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:
- Filter wiring
_wp_connectors_preload_paths()hookswp_options_connectors_wp_admin_preload_paths, but trunk's generatedpage-wp-admin.phpstill hardcodes$preload_pathsand does not callapply_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.
- 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 whenparse: 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.
![(please configure the [header_logo] section in trac.ini)](/chrome/site/your_project_logo.png)
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:
The data format for a preloaded request was previously two possibilities:
string: The REST API path to preload.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:
This allows both
200 OKresponses and404 Not Foundresponses to be served to the preload middleware. If this 3rd item of the tuple is omitted, then it retains the previous behavior of only allowing200 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.