Automatically load JavaScript translations when scripts are enqueued if these exist.

[herregroen]

In order to support javascript translations and allow these to be selectively loaded as described in ​#Meta3875 WordPress should load translations for a given JS file when that file is enqueued and the appropriate language pack exists.

Translations for JS files should be generated when ​#Meta3876 is resolved. This will mean that there will be separate language packs for every JS file present.

When a script is enqueued WordPress should check if a translation file for that specific script exists and, if so, use wp_localize_script to load all these translations. An additional script should also be added to then load these translation using wp.i18n.setLocaleData ( see ​@wordpress/i18n. Note that setLocaleData will only create a new Jed instance if none exist, otherwise the new domain will be merged into any existing domains ).

Because we do not know in advance which domains a script uses we should load all domains for which files exist, likely using glob to find all translation files for the given filename and current locale ).

Doing this would ensure that all translations a JS file uses are loaded into wp.i18n before that script is run and that only the translations that might possibly be used are loaded instead of every single translation.

[swissspidy]

Because we do not know in advance which domains a script uses we should load all domains for which files exist, likely using glob to find all language packs for the given filename and current locale ).

-1

We require developers to only use exactly one text domain in their projects, which is the plugin/theme slug.

If they're not following that, they're doing_it_wrong().

[nerrad]

There is a bit of code I worked on here: ​https://github.com/WordPress/gutenberg/pull/6051 with regards to the php side of things. Obviously the source of the strings will be different from that pull, but I think something similar would work well in core.

The main file/class taking care of detecting and queuing up which strings to register on wp.i18n.setLocaleData (via wp_add_inline_script which in turn is registered to the js files the strings belong to so when they are enqueued the strings are registered): ​https://github.com/WordPress/gutenberg/blob/8bc06fcc97da396894d145e60dd3e0f6f9cc4c3a/lib/class-gb-scripts.php

[ocean90]

Moving to 5.0 since this is required for the new editor.

[swissspidy]

Note that with ​#meta3876 we'd get Jed-formatted JSON files (one JSON file per JS file). It would be way easier if we can just pass these files to setLocaleData() instead of doing some hacky things server-side to get different domains.

[herregroen]

Replying to swissspidy:

Because we do not know in advance which domains a script uses we should load all domains for which files exist, likely using glob to find all language packs for the given filename and current locale ).

-1

We require developers to only use exactly one text domain in their projects, which is the plugin/theme slug.

If they're not following that, they're doing_it_wrong().

Good point and agreed. In that case it's a matter of figuring out the originating plugin by the enqueued's script's path, and loading the appropriate file from there. There is the use case of premium plugins which bundle functionality of their free counterpart and have separate domains for translations from the free plugin and premium additions so I do believe this should be filterable.

Replying to swissspidy:

Note that with ​#meta3876 we'd get Jed-formatted JSON files (one JSON file per JS file). It would be way easier if we can just pass these files to setLocaleData() instead of doing some hacky things server-side to get different domains.

Agreed. I think the ideal implementation would be basically @nerrad his function except it can perhaps take a file path directly and simply use get_file_contents instead of having to decode and then encode the json again.

That just leaves automatically detecting if translation files exist for enqueued scripts I think.

[nerrad]

I'm wondering if we should introduce an api for scripts to indicate they have translation strings. It be nice if we could put extra arguments on wp_register_script but that already has a lot of arguments and retooling could be painful.

What about something like:

function wp_script_has_translation( $handle, $domain ) {
    $wp_scripts = wp_scripts();
    if ( ! ( $wp_scripts instanceof WP_Scripts ) ) {
        _wp_scripts_maybe_doing_it_wrong( __FUNCTION__ );
        return false;
    }
    return $wp_scripts->has_translation( $handle, $domain );
}

Then in WP_Scripts there could be tracking of what handles have been registered as having translation strings for when time comes to write wp.i18n.setLocaleData inline. The beauty of this is is saves having to auto-detect whether js files have translations or not (which could get expensive) and also ensures that the translation is only loaded when the script itself is enqueued. Plus it handily links the domain to the script so there's no guessing at what domain the script file belongs to (or trying to auto-detect that which could be difficult).

So the new api bits would be the function wp_script_has_translation and the logic etc for WP_Scripts::has_translation

[swissspidy]

I'm wondering if we should introduce an api for scripts to indicate they have translation strings.

I was about so say the same. We did something like that in #20491. See WP_Scripts:: load_translation_file() in https://core.trac.wordpress.org/attachment/ticket/20491/20491.7.diff

[herregroen]

