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

Add a public bundledPackages/excludedExternals option to DEWP #45948

Open
wants to merge 5 commits into
base: trunk
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 2 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
16 changes: 16 additions & 0 deletions packages/dependency-extraction-webpack-plugin/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -135,6 +135,22 @@ This option is useful only when the `combineAssets` option is enabled. It allows

Pass `useDefaults: false` to disable the default request handling.

##### `excludedExternals`

- Type: `array<string>`
- Default: `[ '@wordpress/icons', '@wordpress/interface' ]`

The list of dependencies to exclude from externalizing. The packages listed here will not reach `requestToExternal` callback.
You can provide a new or extend that list with
```javascript
new DependencyExtractionWebpackPlugin( {
excludedExternals: [
...DependencyExtractionWebpackPlugin.excludedExternals,
'@wordpress/components'
]
} )
```

##### `injectPolyfill`

- Type: boolean
Expand Down
11 changes: 11 additions & 0 deletions packages/dependency-extraction-webpack-plugin/lib/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ const { createHash } = webpack.util;
* Internal dependencies
*/
const {
EXCLUDED_EXTERNALS,
defaultRequestToExternal,
defaultRequestToHandle,
} = require( './util' );
Expand All @@ -23,6 +24,7 @@ class DependencyExtractionWebpackPlugin {
constructor( options ) {
this.options = Object.assign(
{
excludedExternals: this.constructor.excludedExternals,
combineAssets: false,
combinedOutputFile: null,
externalizedReport: false,
Expand Down Expand Up @@ -53,6 +55,9 @@ class DependencyExtractionWebpackPlugin {
}

externalizeWpDeps( _context, request, callback ) {
if ( this.options.excludedExternals.includes( request ) ) {
return callback();
}
let externalRequest;

// Handle via options.requestToExternal first.
Expand Down Expand Up @@ -277,4 +282,10 @@ class DependencyExtractionWebpackPlugin {
}
}

// Define a static member in the eslint's ES supported way.
/**
* Set of packages to be bundled regardless of `requestToExternal` result.
*/
DependencyExtractionWebpackPlugin.excludedExternals = EXCLUDED_EXTERNALS;

module.exports = DependencyExtractionWebpackPlugin;
7 changes: 2 additions & 5 deletions packages/dependency-extraction-webpack-plugin/lib/util.js
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
const WORDPRESS_NAMESPACE = '@wordpress/';
const BUNDLED_PACKAGES = [ '@wordpress/icons', '@wordpress/interface' ];
const EXCLUDED_EXTERNALS = [ '@wordpress/icons', '@wordpress/interface' ];

/**
* Default request to global transformation
Expand Down Expand Up @@ -38,10 +38,6 @@ function defaultRequestToExternal( request ) {
return 'ReactRefreshRuntime';
}

if ( BUNDLED_PACKAGES.includes( request ) ) {
return undefined;
}

if ( request.startsWith( WORDPRESS_NAMESPACE ) ) {
return [
'wp',
Expand Down Expand Up @@ -93,6 +89,7 @@ function camelCaseDash( string ) {
}

module.exports = {
EXCLUDED_EXTERNALS,
camelCaseDash,
defaultRequestToExternal,
defaultRequestToHandle,
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,23 @@
// Jest Snapshot v1, https://goo.gl/fbAQLP

exports[`DependencyExtractionWebpackPlugin Webpack \`bundled-packages\` should produce expected output: Asset file 'index.min.asset.php' should match snapshot 1`] = `
"<?php return array('dependencies' => array('wp-blob'), 'version' => 'e46beb725c995b0c555a');
"
`;

exports[`DependencyExtractionWebpackPlugin Webpack \`bundled-packages\` should produce expected output: External modules should match snapshot 1`] = `
Array [
Object {
"externalType": "window",
"request": Array [
"wp",
"blob",
],
"userRequest": "@wordpress/blob",
},
]
`;

exports[`DependencyExtractionWebpackPlugin Webpack \`combine-assets\` should produce expected output: Asset file 'assets.php' should match snapshot 1`] = `
"<?php return array('fileA.js' => array('dependencies' => array('lodash', 'wp-blob'), 'version' => 'cf268e19006bef774112'), 'fileB.js' => array('dependencies' => array('wp-token-list'), 'version' => '7f3970305cf0aecb54ab'));
"
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
/**
* External dependencies
*/
import { isEmpty } from 'lodash';
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Note that those imports might need an ESLint ignore now that we're fully removing Lodash in #52571

Suggested change
import { isEmpty } from 'lodash';
// eslint-disable-next-line no-restricted-imports
import { isEmpty } from 'lodash';

Copy link
Contributor Author

@tomalec tomalec Jul 14, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the heads-up.

I changed lodash to react-dom in the test fixture (6c0863c). I think it's more sustainable and future-proof to test against something we plan to support in the longer term instead of silencing warnings.


/**
* WordPress dependencies
*/
import { isBlobURL } from '@wordpress/blob';

isEmpty( isBlobURL( '' ) );
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
/**
* Internal dependencies
*/
const DependencyExtractionWebpackPlugin = require( '../../..' );

module.exports = {
output: {
filename: 'index.min.js',
},
plugins: [
new DependencyExtractionWebpackPlugin( {
excludedExternals: [ 'lodash' ],
} ),
],
};
13 changes: 13 additions & 0 deletions packages/dependency-extraction-webpack-plugin/test/statics.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
/**
* Internal dependencies
*/
const DependencyExtractionWebpackPlugin = require( '../lib/index' );

describe( 'DependencyExtractionWebpackPlugin', () => {
test( 'should have .excludedExternals static property', () => {
expect( DependencyExtractionWebpackPlugin ).toHaveProperty(
'excludedExternals',
[ '@wordpress/icons', '@wordpress/interface' ]
);
} );
} );