Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Externalize all @woocommerce packages via DEWP and remove the selective bundling implementation #2072

Merged
merged 5 commits into from
Sep 1, 2023
Merged
Show file tree
Hide file tree
Changes from 4 commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion .externalized.json
Original file line number Diff line number Diff line change
@@ -1 +1 @@
["@woocommerce/components","@woocommerce/customer-effort-score","@woocommerce/data","@woocommerce/navigation","@woocommerce/settings","@wordpress/api-fetch","@wordpress/components","@wordpress/compose","@wordpress/data","@wordpress/data-controls","@wordpress/date","@wordpress/deprecated","@wordpress/dom","@wordpress/element","@wordpress/hooks","@wordpress/html-entities","@wordpress/i18n","@wordpress/primitives","@wordpress/url","lodash","moment","react","react-dom"]
["@woocommerce/components","@woocommerce/currency","@woocommerce/customer-effort-score","@woocommerce/data","@woocommerce/date","@woocommerce/navigation","@woocommerce/number","@woocommerce/settings","@woocommerce/tracks","@wordpress/api-fetch","@wordpress/components","@wordpress/compose","@wordpress/data","@wordpress/data-controls","@wordpress/date","@wordpress/dom","@wordpress/element","@wordpress/hooks","@wordpress/html-entities","@wordpress/i18n","@wordpress/primitives","@wordpress/url","lodash","react","react-dom"]
9 changes: 7 additions & 2 deletions Working with DEWP.md
Original file line number Diff line number Diff line change
Expand Up @@ -19,12 +19,17 @@ So we could later inspect that.

## Selective bundling & extracting.

Sometimes the fact we do bundle a package that is provided by WordPress/WooCommerce instance introduces errors, as some packages are not effectively modular, so we face version conflicts, style collisions, etc.
In the past, we did bundle a package that is provided by WordPress/WooCommerce instance introduces errors, as some packages are not effectively modular, so we face version conflicts, style collisions, etc.
Also, we'd like to reduce the size of our bundle, so eventually, we aim to extract/externalize as much as possible and when possible import from an external package.

To help with that we implemented the `extracted/` prefix. It's also a custom implementation in [/webpack.config.js](/develop/webpack.config.js).
To help with that we had ever implemented the `extracted/` prefix. It was also a custom implementation in webpack.config.js.
Thanks to that even though a package is bundled, the given import would fetch it from external.

Now, we have already externalized all DEWP-able packages, so the implementation of `extracted/` prefix was removed as well.
If someday we ever need it again, please refer to:
- The PR implemented it: https://github.com/woocommerce/google-listings-and-ads/pull/1762
- The commit removed it: https://github.com/woocommerce/google-listings-and-ads/commit/5a2e20409a53ccb3b7fcbfe5c46988ca2b153b38

## NPM scripts

We also have a bunch of NPM scripts to help us work with them daily.
Expand Down
1 change: 0 additions & 1 deletion jest.config.js
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,6 @@ module.exports = {
'\\.scss$': '<rootDir>/tests/mocks/assets/styleMock.js',
// Transform our `.~/` alias.
'^\\.~/(.*)$': '<rootDir>/js/src/$1',
'^extracted/(.*)$': '$1',
'@woocommerce/settings':
'<rootDir>/js/src/tests/dependencies/woocommerce/settings',
'@automattic/calypso-config':
Expand Down
3 changes: 1 addition & 2 deletions js/src/jsconfig.json
Original file line number Diff line number Diff line change
Expand Up @@ -2,8 +2,7 @@
"compilerOptions": {
"baseUrl": ".",
"paths": {
".~/*": [ "./*" ],
"extracted/*": [ "*" ]
".~/*": [ "./*" ]
}
}
}
1 change: 0 additions & 1 deletion package-lock.json

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

5 changes: 2 additions & 3 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -59,7 +59,6 @@
"@types/jest": "^27.5.2",
"@woocommerce/dependency-extraction-webpack-plugin": "^2.0.0",
"@woocommerce/eslint-plugin": "^1.2.0",
"@wordpress/dependency-extraction-webpack-plugin": "^4.4.0",
"@wordpress/env": "^8.4.0",
"@wordpress/jest-preset-default": "^11.9.0",
"@wordpress/prettier-config": "2.18.1",
Expand Down Expand Up @@ -145,7 +144,7 @@
},
{
"path": "./js/build/index.js",
"maxSize": "209.08 kB"
"maxSize": "148.52 kB"
},
{
"path": "./js/build/*.css",
Expand All @@ -157,7 +156,7 @@
},
{
"path": "./google-listings-and-ads.zip",
"maxSize": "8.48 mB",
"maxSize": "8.22 mB",
"compression": "none"
}
],
Expand Down
40 changes: 0 additions & 40 deletions webpack.config.js
Original file line number Diff line number Diff line change
@@ -1,50 +1,13 @@
const defaultConfig = require( '@wordpress/scripts/config/webpack.config' );
const { hasArgInCLI } = require( '@wordpress/scripts/utils' );
const WooCommerceDependencyExtractionWebpackPlugin = require( '@woocommerce/dependency-extraction-webpack-plugin' );
const {
defaultRequestToExternal: defaultRequestToExternalWP,
defaultRequestToHandle: defaultRequestToHandleWP,
} = require( '@wordpress/dependency-extraction-webpack-plugin/lib/util' );

const ReactRefreshWebpackPlugin = require( '@pmmmwh/react-refresh-webpack-plugin' );
const path = require( 'path' );

const isProduction = process.env.NODE_ENV === 'production';
const hasReactFastRefresh = hasArgInCLI( '--hot' ) && ! isProduction;

const explicitlyExtractPrefix = 'extracted/';

const requestToExternal = ( request ) => {
// Externalized when explicitely asked for.
if ( request.startsWith( explicitlyExtractPrefix ) ) {
request = request.substr( explicitlyExtractPrefix.length );
return defaultRequestToExternalWP( request );
}
const bundledPackages = [
// Opt-out WooCommerce packages.
'@woocommerce/currency',
'@woocommerce/date',
'@woocommerce/number',
'@woocommerce/tracks',
];
if ( bundledPackages.includes( request ) ) {
return false;
}

// Follow with the default behavior for any other.
return undefined;
};

const requestToHandle = ( request ) => {
// Externalized when explicitely asked for.
if ( request.startsWith( explicitlyExtractPrefix ) ) {
request = request.substr( explicitlyExtractPrefix.length );
return defaultRequestToHandleWP( request );
}
// Follow with the default behavior for any other.
return undefined;
};

const exceptSVGAndPNGRule = ( rule ) => {
return ! rule.test.toString().match( /svg|png/i );
};
Expand Down Expand Up @@ -76,7 +39,6 @@ const webpackConfig = {
...defaultConfig.resolve,
alias: {
'.~': path.resolve( process.cwd(), 'js/src/' ),
extracted: path.resolve( __dirname, 'node_modules' ),
},
fallback: {
/**
Expand Down Expand Up @@ -108,8 +70,6 @@ const webpackConfig = {
new WooCommerceDependencyExtractionWebpackPlugin( {
externalizedReport:
! hasReactFastRefresh && '../../.externalized.json',
requestToExternal,
requestToHandle,
} ),
],
entry: {
Expand Down
Loading