Add URL Metric mutation helpers to extension initialization API #1951
Conversation
…nalize` args Signed-off-by: Shyamsundar Gadde <[email protected]>
Signed-off-by: Shyamsundar Gadde <[email protected]>
Signed-off-by: Shyamsundar Gadde <[email protected]>
…ric has been initialized Signed-off-by: Shyamsundar Gadde <[email protected]>
Signed-off-by: Shyamsundar Gadde <[email protected]>
ShyamGadde
added
[Type] Enhancement
plugins/image-prioritizer/detect.js
Outdated
| /** | ||
| * Detected LCP external background image candidates. | ||
| * | ||
| * @type {Array<{ | ||
| * url: string, | ||
| * tag: string, | ||
| * id: string|null, | ||
| * class: string|null, | ||
| * }>} | ||
| */ | ||
| const externalBackgroundImages = []; | ||
|
|
There was a problem hiding this comment.
Do we even need this anymore? I think we can remove this and then replace this:
externalBackgroundImages.push( externalBackgroundImage );With:
log(
'Detected external background image for LCP element:',
externalBackgroundImage
);
extendRootData( { lcpElementExternalBackgroundImage: externalBackgroundImage } );Note that this handleLCPMetric could get called multiple times as new LCP candidates are expected.
There was a problem hiding this comment.
Done in 8e5b031.
| // Finalize extensions. | ||
| if ( extensions.size > 0 ) { | ||
| /** @type {Promise[]} */ | ||
| const extensionFinalizePromises = []; | ||
|
|
||
| /** @type {string[]} */ | ||
| const finalizingExtensionModuleUrls = []; | ||
|
|
||
| for ( const [ | ||
| extensionModuleUrl, | ||
| extension, | ||
| ] of extensions.entries() ) { | ||
| if ( extension.finalize instanceof Function ) { | ||
| const extensionLogger = createLogger( | ||
| isDebug, | ||
| `[Optimization Detective: ${ | ||
| extension.name || 'Unnamed Extension' | ||
| }]` | ||
| ); | ||
|
|
||
| try { | ||
| const finalizePromise = extension.finalize( { | ||
| isDebug, | ||
| ...extensionLogger, | ||
| getRootData, | ||
| getElementData, | ||
| extendElementData, | ||
| extendRootData, | ||
| } ); | ||
| if ( finalizePromise instanceof Promise ) { | ||
| extensionFinalizePromises.push( finalizePromise ); | ||
| finalizingExtensionModuleUrls.push( | ||
| extensionModuleUrl | ||
| ); | ||
| } | ||
| } catch ( err ) { | ||
| error( | ||
| `Unable to start finalizing extension '${ extensionModuleUrl }':`, | ||
| err | ||
| ); | ||
| } | ||
| } | ||
| } | ||
|
|
||
| // Wait for all extensions to finish finalizing. | ||
| const settledFinalizePromises = await Promise.allSettled( | ||
| extensionFinalizePromises | ||
| ); | ||
| for ( const [ | ||
| i, | ||
| settledFinalizePromise, | ||
| ] of settledFinalizePromises.entries() ) { | ||
| if ( settledFinalizePromise.status === 'rejected' ) { | ||
| error( | ||
| `Failed to finalize extension '${ finalizingExtensionModuleUrls[ i ] }':`, | ||
| settledFinalizePromise.reason | ||
| ); | ||
| } | ||
| } | ||
| } | ||
|
|
There was a problem hiding this comment.
We need to keep this for now, especially since existing plugins are using this, but also because we're in beta so we can't just remove something. We can deprecate it, however. For example, of extension.finalize instanceof Function is truthy, then we can emit a deprecation warning:
extensionLogger.warn( 'Use of the finalize function in extensions is deprecated. Please refactor your extension to use the initialize function instead, and update the URL Metric data as soon as a change is detected rather than waiting until finalization.' );
There was a problem hiding this comment.
Done in b2b48b3.
Signed-off-by: Shyamsundar Gadde <[email protected]>
Signed-off-by: Shyamsundar Gadde <[email protected]>
|
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. |
|
@ShyamGadde I just realized that since the these functions are newly exposed among the I've also improved typing in 2f20963. Please review and test the additional changes to ensure they're working as expected. |
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## trunk #1951 +/- ##
=======================================
Coverage 72.24% 72.24%
=======================================
Files 85 85
Lines 6953 6953
=======================================
Hits 5023 5023
Misses 1930 1930
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
|
Thanks for catching that! I've tested them and it's working correctly. |
Summary
Fixes #1930
Relevant technical choices
getRootData,extendRootData,getElementData,extendElementData) in theinitializeargs to match what was already available infinalizeargsurlMetricdefinitionfinalizephase in extensionsfinalizefunction