Skip to content

Reduce size of Webpack i18n plugin template boilerplate#9180

Merged
aduth merged 5 commits intomainfrom
aduth-webpack-i18n-template
Sep 11, 2023
Merged

Reduce size of Webpack i18n plugin template boilerplate#9180
aduth merged 5 commits intomainfrom
aduth-webpack-i18n-template

Conversation

@aduth
Copy link
Contributor

@aduth aduth commented Sep 8, 2023

🛠 Summary of changes

Updates the Webpack i18n template to reduce its size. The original implementation was meant as largely equivalent to Object.assign, but compatible with older browsers which didn't support it. Now that our browser support is such that Object.assign should always be available, we can compact the size of the template.

Since this is a boilerplate "header" of sorts, the impact is bigger for smaller translation files. Bigger files like document capture won't benefit as much.

Example:

Small file

yarn build && brotli-size public/packs/js/password_confirmation_component.en.js

Before: 130 bytes
After: 87 bytes
Diff: -43 bytes (-33.1%)

Medium file:

yarn build && brotli-size public/packs/js/pw-strength.en.js

Before: 952 bytes
After: 927 bytes
Diff: -25 bytes (-2.63%)

Large file:

yarn build && brotli-size public/packs/js/document-capture.en.js

Before: 3.02kb
After: 2.98kb
Diff: -0.4kb (-1.32%)

📜 Testing Plan

Verify no regression in behavior of JavaScript localization.

changelog: Internal, Performance, Reduce size of translation JavaScript files
@aduth aduth requested a review from allthesignals September 8, 2023 20:12
Copy link
Contributor

@allthesignals allthesignals left a comment

Choose a reason for hiding this comment

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

Seems reasonable to me. Any implications for running this in a node environment?

@aduth
Copy link
Contributor Author

aduth commented Sep 8, 2023

Seems reasonable to me. Any implications for running this in a node environment?

Not sure if you had specific concerns in mind, but Object.assign has been supported since Node.js v4, so we should be alright on that front at least.

@allthesignals
Copy link
Contributor

Seems reasonable to me. Any implications for running this in a node environment?

Not sure if you had specific concerns in mind, but Object.assign has been supported since Node.js v4, so we should be alright on that front at least.

More thinking about the window object since it was previously inside a function and wasn't sure what the implications might've been in a node context 🤷

@aduth
Copy link
Contributor Author

aduth commented Sep 11, 2023

Never one to miss a code golf opportunity, I found some small savings in 0db75fe by avoiding the initial undefined fallback relying on Object.assign disregarding undefined values other than the first parameter. I edited the original comment with updated performance impact. I just need to update the spec fixtures now.

Object.assign({ foo: 'bar' }, undefined);
// { foo: 'bar' }

@aduth
Copy link
Contributor Author

aduth commented Sep 11, 2023

Alright, I think I've squeezed as much as I'm going to get out of this after 2f93d03.

I also toyed with an idea to use spread operator, which was shorter uncompressed template, but (a) may have marginally worse performance due to doing a shallow clone of both objects rather than direct (mutative) merge, and (b) it compressed worse than the current implementation (91 bytes vs. 87 bytes brotli'd for password_confirmation_component.en.js).

Diff, for reference:

diff --git a/app/javascript/packages/rails-i18n-webpack-plugin/rails-i18n-webpack-plugin.js b/app/javascript/packages/rails-i18n-webpack-plugin/rails-i18n-webpack-plugin.js
index 520f48ab7..09c9ddbee 100644
--- a/app/javascript/packages/rails-i18n-webpack-plugin/rails-i18n-webpack-plugin.js
+++ b/app/javascript/packages/rails-i18n-webpack-plugin/rails-i18n-webpack-plugin.js
@@ -114,5 +114,5 @@ class RailsI18nWebpackPlugin extends ExtractKeysWebpackPlugin {
   /** @type {RailsI18nWebpackPluginOptions} */
   static DEFAULT_OPTIONS = {
-    template: '_locale_data=Object.assign(%j,this._locale_data)',
+    template: '_locale_data={...this._locale_data,...%j}',
     configPath: path.resolve(process.cwd(), 'config/locales'),
     defaultLocale: 'en',

@aduth aduth merged commit e9885a4 into main Sep 11, 2023
@aduth aduth deleted the aduth-webpack-i18n-template branch September 11, 2023 20:55
@aduth
Copy link
Contributor Author

aduth commented Sep 11, 2023

And another idea which works, is shorter uncompressed, but compresses worse (91 bytes for password_confirmation_component.en.js) and may have worse browser support on the nullish coalescing assignment ??= operator.

diff --git a/app/javascript/packages/rails-i18n-webpack-plugin/rails-i18n-webpack-plugin.js b/app/javascript/packages/rails-i18n-webpack-plugin/rails-i18n-webpack-plugin.js
index 520f48ab7..111876c2e 100644
--- a/app/javascript/packages/rails-i18n-webpack-plugin/rails-i18n-webpack-plugin.js
+++ b/app/javascript/packages/rails-i18n-webpack-plugin/rails-i18n-webpack-plugin.js
@@ -114,5 +114,5 @@ class RailsI18nWebpackPlugin extends ExtractKeysWebpackPlugin {
   /** @type {RailsI18nWebpackPluginOptions} */
   static DEFAULT_OPTIONS = {
-    template: '_locale_data=Object.assign(%j,this._locale_data)',
+    template: 'Object.assign(this._locale_data??={},%j)',
     configPath: path.resolve(process.cwd(), 'config/locales'),
     defaultLocale: 'en',

@aduth aduth mentioned this pull request Sep 13, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants