Handle translations in lazy loaded files #4392
Conversation
|
Size Change: -62.8 kB (-6%) ✅ Total Size: 1.04 MB
ℹ️ View Unchanged
|
There was a problem hiding this comment.
The RequireChunkCallbackPlugin has one flaw that prevents it from working correctly when multiple blocks that use it (i.e., multiple independent webpack compilations) are loaded into one HTML document: there is only one global window.__requireChunkCallback__ object and different blocks will overwrite each other's data.
One solution is to not assign __requireChunkCallback__ to window, but to make it into a special webpack-interpreted global variable. Webpack has many such module variables, like __webpack_public_path__.
Here is the webpack code that finds these variables in the AST and outputs code to interpret them specially: https://github.com/webpack/webpack/blob/master/lib/APIPlugin.js
@yuliyan This could be a valuable improvement of the plugin. And if it gets popular, we could move it to the Gutenberg repo and make it a standard part of the block building workflow.
| // This fixes an issue with multiple webpack projects using chunking | ||
| // overwriting each other's chunk loader function. | ||
| // See https://webpack.js.org/configuration/output/#outputjsonpfunction | ||
| jsonpFunction: 'webpackWcBlocksJsonp', |
There was a problem hiding this comment.
What has changed so that the JSONP functions from different blocks no longer conflict and don't need to be assigned different names?
There was a problem hiding this comment.
jsonpFunction is no longer an option in webpack 5, it's now replaced with chunkLoadingGlobal.
However, I couldn't find any reference for this webpackWcBlocksJsonp. I'm not sure what we're trying to solve here.
The original PR that introduced this (2 years ago) provides very little rationale about what was happening here.
Deleting it doesn't seem to break anything in my testing.
| ); | ||
| return `${ blocksConfig.languageUrl }/${ domain }-${ blocksConfig.locale }-${ hash }.json`; | ||
| }, | ||
| translatedChunks: [ |
There was a problem hiding this comment.
I'm not sure if having the list of translation chunks hardcoded is the best approach. For languages with incomplete translations, some of the language files for the listed chunks might be missing, which would result in 404 for the translation request.
Would it be possible to generate the translated chunks list from the back-end and provide it to the app using the blocksConfig object, e.g.:
$translated_chunks = array_map(
'basename',
glob( WP_LANG_DIR . '/plugins/woo-gutenberg-products-block-' . get_locale() . '*.json' )
);There was a problem hiding this comment.
We still have to do some cross-checking to know which file to load I guess right? which means we still need to generate the hash on the frontend again? $translated_chunks returns 227 files for the Japanse translation of WooCommerce blocks.
There was a problem hiding this comment.
I applied this feedback @yuliyan if you want to take another look.
I'm not fully convinced of this approach. Passing the full list of files being translated adds 16kb to the page size (227) while we might need only 4 or 5. So on a later PR I might just pass down atomic blocks (the ones being dynamically loaded).
There was a problem hiding this comment.
I didn't expect the files to be that many, but yeah, in this case it doesn't make a lot of sense to provide all that data to the front-end and increase the page size unnecessarily.
As you suggested, it would be better if you could pass only the files that are relevant to the front, maybe something like that could do the job:
$files_with_translations = array_map(
'md5',
array(
'build/atomic-block-components/add-to-cart.js',
'build/atomic-block-components/button-frontend.js',
'build/atomic-block-components/rating.js',
'build/atomic-block-components/title.js',
'build/atomic-block-components/tag-list.js',
...
) // hardcode the atomic block filepaths, or get them dynamically with another glob.
);And then the glob pattern would be:
glob( WP_LANG_DIR . '/plugins/woo-gutenberg-products-block-' . get_locale() . "-{{$files_with_translations}}" . '*.json', GLOB_BRACE );
|
@jsnajdr I wasn't able to reliably add a module variable, the webpack page doesn't provide any meaningful info about adding. I tried both |
senadir
force-pushed
the
fix/4019-lazy-translations
branch
from
56a8de1 to
a6a0115
Compare
mikejolley
force-pushed
the
fix/4019-lazy-translations
branch
from
473eae3 to
f158a9a
Compare
nielslange
added
the
focus: i18n
Adds support for loading translations for lazy-loaded components.
RequireChunkCallbackPluginis borrowed from Calypso.On the blocks side, we use
addRequireChunkTranslationsHandlerto provide a list of Chunk IDs containing translations.getTranslationChunkFileUrlis a callback that maps the Chunk ID to a JSON filename in the WP_LANG dir. This consists of the textdomain, locale, and a hash for the script file itself.When a file is imported by React,
addRequireChunkTranslationsHandlerwill run and fetch the JSON file. If successful, we simply pass the body tosetLocaleDataso the translation is then performed by the@wordpress/i18npackage.Fixes #4019
Closes #4392
Testing
Testing translations:
Testing builds:
Also, test builds (dev and production) because this required an update to Webpack 5x.
Core plugin
TBC - We need a way to test under core context because the domain and paths are different. I've added handling for this but it's not tested.
Changelog