wp-build: strip unminified bundles from production builds#48729
wp-build: strip unminified bundles from production builds#48729dhasilva wants to merge 2 commits into
Conversation
|
Are you an Automattician? Please test your changes on all WordPress.com environments to help mitigate accidental explosions.
Interested in more tips and information?
|
|
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. Videopress 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. |
cdc0fe3 to
3b0b77c
Compare
3b0b77c to
68c650f
Compare
Code Coverage SummaryThis PR did not change code coverage! That could be good or bad, depending on the situation. Everything covered before, and still is? Great! Nothing was covered before? Not so great. 🤷 |
|
Took this for a careful read — really like the shape of it. The defensive One thing worth a second look before merge, and a couple of smaller nits:
Either works — (2) is a smaller diff if you want to defer the styles handling to a follow-up. Tests would buy a lot of confidence here. Tiny nits, take or leave:
One question: Nothing here is a blocker — the styles.php thing is the only one I'd genuinely want addressed before merge, and even that has a one-line fix path (widen the guard). Really nice work. |
Each wp-build consumer now runs a new strip-unminified-prod step after
wp-build, removing the *.js / *.css siblings of *.min.js / *.min.css
under build/{routes,scripts,modules}/** and collapsing the SCRIPT_DEBUG
ternary in the generated routes.php / scripts.php / modules.php loaders
to '.min.js'. Cuts ~10 MiB of dead bytes from jetpack-production while
keeping local pnpm watch / non-production builds untouched.
Wired into: newsletter, podcast, forms, backup, scan, videopress.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
68c650f to
bbd5136
Compare
|
On premium-analytics: that one was a deliberate decision because it does not use the polyfills package and is a small package itself. It would require adding the polyfills package as a dependency just for this. About changes made to address the comment, I'll let Claudio do the talk: Generated by Claude: Thanks for the careful read — addressed the styles.php gap and the smaller nits in
|
…od tests Replaces the synthesised PHP fixtures with a tiny wp-build-compatible package that actually runs through @wordpress/build at test time. The before hook invokes wp-build, the test asserts deletions / PHP rewrites against the real generated output, and the after hook cleans up. If a future wp-build release shifts the SCRIPT_DEBUG ternary shape or renames a loader, the polyfills test suite now fails one PR earlier than any consumer's build-production would. The fixture covers all four strip targets — one route (paired .js/.min.js under build/routes/) and one wpScript sub-package with a build-style/style.css (paired .js/.min.js under build/scripts/ plus paired .css/.min.css under build/styles/). @wordpress/build and its browserslist/babel runtime deps move into polyfills' devDependencies. tests/** is already production-exclude in .gitattributes, and the new wp-build output paths are gitignored, so nothing about this commit ships to any production mirror. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
ddec2c3 to
d7230a8
Compare
Fixes # N/A
Proposed changes
strip-unminified-prodin@automattic/jetpack-wp-build-polyfills. It deletes paired.js/.csssiblings of.min.js/.min.css(plus any paired.js.map) underbuild/{routes,scripts,modules,styles}/**, and rewrites the generatedroutes.php/scripts.php/modules.php/styles.phploaders so theSCRIPT_DEBUGternary collapses to the minified asset. The PHP rewrite keeps the asset reachable even when a deploy target setsSCRIPT_DEBUG=true, so loaders never 404.build-productionpipeline of every wp-build consumer: newsletter, podcast, forms, backup, scan, videopress. The non-productionbuildscript andpnpm watchare unchanged — local developers still have both bundles available for SCRIPT_DEBUG debugging.composer.jsonbuild-productionnormalised to delegate topnpm run build-production, matching the other five packages.@wordpress/buildever changes the shape of the SCRIPT_DEBUG ternary (in either the$extensionor the$suffixform), the script throws instead of silently shipping a loader that 404s under SCRIPT_DEBUG. Logs a one-line skip notice ifbuild/is missing entirely.--testcoverage for the bin (8 tests): fixture-based deletion and PHP rewrite assertions for all four loader shapes, idempotency, and the safety-net throw for both unrecognised ternary forms.Caveat about SCRIPT_DEBUG-on-production debugging: with this PR, a deploy target running with
SCRIPT_DEBUG=truegets the minified bundle instead of the unminified one. Devtools sourcemaps still resolve names, but in-page stack traces andview-sourceare no longer readable. This is the small tradeoff in exchange for cutting ~10 MiB of dead bytes from every production mirror.Measured savings (each branch built end-to-end via
composer run-script build-productionper package;du -sbonbuild/):Premium-analytics was intentionally left out of this PR.
Related product discussion/links
This is the "Recommendation A" follow-up to the internal production-size investigation. Companion PR #48627 already addresses the cross-plugin
jetpack-newsletterduplication (Recommendation B in the same report).Does this pull request change what data or activity we track or use?
No. Build-only change; no runtime behaviour changes for end users.
Testing instructions
The bug this PR fixes is only visible at production-build time, so the tests are build-side. The end-to-end fix is verified by the size-delta table above; you can repeat it locally for any wp-build package:
build/routes/*/content.jsandbuild/routes/*/route.jsare present.pnpm install(the new bin is a workspace symlink), and re-run the same commands. Confirm:build/is noticeably smaller (see table above for the expected delta)..js/.csssiblings underbuild/{routes,scripts,modules,styles}/**are gone; their.min.js/.min.csscounterparts remain.build/routes.php,build/scripts.php,build/modules.phpnow use$extension = '.min.js';instead of theSCRIPT_DEBUG ? '.js' : '.min.js'ternary.build/styles.phpnow uses$suffix = '.min';instead ofSCRIPT_DEBUG ? '' : '.min'.composer run-script build-productiona second time withoutcleanis a silent no-op (strip-unminified-prodis idempotent).SCRIPT_DEBUGbothfalseandtrue. Both should load and render correctly — underSCRIPT_DEBUG=truethe loader now falls back to the minified bundle (no 404s).pnpm --filter @automattic/jetpack-newsletter run watchshould still produce bothcontent.jsandcontent.min.jsand not invoke the strip step. The strip step only runs inbuild-production.