From 6c64d58528fb65cfc12eb2b95fc1b01d161af8cf Mon Sep 17 00:00:00 2001 From: Valerie Pomerleau Date: Tue, 29 Nov 2022 16:09:33 -0800 Subject: [PATCH] refactor(l10n): Ensure shared FTL strings not duplicated on Pontoon Because: - Concatenating shared FTL strings into multiple packages before sending for localization created duplication of localization work, and we want each string to only be translated once. This commit: - Remove concatenation of shared files (branding.ftl, fxa-react ftl files) from grunttasks - Add grunttasks in fxa-react to concatenate fxa-react FTL files into one react.ftl file - Add grunt to fxa-react dev dependencies - Update fxa-react scripts to run l10n and ftl tasks - Update clone-l10n script to distribute react.ftl to packages - Bundle branding and shared into AppLocalizationProvider Closes #FXA-6388 --- .gitignore | 4 ++ _scripts/clone-l10n.sh | 10 ++++- packages/fxa-auth-server/grunttasks/ftl.js | 8 +--- .../.storybook/components/MockApp.tsx | 2 +- packages/fxa-payments-server/Gruntfile.js | 1 - packages/fxa-payments-server/src/App.tsx | 2 +- packages/fxa-react/Gruntfile.js | 42 +++++++++++++++++++ .../fxa-react/lib/AppLocalizationProvider.tsx | 2 +- packages/fxa-react/lib/test-utils/index.ts | 28 ++++++++++++- packages/fxa-react/package.json | 14 +++++-- packages/fxa-settings/Gruntfile.js | 6 +-- packages/fxa-settings/src/index.tsx | 2 +- yarn.lock | 4 ++ 13 files changed, 104 insertions(+), 21 deletions(-) create mode 100644 packages/fxa-react/Gruntfile.js diff --git a/.gitignore b/.gitignore index 7ee1dbf8bf..5ec2255b03 100644 --- a/.gitignore +++ b/.gitignore @@ -132,8 +132,12 @@ packages/fxa-profile-server/BUCKET_NAME # fxa-react packages/fxa-react/**/*.js +!packages/fxa-react/**/gruntfile.js packages/fxa-react/**/*.d.ts !packages/fxa-react/pm2.config.js +packages/fxa-react/fxa-content-server-l10n/ +packages/fxa-react/public/locales +packages/fxa-react/test/ # fxa-settings packages/fxa-settings/fxa-content-server-l10n/ diff --git a/_scripts/clone-l10n.sh b/_scripts/clone-l10n.sh index 5c54191965..6e9d82e476 100755 --- a/_scripts/clone-l10n.sh +++ b/_scripts/clone-l10n.sh @@ -131,16 +131,24 @@ copy_ftl() { PAYMENTS="fxa-payments-server" SETTINGS="fxa-settings" AUTH="fxa-auth-server" +REACT="fxa-react" -# Copy .ftl files for payments, settings, and auth (emails) +# Copy .ftl files for payments, settings, auth (emails), +# and react (shared react components) case "$MODULE" in "$PAYMENTS") copy_ftl "payments" + copy_ftl "react" ;; "$SETTINGS") copy_ftl "settings" + copy_ftl "react" ;; "$AUTH") copy_ftl "auth" + copy_ftl "branding" + ;; + "$REACT") + copy_ftl "react" ;; esac diff --git a/packages/fxa-auth-server/grunttasks/ftl.js b/packages/fxa-auth-server/grunttasks/ftl.js index 78d8ea8ba0..893e8af14a 100644 --- a/packages/fxa-auth-server/grunttasks/ftl.js +++ b/packages/fxa-auth-server/grunttasks/ftl.js @@ -7,11 +7,7 @@ module.exports = function (grunt) { grunt.config('concat', { ftl: { - src: [ - '../fxa-shared/l10n/branding.ftl', - 'lib/l10n/server.ftl', - 'lib/**/senders/emails/**/en.ftl', - ], + src: ['lib/l10n/server.ftl', 'lib/**/senders/emails/**/en.ftl'], dest: 'public/locales/en/auth.ftl', }, 'ftl-test': { @@ -25,7 +21,7 @@ module.exports = function (grunt) { grunt.config('watch', { ftl: { - files: ['lib/l10n/branding.ftl', 'lib/l10n/server.ftl', 'lib/**/en.ftl'], + files: ['lib/l10n/server.ftl', 'lib/**/en.ftl'], tasks: ['merge-ftl'], options: { interrupt: true, diff --git a/packages/fxa-payments-server/.storybook/components/MockApp.tsx b/packages/fxa-payments-server/.storybook/components/MockApp.tsx index 6e1c175704..02c2b9ee16 100644 --- a/packages/fxa-payments-server/.storybook/components/MockApp.tsx +++ b/packages/fxa-payments-server/.storybook/components/MockApp.tsx @@ -95,7 +95,7 @@ export const MockApp = ({ diff --git a/packages/fxa-payments-server/Gruntfile.js b/packages/fxa-payments-server/Gruntfile.js index d5bc6994eb..46dabf452e 100644 --- a/packages/fxa-payments-server/Gruntfile.js +++ b/packages/fxa-payments-server/Gruntfile.js @@ -12,7 +12,6 @@ module.exports = function (grunt) { // conflicting IDs for identical terms. 'src/branding.ftl', 'src/**/*.ftl', - '../fxa-react/components/**/*.ftl', ]; grunt.initConfig({ diff --git a/packages/fxa-payments-server/src/App.tsx b/packages/fxa-payments-server/src/App.tsx index 059a21a110..1fb402c1c0 100644 --- a/packages/fxa-payments-server/src/App.tsx +++ b/packages/fxa-payments-server/src/App.tsx @@ -103,7 +103,7 @@ export const App = ({ diff --git a/packages/fxa-react/Gruntfile.js b/packages/fxa-react/Gruntfile.js new file mode 100644 index 0000000000..b954eb7d90 --- /dev/null +++ b/packages/fxa-react/Gruntfile.js @@ -0,0 +1,42 @@ +/* This Source Code Form is subject to the terms of the Mozilla Public + * License, v. 2.0. If a copy of the MPL was not distributed with this + * file, You can obtain one at http://mozilla.org/MPL/2.0/. */ + +'use strict'; + +module.exports = function (grunt) { + grunt.initConfig({ + pkg: grunt.file.readJSON('package.json'), + concat: { + ftl: { + src: 'components/**/*.ftl', + dest: 'public/locales/en/react.ftl', + }, + + // We need this for tests because we pull the latest from `fxa-content-server-l10n` + // and place those in our `public` directory at `postinstall` time, and sometimes we have + // FTL updates on our side that haven't landed yet on the l10n side. We want to test + // against _our_ latest, and not necessarily the l10n repo's latest. + 'ftl-test': { + src: 'components/**/*.ftl', + dest: 'test/react.ftl', + }, + }, + watch: { + ftl: { + files: 'components/**/*.ftl', + tasks: ['merge-ftl'], + options: { + interrupt: true, + }, + }, + }, + }); + + grunt.loadNpmTasks('grunt-contrib-watch'); + grunt.loadNpmTasks('grunt-contrib-concat'); + + grunt.registerTask('merge-ftl', ['concat:ftl']); + grunt.registerTask('merge-ftl:test', ['concat:ftl-test']); + grunt.registerTask('watch-ftl', ['watch:ftl']); +}; diff --git a/packages/fxa-react/lib/AppLocalizationProvider.tsx b/packages/fxa-react/lib/AppLocalizationProvider.tsx index fe4a6567fb..a8a6dfd01a 100644 --- a/packages/fxa-react/lib/AppLocalizationProvider.tsx +++ b/packages/fxa-react/lib/AppLocalizationProvider.tsx @@ -76,7 +76,7 @@ type Props = { export default class AppLocalizationProvider extends Component { static defaultProps: Props = { baseDir: '/locales', - userLocales: ['en-US'], + userLocales: ['en'], bundles: ['main'], children: React.createElement('div'), }; diff --git a/packages/fxa-react/lib/test-utils/index.ts b/packages/fxa-react/lib/test-utils/index.ts index 1c45851ba4..1601e5f6be 100644 --- a/packages/fxa-react/lib/test-utils/index.ts +++ b/packages/fxa-react/lib/test-utils/index.ts @@ -8,7 +8,7 @@ import fs from 'fs'; import path from 'path'; import { checkMessage } from '../utils'; -type PackageName = 'settings' | 'payments' | null; +type PackageName = 'settings' | 'payments' | 'react' | null; function getFtlPath(packageName: string | null, locale: string) { let ftlPath: string; @@ -66,6 +66,32 @@ function getFtlPath(packageName: string | null, locale: string) { ); } break; + case 'react': + if (locale === 'en') { + ftlPath = path.join( + __dirname, + '..', + '..', + '..', + 'fxa-react', + 'test', + 'react.ftl' + ); + } else { + // TODO: Not currently used, but probably want to add one translation test + ftlPath = path.join( + __dirname, + '..', + '..', + '..', + 'fxa-react', + 'public', + 'locales', + locale, + 'react.ftl' + ); + } + break; default: ftlPath = path.join(__dirname, 'test.ftl'); break; diff --git a/packages/fxa-react/package.json b/packages/fxa-react/package.json index c4d639d9b5..e09ff09976 100644 --- a/packages/fxa-react/package.json +++ b/packages/fxa-react/package.json @@ -10,18 +10,22 @@ "./lib/": "./dist/lib/" }, "scripts": { + "postinstall": "grunt merge-ftl &&../../_scripts/clone-l10n.sh fxa-react", "build-css": "tailwindcss -i ./styles/tailwind.css -o ./styles/tailwind.out.css", "build-storybook": "NODE_ENV=production npm run build-css && build-storybook", - "build": "tsc --build", + "build": "tsc --build && npm run merge-ftl", "clean": "rimraf dist", "lint": "eslint . .storybook", "format": "prettier --write --config ../../_dev/.prettierrc '**'", "restart": "pm2 restart pm2.config.js", - "start": "pm2 start pm2.config.js", + "start": "grunt merge-ftl && pm2 start pm2.config.js", "stop": "pm2 stop pm2.config.js", "delete": "pm2 delete pm2.config.js", "storybook": "npm run build-css && start-storybook -p 6007 --no-version-updates", - "test": "jest --runInBand --env=jest-environment-jsdom" + "test": "yarn merge-ftl:test && jest --runInBand --env=jest-environment-jsdom", + "merge-ftl": "grunt merge-ftl", + "merge-ftl:test": "grunt merge-ftl:test", + "watch-ftl": "grunt watch-ftl" }, "dependencies": { "@fluent/bundle": "^0.17.1", @@ -69,6 +73,10 @@ "camelcase": "^6.3.0", "eslint": "^7.32.0", "file-loader": "^4.3.0", + "grunt": "^1.5.3", + "grunt-cli": "^1.4.3", + "grunt-contrib-concat": "^2.1.0", + "grunt-contrib-watch": "^1.1.0", "identity-obj-proxy": "^3.0.0", "jest": "27.5.1", "jest-environment-jsdom": "^27.5.1", diff --git a/packages/fxa-settings/Gruntfile.js b/packages/fxa-settings/Gruntfile.js index 77ec752976..bc9b0fea03 100644 --- a/packages/fxa-settings/Gruntfile.js +++ b/packages/fxa-settings/Gruntfile.js @@ -5,11 +5,7 @@ 'use strict'; module.exports = function (grunt) { - const srcPaths = [ - '.license.header', - 'src/**/*.ftl', - '../fxa-react/components/**/*.ftl', - ]; + const srcPaths = ['.license.header', 'src/**/*.ftl']; grunt.initConfig({ pkg: grunt.file.readJSON('package.json'), diff --git a/packages/fxa-settings/src/index.tsx b/packages/fxa-settings/src/index.tsx index 786c512e17..ad1b39fa36 100644 --- a/packages/fxa-settings/src/index.tsx +++ b/packages/fxa-settings/src/index.tsx @@ -61,7 +61,7 @@ try { {showNewReactApp ? ( diff --git a/yarn.lock b/yarn.lock index eb17e2def3..786ff3cefe 100644 --- a/yarn.lock +++ b/yarn.lock @@ -25645,6 +25645,10 @@ fsevents@~2.1.1: fetch-mock: ^9.11.0 file-loader: ^4.3.0 fxa-shared: "workspace:*" + grunt: ^1.5.3 + grunt-cli: ^1.4.3 + grunt-contrib-concat: ^2.1.0 + grunt-contrib-watch: ^1.1.0 identity-obj-proxy: ^3.0.0 jest: 27.5.1 jest-environment-jsdom: ^27.5.1