GoogleChrome / GoogleChrome/lighthouse

refactor: share i18n formatting and locales between core and report

Open
#13,120 2 comments 1 reaction 1 assignee Claimed by @paulirish View on GitHub
i18n P3 report
Dominant language
JavaScript
Stars
30.8k
Forks
9.8k
Avg merge
1d 14h
Merged PRs (30d)
20

Description

tl;dr: our formatting code + `intl-messageformat` is relatively small. At the very least we can rearrange it to easily ship with `swap-locale` (#10148), but we should seriously consider shipping it with the main report as well.

This came up in https://github.com/GoogleChrome/lighthouse/pull/10148#discussion_r713454527 most recently, but also just came up in the context of the flow-report (https://github.com/GoogleChrome/lighthouse/pull/13034#discussion_r708637669). We've also had a few instances where we've wanted to format strings in the standalone report with replacement values and haven't been able to.

Starting with concrete plans and moving to more fanciful ideas:
- move formatting code to a new directory so it can be easily shared by core and report-adjacent code. This solves the `tsc` issues in https://github.com/GoogleChrome/lighthouse/pull/10148#discussion_r713454527, and makes the rolluping more straightforward: it's not reaching deep in core for files to bundle and it's clearer from file location that what's imported/required needs to be kept compatible across environments.
- I have a WIP branch for this here: https://github.com/GoogleChrome/lighthouse/compare/master...d232f8d
- The basic layout would be something like
```
lighthouse-core/lib/
- i18n.js (mostly `UIStrings` and `str_` functionality)

shared/localization/
- format.js (`isIcuMessage`, `getFormatted`, `replaceIcuMessages`, etc)
- swap-locale.js
- locales.js
- locales/*.json
```
- happy to iterate on any part of this. `shared/` is a terrible name so would be happy to get suggestions :)
- the possibility of a `shared/` came up before in the context of [where the `report-generator` should live](https://github.com/GoogleChrome/lighthouse/pull/12940#issue-974848549). It should probably move to `shared/` as well.
- where the i18 readme and the supporting scripts should live relative to all this is an open question. It is unfortunate that it gets more spread out
- ship #10148 using the separate directory. Will fix the tsc issues and some of the bundling should be simpler (e.g. no shimming `path` or `lighthouse-logger`)
- stripping the locale files from the bundle then [adding them back on demand](https://github.com/GoogleChrome/lighthouse/pull/10148/files#diff-bd16efc0be069424d1a6ea41b593a6f7fb859972765bd92958a8e2829a58429aR92) via `registerLocaleData` seems 👍
- Shrink the plain `format.js` bundle. The swap-locale bundle is already [down to 34KiB (10KiB after gzip)](https://github.com/GoogleChrome/lighthouse/pull/10148#discussion_r712583096) just dropping `path` and `lighthouse-logger`, should be smaller without the extra `str_`/UIStrings functionality that will be in a different file now, and will be even smaller with no swap-locale (which means no `_.get`/`_.set`, another 4+KiB each)
- Switching to esmodules means better tree shaking. Wouldn't ditch much code in `format.js`, but later `intl-messageformat` versions ship ESM builds. They are bigger overall, but that means they can be tree shaken, so it's possibly a win.
- if we do update, will have to deal with https://github.com/formatjs/formatjs/issues/1437. Apparently single quotes are supposed to escape curly braces in the ICU strings. We have two strings that have `'{replacement}'` and will have to be updated and/or dealt with so old LHRs aren't broken
- Ship with `flow-report`. No reason to artificially limit the text we want to show. Even if we only got it down to 25KiB (w/o gzip) that's tiny compared to a series of LHRs :)
- Ship with the standalone report for the same reason.
- Next-gen renderFormattedStrings that support replacement.

Contributor guide

Open the contributing guide

Assessment

This issue has not been assessed yet.

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.