-
Notifications
You must be signed in to change notification settings - Fork 47
front: extract i18n keys with TypeScript #11554
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: dev
Are you sure you want to change the base?
Conversation
cc66535
to
3913537
Compare
3913537
to
8fdc600
Compare
0758246
to
a20acb6
Compare
For what it's worth, this has been extracted into https://github.com/emersion/i18next-typescript-parser |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Great work! Reviewed all commits but the script one. I still need some time to completely understand that one ^^''.
|
||
import { glob } from 'glob'; | ||
// TODO : remove eslint-disable once https://github.com/i18next/i18next-parser/issues/1000 gets fixed | ||
// eslint-disable-next-line import/no-unresolved | ||
import * as I18nextParser from 'i18next-parser'; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should we drop i18next-parser from package.json?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good point! Done.
front/scripts/i18n-checker.ts
Outdated
import vfs from 'vinyl-fs'; | ||
import * as ts from 'typescript'; | ||
|
||
const LANGUAGES = ['en', 'fr']; | ||
|
||
const IGNORE_MISSING: RegExp[] = [ | ||
// key used by a t function in modules/trainschedule/components/ManageTrainSchedule/helpers/checkCurrentConfig.ts | ||
/translation:errorMessages\..*/, | ||
/translation:error/, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I saw you removed t('error') at the profit of t('default') in one commit, is this still used somewhere else?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Indeed, this can be dropped.
front/scripts/i18n-checker.ts
Outdated
/translation:unspecified/, | ||
// key used by upsertMapWaypointsInOperationalPoints | ||
/translation:requestedPoint/, | ||
// key used by checkStdcmConfigErrors |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should we remove this comment?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yup, done!
Ensures our i18n-checker script can recognize that translations accessed from inside this function are in use. Signed-off-by: Simon Ser <[email protected]>
Otherwise translations aren't live-updated when switching the language. Signed-off-by: Simon Ser <[email protected]>
We want to grab i18next/react-i18next#1842 Signed-off-by: Simon Ser <[email protected]>
We don't need to interpolate variables here. Makes it easier to statically extract the constant string in the i18n checker script. Signed-off-by: Simon Ser <[email protected]>
Helps with static analysis in the i18next checker script. Signed-off-by: Simon Ser <[email protected]>
'error' is not a valid translation key, it doesn't exist. Use 'default' just like getErrorMessage(), which exists. Signed-off-by: Simon Ser <[email protected]>
We don't need to pass multiple of these. Signed-off-by: Simon Ser <[email protected]>
i18next-parser uses the TypeScript compiler only for lexing. In other words, it has no contextual information about the translation calls. This makes extracting translation keys more fragile and error-prone. For instance, when TFunctions are passed around, the namespace is lost. As another example, "i18n.t" is not recognized as a translation function. In general, this is a fool's errand: operating on tokens without type information will always result in a lot of unhandled corner cases. Instead, use the TypeScript compiler API to extract type information and iterate over all calls to the translation function. Signed-off-by: Simon Ser <[email protected]>
These reference non-existing keys or are unnecessary. Signed-off-by: Simon Ser <[email protected]>
Signed-off-by: Simon Ser <[email protected]>
This is a hook, we can just call useTranslation() there. Signed-off-by: Simon Ser <[email protected]>
…tionalPoints() Signed-off-by: Simon Ser <[email protected]>
Ensure we don't have unused keys in translation files, to reduce the workfload for community translators and keep our files neat and tidy. Signed-off-by: Simon Ser <[email protected]>
a20acb6
to
c89a934
Compare
i18next-parser uses the TypeScript compiler only for lexing. In
other words, it has no contextual information about the translation
calls. This makes extracting translation keys more fragile and
error-prone. For instance, when TFunctions are passed around, the
namespace is lost. As another example, "i18n.t" is not recognized
as a translation function. In general, this is a fool's errand:
operating on tokens without type information will always result in
a lot of unhandled corner cases.
Instead, use the TypeScript compiler API to extract type information
and iterate over all calls to the translation function.
(This should probably be extracted into a re-usable NPM package.)
See individual commits.
Typescript compiler API debugging notes