Boost: Use "critical css gen" package from the monorepo (#39509)

* Update Boost to use "critical css gen" package from the monorepo * add changelog * Update Boost to use new browser and node from "critical css gen" package * add changelog * Update package entry points * Cleanup package entry points * Use dynamic import for global type definitions * Add browser entry point back to package * Use dynamic import for css generator * Don't use destructuring * Update default entry point to use browser version * Update Boost to use updated package entry point * Simplify package build * Remove unnecessary dependency enqueue * Attempt to fix tests * Rename dynamically imported file * Merge dynamic import files * Add build step for tests * Cleanup build for tests * Fix tests breaking due to missing package build * Update package entry points Co-authored-by: Brad Jorsch <[email protected]> * Add helper for critical css generator import * update changelogs --------- Co-authored-by: Brad Jorsch <[email protected]>
Automattic · Oct 7, 2024 · ce32881 · ce32881
1 parent 6b41b2a
commit ce32881
Show file tree

Hide file tree

Showing 19 changed files with 164 additions and 352 deletions.
diff --git a/pnpm-lock.yaml b/pnpm-lock.yaml
diff --git a/projects/js-packages/critical-css-gen/.gitignore b/projects/js-packages/critical-css-gen/.gitignore
@@ -1,4 +1,4 @@
 /vendor
 /node_modules
-/build-node
-/build-browser
+/build
+/tests/build
diff --git a/projects/js-packages/critical-css-gen/changelog/add-playwright-endpoint b/projects/js-packages/critical-css-gen/changelog/add-playwright-endpoint
@@ -0,0 +1,4 @@
+Significance: minor
+Type: added
+
+Add /playwright entry point for BrowserInterfacePlaywright.
diff --git a/projects/js-packages/critical-css-gen/changelog/update-default-entry-point b/projects/js-packages/critical-css-gen/changelog/update-default-entry-point
@@ -0,0 +1,4 @@
+Significance: major
+Type: changed
+
+Change default entry point of package to include BrowserInterfaceIframe instead of BrowserInterfacePlaywright.
diff --git a/projects/js-packages/critical-css-gen/package.json b/projects/js-packages/critical-css-gen/package.json
@@ -15,48 +15,46 @@
 	"license": "GPL-2.0-or-later",
 	"author": "Automattic",
 	"scripts": {
-		"build:browser": "rollup -c",
-		"build:node": "tsc",
-		"build": "pnpm run clean && pnpm run build:browser && pnpm run build:node",
-		"clean": "rm -rf build-node/ && rm -rf build-browser/",
-		"test": "pnpm build && NODE_ENV=test NODE_PATH=./node_modules jest --forceExit --config=tests/config/jest.config.js"
+		"build": "pnpm run clean && tsc",
+		"build:test": "pnpm run clean:test && webpack --config tests/data/webpack.config.cjs",
+		"clean": "rm -rf build/",
+		"clean:test": "rm -rf tests/build/",
+		"test": "pnpm build && pnpm build:test && NODE_ENV=test NODE_PATH=./node_modules jest --forceExit --config=tests/config/jest.config.js"
 	},
-	"main": "./build-node/node.js",
-	"browser": "./build-browser/bundle.js",
+	"main": "./build/browser.js",
 	"devDependencies": {
+		"@automattic/jetpack-webpack-config": "workspace:*",
 		"@babel/core": "7.24.7",
 		"@babel/preset-env": "7.24.7",
 		"@babel/preset-typescript": "7.24.7",
-		"@rollup/plugin-commonjs": "26.0.1",
-		"@rollup/plugin-json": "6.1.0",
-		"@rollup/plugin-node-resolve": "15.3.0",
-		"@rollup/plugin-terser": "0.4.3",
-		"@rollup/plugin-typescript": "12.1.0",
 		"@types/clean-css": "4.2.11",
 		"@types/css-tree": "2.3.8",
 		"@types/node": "^20.4.2",
 		"express": "4.20.0",
 		"jest": "29.7.0",
+		"path-browserify": "1.0.1",
 		"playwright": "1.45.1",
 		"playwright-core": "^1.45.1",
 		"prettier": "npm:[email protected]",
-		"rollup": "3.29.5",
-		"rollup-plugin-polyfill-node": "0.13.0",
+		"process": "0.11.10",
 		"source-map": "0.7.4",
 		"source-map-js": "1.2.0",
 		"tslib": "2.5.0",
 		"typescript": "5.0.4",
 		"webpack": "5.94.0",
+		"webpack-cli": "4.9.1",
 		"webpack-dev-middleware": "5.3.4"
 	},
 	"exports": {
 		".": {
-			"jetpack:src": "./src/node.ts",
-			"types": "./build-node/node.d.ts",
-			"browser": "./build-browser/bundle.js",
-			"import": "./build-node/node.js",
-			"require": "./build-node/node.js",
-			"default": "./build-node/node.js"
+			"jetpack:src": "./src/browser.ts",
+			"types": "./build/browser.d.ts",
+			"default": "./build/browser.js"
+		},
+		"./playwright": {
+			"jetpack:src": "./src/playwright.ts",
+			"types": "./build/playwright.d.ts",
+			"default": "./build/playwright.js"
 		}
 	},
 	"dependencies": {

diff --git a/projects/js-packages/critical-css-gen/rollup.config.js b/projects/js-packages/critical-css-gen/rollup.config.js
diff --git a/.../js-packages/critical-css-gen/src/node.ts → ...ckages/critical-css-gen/src/playwright.ts b/.../js-packages/critical-css-gen/src/node.ts → ...ckages/critical-css-gen/src/playwright.ts
diff --git a/projects/js-packages/critical-css-gen/tests/data/webpack.config.cjs b/projects/js-packages/critical-css-gen/tests/data/webpack.config.cjs
@@ -0,0 +1,50 @@
+const path = require( 'path' );
+const jetpackWebpackConfig = require( '@automattic/jetpack-webpack-config/webpack' );
+const webpack = require( 'webpack' );
+
+module.exports = {
+	entry: path.join( __dirname, '../../src/browser.ts' ),
+	mode: 'development',
+	devtool: false,
+	output: {
+		...jetpackWebpackConfig.output,
+		path: path.join( __dirname, '../build' ),
+		filename: 'bundle.js',
+		library: 'CriticalCSSGenerator',
+	},
+	resolve: {
+		...jetpackWebpackConfig.resolve,
+		// These are needed for the build to work,
+		// otherwise it errors out because of the clean-css dependency.
+		fallback: {
+			...jetpackWebpackConfig.resolve.fallback,
+			path: require.resolve( 'path-browserify' ),
+			process: require.resolve( 'process/browser' ),
+			url: false,
+			https: false,
+			http: false,
+			fs: false,
+			os: false,
+		},
+	},
+	node: false,
+	plugins: [
+		new webpack.ProvidePlugin( {
+			process: require.resolve( 'process/browser' ),
+		} ),
+	],
+	module: {
+		strictExportPresence: true,
+		rules: [
+			// Transpile JavaScript
+			jetpackWebpackConfig.TranspileRule( {
+				exclude: /node_modules\//,
+			} ),
+
+			// Transpile @automattic/jetpack-* in node_modules too.
+			jetpackWebpackConfig.TranspileRule( {
+				includeNodeModules: [ '@automattic/jetpack-' ],
+			} ),
+		],
+	},
+};
diff --git a/projects/js-packages/critical-css-gen/tests/lib/test-server.js b/projects/js-packages/critical-css-gen/tests/lib/test-server.js
@@ -23,10 +23,7 @@ class TestServer {
 	async start() {
 		this.app = express();
 
-		this.app.use(
-			'/bundle.js',
-			express.static( require.resolve( '../../build-browser/bundle.full.js' ) )
-		);
+		this.app.use( '/bundle.js', express.static( require.resolve( '../build/bundle.js' ) ) );
 
 		for ( const [ virtualPath, realDirectory ] of Object.entries( this.staticPaths ) ) {
 			this.app.use( '/' + virtualPath, express.static( realDirectory ) );

diff --git a/projects/js-packages/critical-css-gen/tests/unit/generate-critical-css.test.js b/projects/js-packages/critical-css-gen/tests/unit/generate-critical-css.test.js
@@ -1,6 +1,6 @@
 const path = require( 'path' );
 const { chromium } = require( 'playwright' );
-const { generateCriticalCSS, BrowserInterfacePlaywright } = require( '../../build-node/node.js' );
+const { generateCriticalCSS, BrowserInterfacePlaywright } = require( '../../build/playwright.js' );
 const { dataDirectory } = require( '../lib/data-directory.js' );
 const mockFetch = require( '../lib/mock-fetch.js' );
 const TestServer = require( '../lib/test-server.js' );

diff --git a/projects/js-packages/critical-css-gen/tsconfig.browser.json b/projects/js-packages/critical-css-gen/tsconfig.browser.json
diff --git a/projects/js-packages/critical-css-gen/tsconfig.json b/projects/js-packages/critical-css-gen/tsconfig.json
@@ -1,9 +1,9 @@
 {
 	"extends": "jetpack-js-tools/tsconfig.tsc.json",
-	"include": [ "src/node.ts" ],
+	"include": [ "src/browser.ts", "src/playwright.ts" ],
 	"exclude": [ "node_modules/**/*" ],
 	"compilerOptions": {
-		"outDir": "./build-node",
+		"outDir": "./build",
 		"target": "es2019",
 		"sourceMap": true,
 		"allowJs": false,

diff --git a/projects/plugins/boost/app/admin/class-admin.php b/projects/plugins/boost/app/admin/class-admin.php
@@ -14,9 +14,7 @@
 use Automattic\Jetpack_Boost\Lib\Analytics;
 use Automattic\Jetpack_Boost\Lib\Environment_Change_Detector;
 use Automattic\Jetpack_Boost\Lib\Premium_Features;
-use Automattic\Jetpack_Boost\Modules\Modules_Index;
 use Automattic\Jetpack_Boost\Modules\Modules_Setup;
-use Automattic\Jetpack_Boost\Modules\Optimizations\Critical_CSS\Critical_CSS;
 
 class Admin {
 	/**
@@ -83,29 +81,13 @@ public function enqueue_scripts() {
 		 */
 		$internal_path = apply_filters( 'jetpack_boost_asset_internal_path', 'app/assets/dist/' );
 
-		$critical_css_gen_handle = 'jetpack-boost-critical-css-gen';
-
-		Assets::register_script(
-			$critical_css_gen_handle,
-			$internal_path . 'critical-css-gen.js',
-			JETPACK_BOOST_PATH,
-			array(
-				'in_footer' => true,
-			)
-		);
-
 		$admin_js_handle = 'jetpack-boost-admin';
 
 		$admin_js_dependencies = array(
 			'wp-i18n',
 			'wp-components',
 		);
 
-		// Enqueue the critical CSS generator script if Critical CSS is available.
-		if ( ( new Modules_Index() )->is_module_available( Critical_CSS::get_slug() ) ) {
-			$admin_js_dependencies[] = $critical_css_gen_handle;
-		}
-
 		Assets::register_script(
 			$admin_js_handle,
 			$internal_path . 'jetpack-boost.js',

diff --git a/projects/plugins/boost/app/assets/src/js/features/critical-css/lib/generate-critical-css.ts b/projects/plugins/boost/app/assets/src/js/features/critical-css/lib/generate-critical-css.ts
@@ -6,7 +6,6 @@ import { logPreCriticalCSSGeneration } from '$lib/utils/console';
 import { isSameOrigin } from '$lib/utils/is-same-origin';
 import { prepareAdminAjaxRequest } from '$lib/utils/make-admin-ajax-request';
 import { standardizeError } from '$lib/utils/standardize-error';
-import { SuccessTargetError } from 'jetpack-boost-critical-css-gen';
 
 type Viewport = {
 	width: number;
@@ -47,6 +46,12 @@ interface GeneratorCallbacks extends ProviderCallbacks {
 	onFinished: () => void; // Called when the generator is finished, regardless of success or failure.
 }
 
+async function criticalCssGenerator() {
+	return await import(
+		/* webpackChunkName: "jetpack-critical-css-gen" */ '@automattic/jetpack-critical-css-gen'
+	);
+}
+
 /**
  * Run the local Critical CSS Generator for a set of providers, if it is not already running.
  * The result of generation will not be returned to the caller; it will be sent to the given
@@ -137,10 +142,11 @@ async function generateCriticalCss(
  * @param {Object} requestGetParameters - GET parameters to include with each request.
  * @param {string} proxyNonce           - Nonce to use when proxying CSS requests.
  */
-function createBrowserInterface(
+async function createBrowserInterface(
 	requestGetParameters: Record< string, string >,
 	proxyNonce: string
 ) {
+	const CriticalCSSGenerator = await criticalCssGenerator();
 	return new ( class extends CriticalCSSGenerator.BrowserInterfaceIframe {
 		constructor() {
 			super( {
@@ -164,10 +170,6 @@ function createBrowserInterface(
 	} )();
 }
 
-function isSuccessTargetError( err: unknown ): err is SuccessTargetError {
-	return err instanceof Error && 'isSuccessTargetError' in err;
-}
-
 /**
  * Generate Critical CSS for the specified Provider Keys, sending each block
  * to the server. Throws on error or cancellation.
@@ -187,13 +189,20 @@ async function generateForKeys(
 	callbacks: ProviderCallbacks,
 	signal: AbortSignal
 ): Promise< void > {
+	const CriticalCSSGenerator = await criticalCssGenerator();
 	try {
 		CriticalCSSGeneratorSchema.parse( CriticalCSSGenerator );
 	} catch ( err ) {
 		recordBoostEvent( 'critical_css_library_failure', {} );
 		throw new Error( 'css-gen-library-failure' );
 	}
 
+	function isSuccessTargetError(
+		err: unknown
+	): err is InstanceType< typeof CriticalCSSGenerator.SuccessTargetError > {
+		return err instanceof CriticalCSSGenerator.SuccessTargetError;
+	}
+
 	// eslint-disable-next-line @wordpress/no-unused-vars-before-return
 	const startTime = Date.now();
 	let totalSize = 0;
@@ -209,7 +218,7 @@ async function generateForKeys(
 
 		try {
 			const [ css ] = await CriticalCSSGenerator.generateCriticalCSS( {
-				browserInterface: createBrowserInterface( requestGetParameters, proxyNonce ),
+				browserInterface: await createBrowserInterface( requestGetParameters, proxyNonce ),
 				urls,
 				viewports,
 				progressCallback: ( step: number, total: number ) => {
@@ -336,7 +345,7 @@ function keepAtRule( name: string ): boolean {
 /**
  * Helper method to filter out properties that we don't want.
  * Note this function is used as a filter in the generateCriticalCSS function
- * in the jetpack-boost-critical-css-gen library (https://github.com/Automattic/jetpack-boost-critical-css-gen).
+ * in the @automattic/jetpack-critical-css-gen library (https://github.com/Automattic/jetpack-critical-css-gen).
  *
  * This function has a value parameter which is not being used here but other implementations of this
  * helper function for the library may require the value parameter for filtering.
@@ -355,7 +364,7 @@ function keepProperty( name: string, _value: string ): boolean {
  * Function to verify that a specific page is valid to run the Critical CSS process on it.
  *
  * Note that this function is used as a callback in the generateCriticalCSS function
- * in the jetpack-boost-critical-css-gen library (https://github.com/Automattic/jetpack-boost-critical-css-gen).
+ * in the @automattic/jetpack-critical-css-gen library (https://github.com/Automattic/jetpack-critical-css-gen).
  *
  * This function has a url and innerWindow parameters which are not being used here but this method
  * is called with URL and InnerWindow in that library to offer flexibility of the verification for other implementation.

diff --git a/projects/plugins/boost/app/assets/src/js/global.d.ts b/projects/plugins/boost/app/assets/src/js/global.d.ts
@@ -2,8 +2,6 @@
  * Type definitions for the global namespace. i.e.: things we expect to find in window.
  */
 
-import type { BrowserInterfaceIframe, generateCriticalCSS } from 'jetpack-boost-critical-css-gen';
-
 // <reference types ="@types/jquery"/>
 
 declare global {
@@ -38,12 +36,6 @@ declare global {
 		};
 	};
 
-	// Critical CSS Generator library.
-	const CriticalCSSGenerator: {
-		generateCriticalCSS: typeof generateCriticalCSS;
-		BrowserInterfaceIframe: typeof BrowserInterfaceIframe;
-	};
-
 	const jpTracksAJAX: {
 		record_ajax_event: (
 			eventName: string,

diff --git a/projects/plugins/boost/changelog/update-use-monorepo-css-gen-package b/projects/plugins/boost/changelog/update-use-monorepo-css-gen-package
@@ -0,0 +1,5 @@
+Significance: patch
+Type: changed
+Comment: Use css gen package from monorepo instead of external repo.
+
+