Replying to nerrad:

I'm wondering if we should introduce an api for scripts to indicate they have translation strings. It be nice if we could put extra arguments on wp_register_script but that already has a lot of arguments and retooling could be painful.

What about something like:

function wp_script_has_translation( $handle, $domain ) {
    $wp_scripts = wp_scripts();
    if ( ! ( $wp_scripts instanceof WP_Scripts ) ) {
        _wp_scripts_maybe_doing_it_wrong( __FUNCTION__ );
        return false;
    }
    return $wp_scripts->has_translation( $handle, $domain );
}

Then in WP_Scripts there could be tracking of what handles have been registered as having translation strings for when time comes to write wp.i18n.setLocaleData inline. The beauty of this is is saves having to auto-detect whether js files have translations or not (which could get expensive) and also ensures that the translation is only loaded when the script itself is enqueued. Plus it handily links the domain to the script so there's no guessing at what domain the script file belongs to (or trying to auto-detect that which could be difficult).

So the new api bits would be the function wp_script_has_translation and the logic etc for WP_Scripts::has_translation

That should work.

From there, before a script is output in do_item, we can find it's relative path to the plugin, theme, child theme or root urls. md5 that and find the correct translation file in the appropriate language directory. The domain and locale are known so no issues there.

This can then be output using basically register_inline_script from the GB_Scripts class. Likely as a new method on WP_Scripts.

One more thing I think we should add is that in load_plugin_textdomain, load_muplugin_textdomain, load_theme_textdomain and load_child_theme_textdomain we should remember the paths passed so that, if the translation is for one of those registered domains we can also look in the directories passed there.

[herregroen]

I've added a WIP patch that I believe covers the basics of what we've discussed. It's still very much in progress and requires testing but I wanted to get your input @nerrad and @swissspidy if this feels like the right approach to take.

[nerrad]

Ya it looks like the right approach. However:

This assumes that translation files will always be in a specific format and have a specific filename. So its very oriented towards (I'm assuming) files built by Glotpress (in most cases via wp.org I'm assuming). Will there be allowance for plugins not hosted on wp.org and/or prepare their own translation files? In other words:

