Automattic / jetpack

Security, performance, marketing, and design tools — Jetpack is made by WordPress experts to make WP sites safer and faster, and help you grow your traffic.
https://jetpack.com/
Other
1.59k stars 799 forks source link

Build a bundle with parseable translation functions #11817

Closed simison closed 5 years ago

simison commented 5 years ago

When code is published to WordPress.org plugin repository, there is a new process (using this cli script) that reads JavaScript files, parses wp-i18n translation functions and turns strings from them into translation files for the plugin automatically.

When then enqueuing a script (e.g. editor[.min].js) in the plugin, WordPress knows to pass correct translation file for it.

Problem

When we bundle JavaScript with Webpack, readable functions like __( 'Bestpack' ) are turned into Webpack optimized internal references like:

Object(_wordpress_i18n__WEBPACK_IMPORTED_MODULE_10__["__"])('Bestpack')

If we would push JavaScript source files with readable __()s to SVN, we'd still have issue of enqueued filepaths not matching with source files, so we'll need to have functions readable in final bundles meant for shipping.

Solution(s)

We might need combination of both (and maybe some other options?) or otherwise we'd still have something like Object(o.__)("Bestpack") with only mangling turned off.

We can produce two sets of bundles: non-minified meant for debugging containing readable __() functions and minified files (with .min.js extensions) meant for production. Users can switch between these with SCRIPT_DEBUG constant and WordPress' translation function will recognize these file paths identical.

cc @Automattic/jetpack-crew @Automattic/gutenpack

lezama commented 5 years ago

I played with wp i18n a little bit. The tool that core uses to create pot files from plugins in the repo.

wp i18n make-pot _inc/blocks/map --ignore-domain does work, even without needing to use reserved: [ '__', 'wp.i18n' ].

Seems like webpack magic is being considered by the "Scanner" https://github.com/wp-cli/i18n-command/blob/master/src/JsFunctionsScanner.php#L196-L197

Notes:

lezama commented 5 years ago

it takes around 10 min to finish 😅 for all the blocks, but it works!

we get some warnings:

root@a73c0ee9d2af:/var/www/html/wp-content/plugins/jetpack# wp i18n make-pot _inc/blocks/ --ignore-domain --allow-root
Warning: The string "image %1$d of %2$d in gallery" contains placeholders but has no "translators:" comment to clarify their meaning. (editor-beta.js:25)
Warning: The string "%d character remaining" contains placeholders but has no "translators:" comment to clarify their meaning. (editor-beta.js:25)
Warning: The string "The price cannot have more than %d decimal place." contains placeholders but has no "translators:" comment to clarify their meaning. (editor-beta.js:25)
Warning: The string "%s is not a valid email address." contains placeholders but has no "translators:" comment to clarify their meaning. (editor-beta.js:25)
Warning: The string "%s and %s are not a valid email address." contains placeholders but has no "translators:" comment to clarify their meaning. (editor-beta.js:25)
Warning: Multiple placeholders should be ordered. (editor-beta.js:25)
Warning: The string "%s are not a valid email address." contains placeholders but has no "translators:" comment to clarify their meaning. (editor-beta.js:25)
Warning: The string "%d result found, use up and down arrow keys to navigate." contains placeholders but has no "translators:" comment to clarify their meaning. (editor-beta.js:25)
Warning: The string "From %s to %s" contains placeholders but has no "translators:" comment to clarify their meaning. (editor-beta.js:25)
Warning: Multiple placeholders should be ordered. (editor-beta.js:25)
Warning: The string "Join %s other subscriber" contains placeholders but has no "translators:" comment to clarify their meaning. (editor-beta.js:25)
Warning: Missing singular placeholder, needed for some languages. See https://developer.wordpress.org/plugins/internationalization/how-to-internationalize-your-plugin/#plurals (editor-beta.js:25)
Warning: Missing singular placeholder, needed for some languages. See https://developer.wordpress.org/plugins/internationalization/how-to-internationalize-your-plugin/#plurals (editor-beta.js:25)
Warning: The string "%d character" contains placeholders but has no "translators:" comment to clarify their meaning. (editor-beta.js:25)
Success: POT file successfully generated!
sirreal commented 5 years ago

Would be nice to get lints for those warnings.

simison commented 5 years ago

Would be nice to get lints for those warnings.

Added to the list: https://github.com/Automattic/wp-calypso/issues/31215#issuecomment-478938649

simison commented 5 years ago

Seems like we can close this as the process is picking up translations in our test:

https://translate.wordpress.org/projects/wp-plugins/jetpack/dev/en-gb/default/?filters%5Boriginal_id%5D=7555486

Since “reference” is _inc/blocks/editor-beta.js we can pretty safely assume things will just work. :heart:

p1554289872241500-slack-jetpack-crew