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

Use the new i18n-loader-webpack-plugin #21936

Merged
merged 12 commits into from
Dec 15, 2021
Merged
Show file tree
Hide file tree
Changes from 11 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
18 changes: 10 additions & 8 deletions pnpm-lock.yaml

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

6 changes: 6 additions & 0 deletions projects/js-packages/webpack-config/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -175,6 +175,12 @@ This provides an instance of [duplicate-package-checker-webpack-plugin](https://

This provides an instance of [@wordpress/dependency-extraction-webpack-plugin](https://www.npmjs.com/package/@wordpress/dependency-extraction-webpack-plugin). The `options` are passed to the plugin.

##### `I18nLoaderPlugin( options )`

This provides an instance of [@automattic/i18n-loader-webpack-plugin](https://www.npmjs.com/package/@automattic/i18n-loader-webpack-plugin). The `options` are passed to the plugin.

Note that if the plugin actually does anything in your build, you'll need to specify at least the `domain` option for it.

##### `I18nCheckPlugin( options )`

This provides an instance of [@wordpress/i18n-check-webpack-plugin](https://www.npmjs.com/package/@wordpress/i18n-check-webpack-plugin). The `options` are passed to the plugin.
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
Significance: major
Type: added

Add `@automattic/i18n-loader-webpack-plugin`. This may break some builds.
3 changes: 2 additions & 1 deletion projects/js-packages/webpack-config/package.json
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
{
"private": true,
"name": "@automattic/jetpack-webpack-config",
"version": "0.5.1-alpha",
"version": "0.6.0-alpha",
"description": "Library of pieces for webpack config in Jetpack projects.",
"homepage": "https://jetpack.com",
"bugs": {
Expand All @@ -20,6 +20,7 @@
"@automattic/babel-plugin-preserve-i18n": "1.0.0",
"@automattic/babel-plugin-replace-textdomain": "workspace:^0.1.0",
"@automattic/i18n-check-webpack-plugin": "workspace:^0.1.1-alpha",
"@automattic/i18n-loader-webpack-plugin": "workspace:^0.1.0-alpha",
"@automattic/webpack-rtl-plugin": "5.1.0",
"@babel/compat-data": "7.16.4",
"@babel/helper-compilation-targets": "7.16.3",
Expand Down
5 changes: 5 additions & 0 deletions projects/js-packages/webpack-config/src/webpack.js
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ const DuplicatePackageCheckerWebpackPlugin = require( 'duplicate-package-checker
const MiniCssExtractPlugin = require( 'mini-css-extract-plugin' );
const MiniCSSWithRTLPlugin = require( './webpack/mini-css-with-rtl' );
const WebpackRTLPlugin = require( '@automattic/webpack-rtl-plugin' );
const I18nLoaderWebpackPlugin = require( '@automattic/i18n-loader-webpack-plugin' );
const I18nCheckWebpackPlugin = require( '@automattic/i18n-check-webpack-plugin' );

const MyCssMinimizerPlugin = options => new CssMinimizerPlugin( options );
Expand Down Expand Up @@ -66,6 +67,8 @@ const DuplicatePackageCheckerPlugin = options => [

const DependencyExtractionPlugin = options => [ new DependencyExtractionWebpackPlugin( options ) ];

const I18nLoaderPlugin = options => [ new I18nLoaderWebpackPlugin( options ) ];

const i18nFilterFunction = file => {
if ( ! /\.(?:jsx?|tsx?|cjs|mjs|svelte)$/.test( file ) ) {
return false;
Expand Down Expand Up @@ -101,6 +104,7 @@ const StandardPlugins = ( options = {} ) => {
...( options.DependencyExtractionPlugin === false
? []
: DependencyExtractionPlugin( options.DependencyExtractionPlugin ) ),
...( options.I18nLoaderPlugin === false ? [] : I18nLoaderPlugin( options.I18nLoaderPlugin ) ),
...( options.I18nCheckPlugin === false ? [] : I18nCheckPlugin( options.I18nCheckPlugin ) ),
];
};
Expand Down Expand Up @@ -135,6 +139,7 @@ module.exports = {
WebpackRtlPlugin: MyWebpackRtlPlugin,
DependencyExtractionPlugin,
DuplicatePackageCheckerPlugin,
I18nLoaderPlugin,
// Module rules and loaders.
TranspileRule,
CssRule,
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
Significance: patch
Type: changed

Updated package dependencies.
2 changes: 1 addition & 1 deletion projects/packages/connection-ui/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@
"@wordpress/data": "6.1.4"
},
"devDependencies": {
"@automattic/jetpack-webpack-config": "workspace:^0.5.1-alpha",
"@automattic/jetpack-webpack-config": "workspace:^0.6.0-alpha",
"@babel/core": "7.16.0",
"@babel/preset-env": "7.16.4",
"@babel/register": "7.16.0",
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
Significance: patch
Type: changed

Updated package dependencies.
2 changes: 1 addition & 1 deletion projects/packages/identity-crisis/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@
"@wordpress/data": "6.1.4"
},
"devDependencies": {
"@automattic/jetpack-webpack-config": "workspace:^0.5.1-alpha",
"@automattic/jetpack-webpack-config": "workspace:^0.6.0-alpha",
"@babel/core": "7.16.0",
"@babel/preset-env": "7.16.4",
"@babel/register": "7.16.0",
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
Significance: patch
Type: changed

Updated package dependencies.
2 changes: 1 addition & 1 deletion projects/packages/jitm/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@
},
"browserslist": "extends @wordpress/browserslist-config",
"devDependencies": {
"@automattic/jetpack-webpack-config": "workspace:^0.5.1-alpha",
"@automattic/jetpack-webpack-config": "workspace:^0.6.0-alpha",
"@wordpress/browserslist-config": "4.1.0",
"sass": "1.43.3",
"sass-loader": "12.2.0",
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
Significance: patch
Type: changed

Updated package dependencies.
2 changes: 1 addition & 1 deletion projects/packages/lazy-images/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@
"validate": "pnpm exec validate-es dist/"
},
"devDependencies": {
"@automattic/jetpack-webpack-config": "workspace:^0.5.1-alpha",
"@automattic/jetpack-webpack-config": "workspace:^0.6.0-alpha",
"copy-webpack-plugin": "9.0.1",
"intersection-observer": "0.12.0",
"webpack": "5.64.1"
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
Significance: patch
Type: changed

Updated package dependencies.
2 changes: 1 addition & 1 deletion projects/packages/my-jetpack/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@
],
"devDependencies": {
"@automattic/jetpack-base-styles": "workspace:^0.1.2",
"@automattic/jetpack-webpack-config": "workspace:^0.5.1-alpha",
"@automattic/jetpack-webpack-config": "workspace:^0.6.0-alpha",
"@babel/core": "7.16.0",
"@babel/preset-env": "7.16.4",
"@babel/register": "7.16.0",
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
Significance: patch
Type: changed

Updated package dependencies.
2 changes: 1 addition & 1 deletion projects/packages/search/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -65,7 +65,7 @@
"tiny-lru": "7.0.6"
},
"devDependencies": {
"@automattic/jetpack-webpack-config": "workspace:^0.5.1-alpha",
"@automattic/jetpack-webpack-config": "workspace:^0.6.0-alpha",
"@babel/core": "7.16.0",
"@babel/plugin-proposal-nullish-coalescing-operator": "7.16.0",
"@babel/preset-env": "7.16.4",
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
Significance: patch
Type: changed

Updated package dependencies.
2 changes: 1 addition & 1 deletion projects/plugins/backup/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,7 @@
},
"devDependencies": {
"@automattic/jetpack-base-styles": "workspace:^0.1.2",
"@automattic/jetpack-webpack-config": "workspace:^0.5.1-alpha",
"@automattic/jetpack-webpack-config": "workspace:^0.6.0-alpha",
"@babel/core": "7.16.0",
"@babel/preset-env": "7.16.4",
"@babel/register": "7.16.0",
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
Significance: patch
Type: other

Use `@automattic/i18n-loader-webpack-plugin` to remove the hack for Instant Search's lazy-loaded bundle.
Original file line number Diff line number Diff line change
Expand Up @@ -130,35 +130,6 @@ public function load_assets_with_parameters( $path_prefix, $plugin_base_path ) {
Assets::enqueue_script( 'jetpack-instant-search' );
$this->load_and_initialize_tracks();
$this->inject_javascript_options();

// It only inlines the translations for the script, but does not actually load the script.
$this->inject_translation_for_script(
plugins_url(
$path_prefix . '_inc/build/instant-search/jp-search.chunk-main-payload.js',
$plugin_base_path
)
);
}

/**
* Add inline translations for script `$payload_url` before loading `$before_handle` script.
*
* @param string $payload_url - The payload url for which we load the translations.
* @param string $before_handle - Inline the translations before this handle.
*/
protected function inject_translation_for_script( $payload_url, $before_handle = 'jetpack-instant-search' ) {
// Set a random name for the script.
$handle = 'jetpack-instant-search-' . wp_unique_id();
// Then register it, which is required for the next steps.
// phpcs:ignore WordPress.WP.EnqueuedResourceParameters.NoExplicitVersion
wp_register_script( $handle, $payload_url, array(), false, false );
// Set translation domain to `jetpack`, and we need to explicitly set the `path` to load translations files for WPCOM.
// Otherwise WPCOM would try to load from `WP_LANG_DIR . '/mu-plugins'` and fails.
wp_set_script_translations( $handle, 'jetpack', WP_LANG_DIR . '/plugins' );
Copy link
Contributor

Choose a reason for hiding this comment

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

in WPCOM, the translation path was resolved to mu-plugins if /plugins not specified here.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It should be ok since the path now isn't being determined by wpcom PHP code, but testing is a good idea.

Looks like we may have to wait until #21932 is merged first so the TC build on D70962-code will succeed. Or merge this into #21932 then test it with a D70957-code that contains the changes from here too, if reviewers would rather.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

So it turns out testing this is kind of a pain, because the Fusion patch doesn't include the changes to the Assets package but those are required for it to work.

Once I manually apply those in my sandbox, it almost works. It tries to fetch the right path, but apparently wpcom blocks access to .json files inside wp-content/languages/.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Figured it out, and updated the testing instructions so it should be easy for someone else to test it too.

// Inline the translations before `$before_handle` handle.
wp_add_inline_script( $before_handle, wp_scripts()->print_translations( $handle, false ), 'before' );
// Deregister the script as we don't really enqueue it from PHP side.
wp_deregister_script( $handle );
}

/**
Expand Down
2 changes: 1 addition & 1 deletion projects/plugins/jetpack/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -150,7 +150,7 @@
"devDependencies": {
"@automattic/color-studio": "2.5.0",
"@automattic/jetpack-base-styles": "workspace:^0.1.2",
"@automattic/jetpack-webpack-config": "workspace:^0.5.1-alpha",
"@automattic/jetpack-webpack-config": "workspace:^0.6.0-alpha",
"@automattic/remove-asset-webpack-plugin": "workspace:^0.1.1",
"@babel/core": "7.16.0",
"@babel/plugin-proposal-nullish-coalescing-operator": "7.16.0",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -200,6 +200,7 @@ module.exports = [
plugins: [
...jetpackWebpackConfig.StandardPlugins( {
DependencyExtractionPlugin: false,
I18nLoaderPlugin: false,
I18nCheckPlugin: false,
} ),
new webpack.NormalModuleReplacementPlugin(
Expand Down
1 change: 1 addition & 0 deletions projects/plugins/jetpack/tools/webpack.config.js
Original file line number Diff line number Diff line change
Expand Up @@ -173,6 +173,7 @@ module.exports = [
plugins: [
...jetpackWebpackConfig.StandardPlugins( {
DependencyExtractionPlugin: false,
I18nLoaderPlugin: false,
I18nCheckPlugin: false,
} ),
new StaticSiteGeneratorPlugin( {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,7 @@ module.exports = {
plugins: [
...jetpackWebpackConfig.StandardPlugins( {
DependencyExtractionPlugin: { injectPolyfill: true },
I18nLoaderPlugin: { textdomain: 'jetpack' },
} ),
definePaletteColorsAsStaticVariables(),
],
Expand Down
1 change: 1 addition & 0 deletions projects/plugins/jetpack/tools/webpack.config.search.js
Original file line number Diff line number Diff line change
Expand Up @@ -65,6 +65,7 @@ module.exports = {
requestToExternal,
requestToHandle: defaultRequestToHandle,
},
I18nLoaderPlugin: { textdomain: 'jetpack' },
} ),
// Replace 'debug' module with a dummy implementation in production
...( jetpackWebpackConfig.isDevelopment
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,7 @@ module.exports = {
plugins: [
...jetpackWebpackConfig.StandardPlugins( {
DependencyExtractionPlugin: { injectPolyfill: true },
I18nLoaderPlugin: { textdomain: 'jetpack' },
} ),
definePaletteColorsAsStaticVariables(),
],
Expand Down