Premium Analytics: move the Ads widgets to the jetpack-ads package - #52635
Conversation
retrofox
added
Enhancement
|
Are you an Automattician? Please test your changes on all WordPress.com environments to help mitigate accidental explosions.
Interested in more tips and information?
|
github-actions
Bot
added
[Feature] Ad
[mu wpcom Feature] Premium Analytics
[Package] Ad
|
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! Jetpack plugin: The Jetpack plugin has different release cadences depending on the platform:
If you have any questions about the release process, please ask in the #jetpack-releases channel on Slack. Mu Wpcom plugin:
If you have any questions about the release process, please ask in the #jetpack-releases channel on Slack. Wpcomsh plugin:
If you have any questions about the release process, please ask in the #jetpack-releases channel on Slack. Premium Analytics plugin: No scheduled milestone found for this plugin. If you have any questions about the release process, please ask in the #jetpack-releases channel on Slack. |
Code Coverage SummaryCoverage changed in 6 files. Only the first 5 are listed here.
4 files are newly checked for coverage.
Full summary · PHP report · JS report Coverage check overridden by
Coverage tests to be added later
|
8f77bd5 to
cc5be17
Compare
github-actions
Bot
added
[Package] Wp Build Polyfills
[Plugin] mu wpcom
cc5be17 to
a4999b7
Compare
c153cdb to
ff48b34
Compare
the sections REST route runs before widget-types.php defines WIDGET_API_VERSION
louwie17
left a comment
louwie17
left a comment
There was a problem hiding this comment.
Thanks for the updates @retrofox, the SDK approach is a lot cleaner, and it's nice to see the widgets only import @automattic/jetpack-premium-analytics-sdk now. This looks good locally and I will have to re-test it on Simple, although this may have to be merged first to do a full testing.
I did notice all the SDK types are any at the moment, meaning the widgets lost their type checking in the move, and together with the Jest tests being a follow up nothing checks them right now. I presume this will be a follow up?
I left a few small inline comments, happy for them to be follow ups. One thing to make sure of before merging is the Automattic/jetpack-wordads-analytics mirror repo and Packagist entry, as far as I can tell it doesn't exist yet and the Jetpack plugin requires the package.
Overall this all looks good, and the work is also still behind a feature flag, so this should be good to merge from my perspective 🚢
| export declare const PRESET_LAST_12_MONTHS: string; | ||
|
|
||
| // Data. | ||
| export declare function useStatsWordAdsStats( ...args: any[] ): any; |
There was a problem hiding this comment.
Should the WordAds specific hooks and components (useStatsWordAdsStats, useStatsWordAdsEarnings, EarningsHistoryList, flattenEarningsBreakdown) live in the SDK long term? Given this is meant to become the public API, removing them later would be a breaking change. I presume these will be moving to the WordAds package as well?
There was a problem hiding this comment.
Agreed. They're in because the widgets need them and the SDK doesn't expose the generic report hooks they're built on yet; the two earnings ones are shared with the Earnings report still in PA.
They leave for the Ads package with that report, at no compat cost since the SDK is private until the audit. Marked provisional in the .d.ts (c5df75e).
| * | ||
| * @return bool | ||
| */ | ||
| private static function widget_contract_moved_on() { |
There was a problem hiding this comment.
Could we add a test for the case where WIDGET_API_VERSION isn't defined yet, like on the sections REST route? That's the case 8981f4b fixed, and the current tests always load widget-types.php, so it would be easy for it to come back. Can also be a follow up.
There was a problem hiding this comment.
Done in cabc916: Analytics_Dashboard_Without_Widget_Types_Test, one process per test so the constant stays undefined. It covers both branches: the section registers, the widget types wait.
| * @param bool $minified Whether to serve the minified bundle. | ||
| * @return array|null `src`, `deps` and `version` for `wp_register_script_module()`, or null when the registry has no facade. | ||
| */ | ||
| function get_sdk_module_registration( array $modules, array $constants, $build_dir, $minified = true ) { |
There was a problem hiding this comment.
A small nit, this is only used by the registration below and its tests, should we mark it @internal so it doesn't become part of the package's public surface?
the name scales to the whole WordAds module; mirror Automattic/jetpack-ads
retrofox
changed the title
one process per test keeps WIDGET_API_VERSION undefined, as on the sections route
|
Thanks Lourens. The round is in cabc916..c5df75e, replies inline. Types: yes, a follow-up. The real types live in PA's internal packages, so the plan is a PA build step that emits a bundled Mirror: the package is The push dismissed your approval. Could you re-approve once you've looked at the round? |
PHPUnit feeds the child process from stdin; PHP 7.4 warns on the empty needle
retrofox
added
the
Coverage tests to be added later
louwie17
left a comment
louwie17
left a comment
There was a problem hiding this comment.
Thanks @retrofox, the updates look good, and thanks for adding the test for the sections route and marking the provisional exports. This still looks good locally with the rename to jetpack-ads. Good to merge from my perspective 🚢
…ity-wordads-package # Conflicts: # projects/packages/ads/widgets/wordads-chart-tabs/render.tsx # projects/packages/premium-analytics/widgets/wordads-chart-tabs/stories/wordads-chart-tabs-widget.stories.tsx
github-actions
Bot
added
the
[Status] UI Changes
…ds is on Rebased onto the registrant #52635 moved into jetpack-mu-wpcom: the plan feature alone still decided there, so every Premium-and-up Simple or Atomic site saw an Ads tab whether or not WordAds was ever turned on. The gate now also requires WordAds to be on, read as classic Stats reads it: the approval stickers on Simple, the WordAds module on Atomic. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
…ption-2 Re-apply the Adjustments link change on the widget's new home in packages/ads after #52635; its test and stories were dropped in that move. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
The Earnings History widget lives in packages/ads since #52635, which the standalone plugin does not bundle. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
…ds is on (#52903) * Premium Analytics: show the Ads tab on WordPress.com only while WordAds is on Rebased onto the registrant #52635 moved into jetpack-mu-wpcom: the plan feature alone still decided there, so every Premium-and-up Simple or Atomic site saw an Ads tab whether or not WordAds was ever turned on. The gate now also requires WordAds to be on, read as classic Stats reads it: the approval stickers on Simple, the WordAds module on Atomic. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> * Say the WordPress.com gate is plan plus WordAds on, in docs and comments The docs, the sections diagram, the Ads package's README and docblock and the module registrant's comments still described the plan-only gate. The Atomic comment also claimed to match Odyssey, which reads the approval stickers through the site endpoint; the rule is that a site with the module off is not running ads. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> --------- Co-authored-by: Claude Fable 5.1 <noreply@anthropic.com>
…2752) * Premium Analytics: link to adjustments as text with a count badge An alternative to the badge that is itself the link: the footer reads "Adjustments" with the row count in a neutral badge beside it, so the link has its own text and the badge stays a label. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> * Drop the premium-analytics plugin changelog entry The Earnings History widget lives in packages/ads since #52635, which the standalone plugin does not bundle. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> * Keep the ReportLink hover underline off the badge The underline sat on the link, so it ran through the count badge too. The label now carries it, and children render after the label. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> --------- Co-authored-by: Claude Fable 5.1 <noreply@anthropic.com>
4 participants
Fixes WOOA7S-2188
Proposed changes
Step 3 of 4 of the widget-type extensibility stack (umbrella: #52637).
The Ads section and its three widgets leave the Premium Analytics package and move to their own package,
jetpack-ads.It is the first real consumer of the widget registration added in #52568 and #52634, and it is rebased on trunk now that both are in.
What changes
A new package,
projects/packages/ads, owns the Ads section and its three widgets: chart tabs, highlights, and earnings history.They register as
wordads/chart-tabs,wordads/highlightsandwordads/earnings-history, so the namespace names the owner.The package registers everything through the dashboard's public API: the section with its title and default layout, and the widget types from the manifest it generates.
The WordAds module and
jetpack-mu-wpcomno longer declare the section themselves and only call the package.The module does it outside WordPress.com, and mu-wpcom does it on Simple and Atomic when the plan includes WordAds. The Premium Analytics package loses the three widgets and the Ads layout helper.
The changes the trunk made to those widgets in the meantime (#52683, #52690, #52714, #52794) moved with them.
A new js-package,
projects/js-packages/premium-analytics-sdk, holds the SDK the widgets import, and the dashboard provides it at runtime. More on this below.Why a package
WordAds comes with Premium and higher on WordPress.com, where most sites are Simple and run no Jetpack module.
The Jetpack module reaches self-hosted and Atomic sites, and
jetpack-mu-wpcomregisters on Simple and Atomic from the copy the Jetpack plugin bundles, as #52717 does for Premium Analytics; the package is test-only there.AFAIK, a Composer package is the only unit both callers can share.
How the package uses the dashboard's modules
The widgets import the dashboard's SDK by name, like any package:
That name is three things at once: an npm package, an import specifier and a script module id. Each side of the boundary handles one step.
1. The SDK package is the contract.
projects/js-packages/premium-analytics-sdkis a private workspace package. It holds the types of what the dashboard provides (src/index.d.ts) and awpScriptModuleExportsentry, which marks it as a script module. It has no build: the implementation is the dashboard's. Itssrc/index.jsonly throws, so a build that bundles it by mistake fails loudly instead of shipping a second copy.2. The Ads package builds with wp-build, unchanged. It depends on the SDK with
workspace:*and lists theautomatticscope as an external namespace:wp-build's externals plugin resolves
@automattic/jetpack-premium-analytics-sdkfromnode_modules, seeswpScriptModuleExports, and leaves the import external, recorded as a module dependency of the widget. It is the same rule it applies to@wordpress/*; nothing in wp-build changed. Each widget's asset ends up like this:3. The dashboard provides the implementation.
packages/sdkis a facade module wp-build builds as@jetpack-premium-analytics/sdk: re-exports of the pieces widgets need from the dashboard's own modules (widgets toolkit, data, fields, dates, shared primitives).src/sdk-module.phpregisters that same file a second time, under the SDK's name. As with Connection (#38877), the package that owns the code provides the bundle.4. The browser joins the two. The page import map maps the SDK name to the facade, and the facade's dependencies to the dashboard's modules, so a widget runs on the same module instances the dashboard renders with: one React, one toolkit, one query client.
Nothing in the Ads package points at another project: no
../path in itspackage.jsonortsconfig.json, and the dashboard's internal packages stay out of the workspace (reverted in 05ad025). The runtime shape is the one Connection has had since #38877, and the one Core uses for@wordpress/interactivity: import by name, external in the build, bundle served by the package that owns the code. The SDK's npm package holds only the types for now because its implementation is the dashboard's own modules, re-exported by the facade; moving those internals into js-packages, so the SDK can carry real source, is a separate decision.How it loads in the browser
A widget's code loads only when the dashboard renders it, and a shared module already on the page isn't downloaded again.
The three Ads bundles weigh between 3.5 and 4.9 KB minified, because the toolkit and the data layer stay out of them.
Mirror repository
The mirror repository
Automattic/jetpack-adsand its Packagist entry are in place. The name is the product-agnostic one, so the package can grow into the whole WordAds module without another rename.Follow-ups
The three widget tests and their stories depended on the Premium Analytics test harness, so they leave with this move. The Ads package needs its own Jest and Storybook setup to bring them back, with the empty-CPM case from #52690; the issue follows once we settle how widget test setups are shared. The data-layer tests stay in Premium Analytics.
A layout saved with the old
jpa/wordads-*names shows its tiles as unavailable until you reset it.Ads is outside the customer preview, so only internal sites have such layouts. WOOA7S-2200 tracks a rename map for widget types.
The SDK contract is loosely typed for now. It gets precise types, and an audit of what it exposes, before the package is published to npm.
Related product discussion/links
Umbrella of this stack: #52637. Steps 1 and 2: #52568 and #52634. Linear: WOOA7S-2183 for the stack and WOOA7S-2188 for this step.
Does this pull request change what data or activity we track or use?
No.
Testing instructions
Set up
Ads is outside the customer preview, so open it with this line in a mu-plugin before testing:
Use a connected site with the WordAds module active, as an administrator.
Automated tests
1. The widget types are registered
Request
GET /wp-json/wpcom/v2/widget-modules. It lists the threewordads/*types, with their render modules underjetpack-ads/widgets/and the text domainjetpack-ads-pkg.2. The Ads section renders
Open the dashboard and go to Ads. The section, titled "Ads performance", shows the three widgets.
3. The shared modules load once
In the network tab, the render modules come from
jetpack_vendor/automattic/jetpack-ads/build/widgets/, the SDK resolves topremium-analytics/build/modules/sdk/index.min.js, andwidgets-toolkit/index.min.jsis requested only once.The console shows no
Failed to resolve module specifiererror.4. The picker offers the widgets
Enter edit mode and open the widget picker. The three Ads widgets appear under Stats.
5. Without WordAds, the section is gone
Turn the WordAds module off. The Ads section disappears, and the three types are no longer listed.
Screen.Recording.2026-09-24.at.11.48.59.AM.mov
6. Layouts saved before this change
A layout saved with
jpa/wordads-*instances shows "Widget is no longer available" tiles until it is reset.7. The rest of the dashboard is unchanged
A Premium Analytics report with a CSV export button still shows it.
8. mu-wpcom carries no copy of the package
jetpack_vendor/automattic/jetpack-adsis absent from the build, as #52717 does for Premium Analytics; the mu-wpcom tests still run against it as a test-only dependency.