Skip to content

Commit

Permalink
Merge pull request #2072 from woocommerce/dev/externalize-wc-packages
Browse files Browse the repository at this point in the history
Externalize all `@woocommerce` packages via DEWP and remove the selective bundling implementation
  • Loading branch information
eason9487 authored Sep 1, 2023
2 parents 7016b25 + 0157e87 commit 2854ba9
Show file tree
Hide file tree
Showing 7 changed files with 14 additions and 52 deletions.
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"]
14 changes: 10 additions & 4 deletions Working with DEWP.md
Original file line number Diff line number Diff line change
Expand Up @@ -19,11 +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.
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.
In the past, we did bundle some packages provided by WordPress/WooCommerce instances. We did so to use a specific package version, for example, to ship a new feature we need, fix, or avoid a bug. However, bundling a package tends to introduce other 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 import from an external package when possible.

To help with that we implemented the `extracted/` prefix. It's also a custom implementation in [/webpack.config.js](/develop/webpack.config.js).
Thanks to that even though a package is bundled, the given import would fetch it from external.
To help with that, we had implemented the `extracted/` prefix in the past. It was also a custom implementation in [`webpack.config.js`](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 we also removed the implementation of the `extracted/` prefix.
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
Alternatively, we can consider an opposite approach and selectively **bundle** with a similar prefix implementation.

## NPM scripts

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

0 comments on commit 2854ba9

Please sign in to comment.