Replace fbt with formatjs (react-intl), which is easier to use in the context of a React App.
Details
Details
- Reviewers
majcosta - Group Reviewers
Restricted Project
Run the following commands:
npm install
npm run test
npm start
Then, check if texts are in english.
Switch language/locale to portuguese (Brazil). Restart the server and check if texts are in portuguese.
Switch to a different language that is not supported (not english or portuguese) and check if texts are in english (default).
Diff Detail
Diff Detail
- Repository
- rABC Bitcoin ABC
- Branch
- cashtab-switch-to-react-intl
- Lint
Lint Passed - Unit
No Test Coverage - Build Status
Buildable 16202 Build 32273: Build Diff Build 32272: arc lint + arc unit
Event Timeline
Comment Actions
- extension/src/components/App.js still has fbt imports and tags
- .gitignore still has an fbt related file section
- test plan doesn't test all changes
| web/cashtab/package.json | ||
|---|---|---|
| 173 ↗ | (On Diff #29122) | missed this guy |
Comment Actions
Build Bitcoin ABC Diffs / Diff Testing (cashtab-tests) failed.
Tail of the build log:
/work/web/cashtab /work/abc-ci-builds/cashtab-tests added 2816 packages, and audited 2817 packages in 43s 151 packages are looking for funding run `npm fund` for details 14 vulnerabilities (13 moderate, 1 high) To address issues that do not require attention, run: npm audit fix To address all issues (including breaking changes), run: npm audit fix --force Run `npm audit` for details. npm notice npm notice New minor version of npm available! 7.7.6 -> 7.20.0 npm notice Changelog: <https://github.com/npm/cli/releases/tag/v7.20.0> npm notice Run `npm install -g npm@7.20.0` to update! npm notice > cashtab@1.0.0 prebuild > npm run compile > cashtab@1.0.0 compile > formatjs compile-folder --ast --format transifex lang src/compiled-lang > cashtab@1.0.0 build > node scripts/build.js Creating an optimized production build... Failed to compile. ./src/components/App.js Cannot find module: '@assets/popout.svg'. Make sure this package is installed. You can install this package by running: npm install @assets/popout.svg. Build cashtab-tests failed with exit code 1
Comment Actions
Build Bitcoin ABC Diffs / Diff Testing (cashtab-tests) failed.
Tail of the build log:
/work/web/cashtab /work/abc-ci-builds/cashtab-tests added 2816 packages, and audited 2817 packages in 42s 151 packages are looking for funding run `npm fund` for details 14 vulnerabilities (13 moderate, 1 high) To address issues that do not require attention, run: npm audit fix To address all issues (including breaking changes), run: npm audit fix --force Run `npm audit` for details. npm notice npm notice New minor version of npm available! 7.7.6 -> 7.20.0 npm notice Changelog: <https://github.com/npm/cli/releases/tag/v7.20.0> npm notice Run `npm install -g npm@7.20.0` to update! npm notice > cashtab@1.0.0 prebuild > npm run compile > cashtab@1.0.0 compile > formatjs compile-folder --ast --format transifex lang src/compiled-lang > cashtab@1.0.0 build > node scripts/build.js Creating an optimized production build... Failed to compile. ./src/components/App.js Cannot find module: 'fbt'. Make sure this package is installed. You can install this package by running: npm install fbt. Build cashtab-tests failed with exit code 1
Comment Actions
Looking at react-intl, it seems to me like it is a significant step backward.
The first obvious one is the absurd number of dependencies. But more importantly, it seems to break composability:
<fbt desc="foobar">
You can read more about this very interesting topic on <a href="https://john.blog/">John's blog</a> or by consulting the <a href="https://foobar.archive/">foobar archive</a>.
</fbt>Now that would seem like you could splice and dice this, but you really can't because you don't know if the structure of the sentence will look the same in another language.
There are also no support for genders.
I'm arguably no specialist and may have missed some stuff, but as far as I can see, react-intl did happily jumped into all the traps most intl framework happily jump into and learned nothing from it.
Comment Actions
Please apply changes to the non-extension version of App.js as well
Regarding fbt vs react-intl -- fbt is probably better. However, fbt is creating a number of dev ops issues (npm start throwing warnings, errors).
I think the best approach would be
- Remove all localization and all scripts related to localization
- Upgrade to webpack 5
- Evaluate and implement best localization software
However, I think it's okay to drop in replace fbt with react-intl for now (since it's already done in this diff). If this solves the fbt issues, it's a step in the right direction. However, I think we will still need to remove all localization and make all of the build and dev scripts as simple as possible before upgrading to webpack 5.
Comment Actions
Without specifying what these issues are, there is no way to know it is a step in the right direction.
Comment Actions
Good point. @alcipir please abandon this diff. We should start with no localization to simplify the dependency tree for migration to webpack 5.