fix(settings): order l10n bundling before static asset hashing - #21043
Conversation
| baseDir={baseDir} | ||
| userLocales={userLocales} | ||
| bundles={bundles} | ||
| reportBundleError={reportBundleError} |
There was a problem hiding this comment.
This is the only change in this file. The rest is prettier.
| const json = await mappingsResponse.json(); | ||
| return json; | ||
| if (!mappingsResponse.ok) { | ||
| throw new Error(`Received status ${mappingsResponse.status}`); |
There was a problem hiding this comment.
Would throwing here cause other issues? Failing to load ftl files should still load the english strings I would think.
| } | ||
|
|
||
| /** | ||
| * Reports a failure to load a localization bundle. Individual missing string ids |
There was a problem hiding this comment.
I wonder how useful this reporter is?
vbudhram
left a comment
vbudhram
left a comment
There was a problem hiding this comment.
I don't really have strong feelings for the reporting part. Maybe try to get some of the tests a bit more concise and remove overlap to get diff size down.
Because: - Since v1.342.0, production served English to every locale: the static asset manifest shipped with no main.ftl keys, so the app fetched no FTL. - Sibling dependsOn entries carry no ordering guarantee, so grunt hash-static was free to run before l10n-bundle wrote main.ftl. This commit: - Adds build-l10n to build-static's dependsOn. - Fails hash-static when the generated manifest is missing main.ftl for any locale present on disk. Closes #FXA-14361
|
pulled out reporting changes to #21044 |
There was a problem hiding this comment.
Pull request overview
Ensures localization bundles are generated before static asset hashing and prevents incomplete locale manifests.
Changes:
- Orders
build-l10nbeforebuild-static. - Validates every locale has a hashed
main.ftlmanifest entry.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
packages/fxa-settings/package.json |
Adds the required Nx build dependency. |
packages/fxa-settings/Gruntfile.js |
Adds post-hash localization manifest validation. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Because
main.ftlkeys, so the settings app fetched no FTL at all.build-l10n && build-staticchain with siblingdependsOnentries, which carry no ordering guarantee between each other, so grunthash-staticcould hashpublic/localesbeforel10n-bundlehad written into it.This pull request
build-l10ntobuild-static'sdependsOn, so grunthash-staticcan no longer run beforel10n-bundlehas writtenmain.ftl.hash-staticwhen the generated manifest is missingmain.ftlfor any locale present on disk.Issue that this pull request solves
Closes: FXA-14361
Checklist
Put an
xin the boxes that applyHow to review (Optional)
packages/fxa-settings/package.jsonholds the one-line change that actually fixes the outage;Gruntfile.jsholds the new guard.package.json→Gruntfile.js.public/localesis gitignored, so nx's hasher cannot see those files as inputs — which is why a stale cache can hide this locally.Screenshots (Optional)
Please attach the screenshots of the changes made in case of change in user interface.
Other information (Optional)
efdbd68f37.build-l10nahead ofbuild-static; a cold prime → merge → bundle → hash producesmain.ftlmanifest entries for all 88 locales; and the guard fails with a precise message when a locale is missing one.