WIP: i18n support for plugins #9799

jburger424 · 2019-10-07T21:13:10Z

This is just a start but wanted to get initial thoughts.

This allows for a plugin to define a directory containing their locale files, we assume that all files follow the format ${locale}.json.

I've introduced aliases.js which should be able to be used for both core and plugin locales, however I may be missing something in the purpose of locales.js.

Bug: #9641

Add TODO.

lighthouse-core/lib/i18n/aliases.js

lighthouse-core/lib/i18n/i18n.js

paulirish · 2019-10-11T23:39:12Z

lighthouse-core/lib/i18n/i18n.js

+    const locale = tempPath.replace(/\.json$/, '');
+    const relativePrefix = path.relative(LH_ROOT, rootPath).replace(/\\/g, '/');
+    if (LOCALES[locale]) {
+      const localeToMerge = require(`${fullLocalePath}/${tempPath}`);


can you separate this inner bit into its own function?

appendStringsToLocale ?

lighthouse-core/config/config.js

paulirish · 2019-10-11T23:50:43Z

lighthouse-core/config/config.js

      const rawPluginJson = require(pluginPath);
      const pluginJson = ConfigPlugin.parsePlugin(rawPluginJson, pluginName);

+      if (rawPluginJson['localePath']) {
+        const pluginParentPath = 
+          pluginPath.replace(new RegExp(`/${pluginName}/.*`,'g'), '');


there's gotta be a better way to do this.. ;) probably with path module?

paulirish · 2019-10-11T23:55:05Z

lighthouse-core/config/config.js

      const rawPluginJson = require(pluginPath);
      const pluginJson = ConfigPlugin.parsePlugin(rawPluginJson, pluginName);

+      if (rawPluginJson['localePath']) {


naming bikeshed time?!!?!

first inclination is to call it localesPath (since theres a lot of files in there)

but i wonder if there's a more industry standard term for a directory full of i18n strings. @exterkamp ?

switched to localesPath for now

Now localeStrings due to updated config

paulirish · 2019-10-11T23:56:44Z

should be said:

The overall approach -- a plugin providing a relative path where LH can find a bunch of .json files with LHL messages -- SGTM. 👍

lighthouse-core/config/config.js

lighthouse-core/lib/i18n/i18n.js

paulirish · 2019-10-12T00:19:21Z

The overall approach -- a plugin providing a relative path where LH can find a bunch of .json files with LHL messages -- SGTM. 👍

on second thought... :)

@connorjclark points out something... in that model, LH has to readdir and blindly read everything everything off disk.

Perhaps a more explicit contract would be:

/** @type {LH.Config.Plugin} */
module.exports = {
  audits: [{
    path: 'lighthouse-plugin-example/audits/thing.js',
  }],

  localeStrings: {
    'en-US': require('./locales/en-US.json'),
    'es-419': require('./locales/es-419.json'),
  },

  //...

with this model, LH can validate the locale codes very directly. and not rely on filename as locale code.
and this'll also be more straightforward for browserifying.

connorjclark · 2019-10-12T00:25:27Z

+1 to above. it'd be nice to just have the config provide a path to the locale folder, but that would involve us telling browserify to explicitly bundle everything in those folder for syndicated plugins + expecting the files to be a certain filename (es.json vs es.lhl.json or w/e). Doing the whole thing in the config (like we do in locales.js) makes bundling easy.

We can still merge just the one locale when resolving the config. That's orthogonal to the above.

…cales. Only augment user's locale.

connorjclark · 2020-09-02T17:44:37Z

We are i18n'ing the report in DevTools soon (https://chromium-review.googlesource.com/c/devtools/devtools-frontend/+/2382176), so we should get this plugin translated.

We recently moved stack packs and its strings into its own npm package, and look into our own node_modules to pluck out the i18n string data. We could do the same here. @jburger424 would it be preferable to use our i18n pipeline, instead of you needing to duplicate the same work on your end?

Of course, plugins being able to provide their own locales is a worthy feature, but I think for now we don't need to go there.

jburger424 · 2020-09-08T18:07:26Z

We are i18n'ing the report in DevTools soon (chromium-review.googlesource.com/c/devtools/devtools-frontend/+/2382176), so we should get this plugin translated.

We recently moved stack packs and its strings into its own npm package, and look into our own node_modules to pluck out the i18n string data. We could do the same here. @jburger424 would it be preferable to use our i18n pipeline, instead of you needing to duplicate the same work on your end?

@connorjclark Yeah, sorry to have been sitting on this for so long. We actually have an existing pipeline, with translations that just need to be packaged, I should be able to cut a new release including those this week.

connorjclark · 2020-09-08T18:19:17Z

an existing pipeline

That's fine, we'll need to do this then. We can't prioritize this at the moment, so it's likely this plugin will initially be the only untranslated category within CDT.

connorjclark · 2020-09-08T18:33:04Z

To expand:

LH.Config.Plugin.localeStrings of type Record<string, LocaleJson>, where string is the locale code
augment locale data in config.js when resolving plugins
for CDT/LR bundling, we separate the locale json from the code. in CDT we lazily download the locale file for the language we need. So all data for a locale should be in one file, but this model introduces a file per plugin per locale. On bundling, we should drop LH.Config.Plugin.localeStrings. And we need a way to include the bundled plugin strings in the main locale files.
- Simplest way is to inject them into each locale in collect-strings. They get tracked in git.
- More complex would involve a new build step for locale files, doing the merging, and rolling from these new built locales in roll-to-devtools.sh (EDIT: @paulirish and I prefer this)
- (OR....) we figure out how to merge your TC project strings to LH'S TC project, and modify collect-strings.js to handle pubads too? Or just re-do the work in our TC project.

…9799.

jburger424 · 2020-10-27T23:19:12Z

Replacing with #11602

jburger424 added 2 commits October 7, 2019 16:23

Initial Commit

cfab63d

Add i18n/aliases.js

25019c0

jburger424 requested review from brendankenny, patrickhulce and paulirish as code owners October 7, 2019 21:13

googlebot added the cla: yes label Oct 7, 2019

vercel bot deployed to staging October 7, 2019 21:13 View deployment

Fix lint errors.

9a39989

Add TODO.

paulirish reviewed Oct 11, 2019

View reviewed changes

connorjclark reviewed Oct 12, 2019

View reviewed changes

lighthouse-core/config/config.js Outdated Show resolved Hide resolved

connorjclark reviewed Oct 12, 2019

View reviewed changes

lighthouse-core/lib/i18n/i18n.js Outdated Show resolved Hide resolved

Address minor comments.

ed790c4

vercel bot deployed to staging October 22, 2019 18:10 View deployment

Change to accept localeStrings instead of a path to a directory of lo…

3fab6ae

…cales. Only augment user's locale.

vercel bot deployed to staging October 24, 2019 18:37 View deployment

This was referenced Nov 6, 2019

i18n: Use log.verbose for outdated strings warning #9931

Merged

LH throws error when running plugin globally googleads/publisher-ads-lighthouse-plugin#159

Closed

connorjclark added the needs-discussion label Sep 2, 2020

connorjclark added the P2 label Sep 8, 2020

jburger424 added a commit to jburger424/lighthouse that referenced this pull request Oct 27, 2020

Support i18n for plugins. Much of this is migrated from GoogleChrome#…

9929964

…9799.

jburger424 mentioned this pull request Oct 27, 2020

i18n: support plugins locales #11602

Closed

jburger424 closed this Oct 27, 2020

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

WIP: i18n support for plugins #9799

WIP: i18n support for plugins #9799

jburger424 commented Oct 7, 2019 •

edited

paulirish Oct 11, 2019

paulirish Oct 11, 2019

paulirish Oct 11, 2019

jburger424 Oct 22, 2019

jburger424 Oct 24, 2019

paulirish commented Oct 11, 2019 •

edited

paulirish commented Oct 12, 2019 •

edited

connorjclark commented Oct 12, 2019 •

edited

connorjclark commented Sep 2, 2020 •

edited

jburger424 commented Sep 8, 2020

connorjclark commented Sep 8, 2020

connorjclark commented Sep 8, 2020 •

edited

jburger424 commented Oct 27, 2020

WIP: i18n support for plugins #9799

WIP: i18n support for plugins #9799

Conversation

jburger424 commented Oct 7, 2019 • edited

paulirish Oct 11, 2019

Choose a reason for hiding this comment

paulirish Oct 11, 2019

Choose a reason for hiding this comment

paulirish Oct 11, 2019

Choose a reason for hiding this comment

jburger424 Oct 22, 2019

Choose a reason for hiding this comment

jburger424 Oct 24, 2019

Choose a reason for hiding this comment

paulirish commented Oct 11, 2019 • edited

paulirish commented Oct 12, 2019 • edited

connorjclark commented Oct 12, 2019 • edited

connorjclark commented Sep 2, 2020 • edited

jburger424 commented Sep 8, 2020

connorjclark commented Sep 8, 2020

connorjclark commented Sep 8, 2020 • edited

jburger424 commented Oct 27, 2020

jburger424 commented Oct 7, 2019 •

edited

paulirish commented Oct 11, 2019 •

edited

paulirish commented Oct 12, 2019 •

edited

connorjclark commented Oct 12, 2019 •

edited

connorjclark commented Sep 2, 2020 •

edited

connorjclark commented Sep 8, 2020 •

edited