Repository navigation
PA - VideoPress: expose the Leaderboard component through the SDK - #52885
Merged
Merged
Conversation
Contributor
|
Are you an Automattician? Please test your changes on all WordPress.com environments to help mitigate accidental explosions.
Interested in more tips and information?
|
Contributor
|
Thank you for your PR! When contributing to Jetpack, we have a few suggestions that can help us test and review your patch:
This comment will be updated as you work on your PR and make changes. If you think that some of those checks are not needed for your PR, please explain why you think so. Thanks for cooperation 🤖 Follow this PR Review Process:
If you have questions about anything, reach out in #jetpack-developers for guidance! |
Code Coverage SummaryCoverage changed in 1 file.
|
retrofox
added this pull request to stack #52919
September 29, 2026 11:06
Base automatically changed from
update/pa-extensibility-videopress-former-names
to
trunk
September 30, 2026 06:35
retrofox
force-pushed
the
update/pa-extensibility-videopress-sdk-exports
branch
from
September 30, 2026 06:37
8c74142 to
700bbec
Compare
the video plays hook too, provisional; widget contract 1.2.0; the facade must be fully declared
retrofox
force-pushed
the
update/pa-extensibility-videopress-sdk-exports
branch
from
September 30, 2026 06:48
700bbec to
c76b93e
Compare
…ity-videopress-sdk-exports
with describeError and the row limit; the video plays hook stays provisional
retrofox
marked this pull request as ready for review
September 30, 2026 16:11
the Leaderboard caps its rows; a consumer sizes its own request
Nikschavan
approved these changes
Oct 1, 2026
Nikschavan
left a comment
Member
There was a problem hiding this comment.
Thank you, the changes look good.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes WOOA7S-2222
Proposed changes
Step 3 of the VideoPress consumer stack (WOOA7S-2221), after #52874 and #52965.
@automattic/jetpack-premium-analytics-sdkexports what the three Ads widgets import and nothing else.The Top videos widget cannot leave the package until the SDK carries what it renders with. After #52965 that is one component, not a construction kit.
What the SDK gains
Leaderboard, with its typesLeaderboardProps,LeaderboardRowInput,LeaderboardStatusandLeaderboardDrillDown: ranked rows and a request status in; the chart with its states, comparison and drill-down out.describeError(), the dashboard's mapping from a failed request to the widget's error state: no Retry on an access-denied failure, Retry otherwise.useStatsVideoPlays, provisional like the WordAds hooks: the Videos report reads it too and stays in the package until reports are registrable. It leaves the SDK when VideoPress owns its data (WOOA7S-2234).The
packages/sdkfacade re-exports them, andsrc/index.d.tsof the js-package declares them, loosely typed like the rest of the contract for now.getVideoKey()andgetVideoLabel()stay out: nothing but the widget uses them, so they move with it in step 4.WIDGET_ROW_LIMITstays out too.Leaderboardcaps the rows it shows, and a consumer sizes its own request.The contract
WIDGET_API_VERSIONmoves to 1.2.0: a consumer can rely on something new; nothing built against 1.x breaks. The docs now list what each minor added.The conformance check
packages/sdk/src/conformance.tsalready failed the typecheck when the facade missed a declared value.It now fails the other way too: when the facade exports a value the declaration file doesn't include, since a consumer can't import it.
Related product discussion/links
Linear: WOOA7S-2221 for the stack, WOOA7S-2222 for this step. Umbrella #52867.
Does this pull request change what data or activity we track or use?
No.
Testing instructions
On a connected site with the dashboard built,
GET /wp-json/wpcom/v2/widget-modulesstill lists the Ads widgets, and the Ads section renders as before: the facade only gained exports.In the browser console on the dashboard page...
...lists
Leaderboard,describeErroranduseStatsVideoPlaysnext to the existing names.To see the conformance check work, remove
Leaderboardfromsrc/index.d.tsof the js-package and run the package typecheck: it fails onisDeclarednaming the missing export. Restore it.