• Will core only be handling .json files (which I'm okay with, just want to clarify)? If so, we should document the expected shape of the json object.
• Is the shape of the json object what jed expects? I know in the work I did for the GB pull I had to coerce locale data into an object jed expected. I'm not familiar with what glotpress outputs so I just thought I'd raise this as a point to investigate if its not already known.
• I'm assuming the filename will be expected to be this explicit format (md5 hashed script name etc, and no provision for providing the name of the file)? If so, we'll need to document the expected filename structure.
• Along those lines, at the beginning of print_inline_translations we could maybe have a shortcut filter registered to $filename. And if that has a non empty value then the usual filename logic could be skipped. This could provide plugins the option to provide their own expected filename for a translation file? Or add a $filename argument?

[swissspidy]

Feels like this goes into a good direction 👍

Instead of setting the text domain on the dependency object, WP_Scripts::set_translations() could do all tasks from WP_Scripts:: print_inline_translations() and then simply call WP_Scripts::add_inline_script().

This would reduce complexity a bit as there are no changes needed to _WP_Dependency or WP_Scripts::do_item().

Oh, and WP_Scripts::set_translations() needs to add wp-i18n to the handle's list of dependencies.

I tried to do this in 45103.2.diff​. Please correct me if I'm wrong :-)

Will core only be handling .json files (which I'm okay with, just want to clarify)? If so, we should document the expected shape of the json object.

Yes (IMO) and yes.

Is the shape of the json object what jed expects? I know in the work I did for the GB pull I had to coerce locale data into an object jed expected. I'm not familiar with what glotpress outputs so I just thought I'd raise this as a point to investigate if its not already known.

GlotPress supports simple key-value JSON files and Jed-style JSON files for export. See ​#meta3876. I think it's possible and makes sense to support and use Jed-style files.

Makes documentation and adoption easier too, as this standard has already been around a while.

Should we perhaps reach out to l10n software vendors like Poedit to see if they would support JSON files in the future? Makes developers' lives easier.

I'm assuming the filename will be expected to be this explicit format (md5 hashed script name etc, and no provision for providing the name of the file)? If so, we'll need to document the expected filename structure.

Definitely needs to be documented. Perhaps for plugins shipping their own translations we can also accept {$domain}-{$locale}-{$handle}.json style file names, instead of requiring them to suddenly use md5. On WordPress.org we don't know the script handles, hence the file name hash approach.

This could provide plugins the option to provide their own expected filename for a translation file?

If we support something like {$domain}-{$locale}-{$handle}.json, I don't think such a filter is needed.

However, I think we should move all the filename logic to a more suitable place like wp-includes/l10n.php.

[nerrad]

I agree with all that @swissspidy wrote.

[herregroen]

Replying to swissspidy:

Feels like this goes into a good direction 👍

Instead of setting the text domain on the dependency object, WP_Scripts::set_translations() could do all tasks from WP_Scripts:: print_inline_translations() and then simply call WP_Scripts::add_inline_script().

This would reduce complexity a bit as there are no changes needed to _WP_Dependency or WP_Scripts::do_item().

Oh, and WP_Scripts::set_translations() needs to add wp-i18n to the handle's list of dependencies.

I tried to do this in 45103.2.diff​. Please correct me if I'm wrong :-)

Agree with the upsides to this approach and it might be the correct one, it should be noted however that this means that this means the translations file names will be calculated and all translations loaded into memory for every script that has translations as the inline script will always be set and stored regardless of whether that script is enqueued or not.

This effectively means that setting translations isn't like registering a script but more like enqueueing a script as it will actually perform all calculations even if those aren't output.

As long as this is documented and explained there should be no issue. We might even want to consider adding a doing_it_wrong() when setting translations on a script that's only been registered and not enqueued.

Replying to swissspidy:

Will core only be handling .json files (which I'm okay with, just want to clarify)? If so, we should document the expected shape of the json object.

Yes (IMO) and yes.

Completely in agreement.

Replying to swissspidy:

Is the shape of the json object what jed expects? I know in the work I did for the GB pull I had to coerce locale data into an object jed expected. I'm not familiar with what glotpress outputs so I just thought I'd raise this as a point to investigate if its not already known.

GlotPress supports simple key-value JSON files and Jed-style JSON files for export. See ​#meta3876. I think it's possible and makes sense to support and use Jed-style files.

Makes documentation and adoption easier too, as this standard has already been around a while.

Should we perhaps reach out to l10n software vendors like Poedit to see if they would support JSON files in the future? Makes developers' lives easier.

Since wp.i18n already wraps Jed from what I've been able to find it actually accepts the simple key-value JSON and will internally change this to the Jed-style. Passing Jed-style objects would cause the locale_data itself to become a Jed-style object.

That said it does seem to expect an empty key that contains the configuration settings, I'm actually not sure if this is included in the simple key-value JSON format or if it's something we should add.

Replying to swissspidy:

I'm assuming the filename will be expected to be this explicit format (md5 hashed script name etc, and no provision for providing the name of the file)? If so, we'll need to document the expected filename structure.

Definitely needs to be documented. Perhaps for plugins shipping their own translations we can also accept {$domain}-{$locale}-{$handle}.json style file names, instead of requiring them to suddenly use md5. On WordPress.org we don't know the script handles, hence the file name hash approach.

Great idea!

Replying to swissspidy:

This could provide plugins the option to provide their own expected filename for a translation file?

If we support something like {$domain}-{$locale}-{$handle}.json, I don't think such a filter is needed.

However, I think we should move all the filename logic to a more suitable place like wp-includes/l10n.php.

{$domain}-{$locale}-{$handle}.json would be the best option I think.

I'd say the following should be tried:

1. If a path has been passed, {$custom_path}/{$domain}-{$locale}-{$handle}.json.
2. If a path has been passed, {$custom_path}/{$domain}-{$locale}-{$md5}.json.
3. Always, {$languages_path}/{$domain}-{$locale}-{$md5}.json.

This would also leave the option for plugins to do provide their own language files but do so with their own GlotPress install.

[swissspidy]

Agree with the upsides to this approach and it might be the correct one, it should be noted however that this means that this means the translations file names will be calculated and all translations loaded into memory for every script that has translations as the inline script will always be set and stored regardless of whether that script is enqueued or not.

This effectively means that setting translations isn't like registering a script but more like enqueueing a script as it will actually perform all calculations even if those aren't output.

As long as this is documented and explained there should be no issue. We might even want to consider adding a doing_it_wrong() when setting translations on a script that's only been registered and not enqueued.

You're right, this changes semantics. Documentation + doing_it_wrong() might be enough, though there could be cases where one enqueues a script, translations are loaded, and later on the script is dequeued again. I guess we should do some testing to see whether this an issue worth addressing or not.

In the more modern PHP world I'd probably solve this by passing a closure (or proxy object like in #41305) to WP_Scripts::add_inline_script() instead of a string. So kinda like a mix between the first two patches.

Since wp.i18n already wraps Jed from what I've been able to find it actually accepts the simple key-value JSON and will internally change this to the Jed-style. Passing Jed-style objects would cause the locale_data itself to become a Jed-style object.

That said it does seem to expect an empty key that contains the configuration settings, I'm actually not sure if this is included in the simple key-value JSON format or if it's something we should add.

We need to use Jed-style files so that we automatically have the proper plural forms information and things like that.

Here's an example file: ​https://github.com/GlotPress/GlotPress-WP/blob/0de4570063cba85049bdce214ae7e7ff93a2193e/tests/phpunit/data/translation-jed1x.json

We can get the locale_data object in PHP, maybe override the domain if necessary and then pass this object to wp.i18n.setLocaleData().

I'd say the following should be tried:

Sounds good!

[herregroen]

Added a patch that moves loading the translations to l10n.php as suggested, first checks if the handle filename exists to avoid calculating the relative path and changes the relative path calculation somewhat to account for differing URL protocols and relative URLs.

This also expects the translations to be in JED format. ​#Meta3876 has been updated accordingly.

[herregroen]

@swissspidy and @nerrad if the latest version is looking good to you I'd like to move forward to committing this to the 5.0 branch so we can move forward with #45161.

Please let me know if you feel there's anything further required before we can move forward.

[nerrad]

Apologies for delay in getting back to you, I'm in a bit of a time crunch working on GB related tasks for my own projects :)

Work looks good. After thinking more about it though, I'm a bit concerned about the requirement to set translations after enqueuing scripts. Primary reason is it diverges a bit from the established pattern for working with the wp_scripts api that exists and developers are used to. Namely, wp_localize_script and wp_add_inline_script all currently encourage adding information immediately subsequent to registering a script.

I know in the projects I currently work on, we tend to register scripts, their handles and meta data early in requests and then enqueues are handled separately.

I'd like to see us move back towards just registering locale details with WP_Scripts::set_translations so translations can be registered after registering a script.

I realize there is a bit of a time crunch though, so this could be landed and iterated on as is. I'm just not keen on this being the final approach.

[swissspidy]

I‘ll try to wrap my head around this later the day too, to figure out how we can change this.

In any case we need to add unit tests that describe this functionality. It‘s a great place to document the existing format too.

[omarreiss]

Seems like the current patch is in an acceptable state for testing when I read the comments right. If possible, I would like to be pragmatic and have the first version of this committed tomorrow. I totally think we should iterate, add unit tests and try to implement @nerrad's suggestion. But if we can get a working version of this in the 5.0 branch, that would help us get testing feedback sooner. Whenever a beta2 ships, it would for sure contain a version of this code. I think that would be highly beneficial in polishing this feature.

[herregroen]

Updated the patch to unify .min.js and .js translations as core now contains both minified and unminified files of translations. ​#Meta3876 has also been updated to reflect this change.

Current plan is to move to committing this to the 5.0 branch so we have these functions ready for the next beta.

Ideally we'd also have meta in a state by the time beta2 ships where the necessary translations can be downloaded. Alternatively we could look into shipping the json translation files with beta2 so that this can still be tested.

[herregroen]

In 43825:

I18N: Add JavaScript translation support.

Adds the wp_set_script_translations function which registers translations for a JavaScript file. This function takes a handle, domain and optionally a path and ensures JavaScript translation files are loaded if they exist.

Props atimmer, omarreiss, nerrad, swissspidy, ocean90.
Fixes #45103.

[ocean90]

• For core, the file should be en_US-813e104eb47e13dd4cc5af844c618754.json without the default- prefix
• wp_set_script_translations() should not have the @link tag to this ticket.
• load_script_textdomain() has no @since tag
• The path detection seems fragile, there should be a few more tests to also test that it works with plugins and themes.
• The naming is a bit off, set_translations() vs. Register a translation textdomain.
• Tests are broken in PHP 5.2: 1) Tests_Dependencies_Scripts::test_wp_set_script_translation: Use of undefined constant __DIR__ - assumed '__DIR__'

[SergeyBiryukov]

In 43828:

Tests: Use DIR_TESTDATA in test_wp_set_script_translation().

__DIR__ is not available in PHP 5.2.x.

See #45103.

[vslavik]

Should we perhaps reach out to l10n software vendors like Poedit to see if they would support JSON files in the future? Makes developers' lives easier.

Poedit author here. Just recently, Poedit gained ability to support multiple formats, starting with XLIFF, and adding more formats is relatively easier now. I'd love to support Jed JSON files, including specific WordPress support -- it's just not always clear for a relative outsider like to me to know what the pain points are (besides basic Jed support, obviously), and I could use some guidance. I'm happy to discuss your needs (my email is vaclav at slavik.io, or Poedit's support channels).

[herregroen]

Uploaded a latest patch ( .8.diff was moving one of the tests files but that seems to not show up in the diff ) with the following additions:

• Expand unit tests.
• Make setting translations lazy so they are only loaded when printed. This is because otherwise when enqueueing a script with dependencies you would have to set translations on all your dependencies as well.
• Fix path comparison for plugin files.

@nerrad @swissspidy @ocean90 I'm hoping to commit this tomorrow to ensure it's there once meta gets updated to produce the new language packs and also to allow me to start setting the appropriate translations for all gutenberg scripts for #45161.

I would love your feedback.

[nerrad]

Make setting translations lazy so they are only loaded when printed. This is because otherwise when enqueueing a script with dependencies you would have to set translations on all your dependencies as well.

If you're lazy loading when printing, this should make it possible to set the translations after registering the script right? In which case, it'd be nice to have tests verify that as well.

[herregroen]

Replying to nerrad:

Make setting translations lazy so they are only loaded when printed. This is because otherwise when enqueueing a script with dependencies you would have to set translations on all your dependencies as well.

If you're lazy loading when printing, this should make it possible to set the translations after registering the script right? In which case, it'd be nice to have tests verify that as well.

test_wp_set_script_translations_after_register should cover that scenario.

[nerrad]

Replying to herregroen:

Replying to nerrad:

Make setting translations lazy so they are only loaded when printed. This is because otherwise when enqueueing a script with dependencies you would have to set translations on all your dependencies as well.

If you're lazy loading when printing, this should make it possible to set the translations after registering the script right? In which case, it'd be nice to have tests verify that as well.

test_wp_set_script_translations_after_register should cover that scenario.

Oops completely missed that (obviously)!

[omarreiss]

Signing off on committing that latest patch!

[herregroen]

In 43859:

I18N: Improve JavaScript translation support.

Improves upon [43825] by adding unit tests to cover additional use-cases and changes loading translations to only occur when outputting the script to allow setting translations on dependencies.

Props atimmer, omarreiss, nerrad, swissspidy, ocean90.
Fixes #45103.

[pento]

@herregroen: Could you please write a dev-note for this, showing how plugin developers should register and enqueue script translations?

[tfirdaus]

@herregroen Thank you for the hard work on this function, wp_set_script_translations.
I noticed that this function currently throws a PHP warning notice, as follows:

Undefined index: path at wp-includes/l10n.php:932

One plugin, ​https://wordpress.org/plugins/tarot/, happens to have implemented this function, and this PHP warning persists on this plugin.

Also, using wp.i18n triggers an error in the browser Console.

Jed localization error: 
Error: Domain `my-domain` was not found.

Is there something else we need to configure to get these functions working without the error notices? It seems I've probably missed something here.

[herregroen]

@tfirdaus Thanks for the report!

Just had a quick look. I think there's two things going on here.

1. The tarot plugins doesn't seem have any translations at all: ​https://translate.wordpress.org/locale/en/default/wp-plugins/tarot as such there's no translations to be loaded or registered.
2. In this situation the above error is shown, this doesn't interrupt script execution but I understand this is confusing. I've got a patch ready to make sure that if no translations exist the domain is still registered to prevent these errors.

The patch also includes a fix to ensure no warning is thrown.

[omarreiss]

Fix looks good to me. Feel free to commit!

[ocean90]

Related report: #45256

45103.10.diff​:

• else if should be elseif: ​https://make.wordpress.org/core/handbook/best-practices/coding-standards/php/#use-elseif-not-else-if
• Can we add a test for this case?

[tfirdaus]

@herregroen Thanks :) I can confirm it's fixed now with the Beta 5.

[ocean90]

45103.11.diff​ was committed in [43898].

[pento]

In 44169:

I18N: Add JavaScript translation support.

Adds the wp_set_script_translations() function which registers translations for a JavaScript file. This function takes a handle, domain and optionally a path and ensures JavaScript translation files are loaded if they exist.

Merges [43825,43828,43859,43898] from the 5.0 branch to trunk.

Props herregroen, atimmer, omarreiss, nerrad, swissspidy, ocean90, georgestephanis.
Fixes #45103, #45256.

[desrosj]

This change was detailed in the following dev note: ​https://make.wordpress.org/core/2018/11/09/new-javascript-i18n-support-in-wordpress/