i18n: detect unused keys #1411
i18n: detect unused keys #1411
Conversation
\# Blocked by intlify/eslint-plugin-vue-i18n#643 Currently the ignore rules are _very_ general, and won't detect e.g. unused `back` keys mentioned in getodk/central#1350. This might be improved by something like intlify/eslint-plugin-vue-i18n#668. Closes getodk/central#829
matthew-white
left a comment
matthew-white
left a comment
There was a problem hiding this comment.
Just wanted to add some initial thoughts! As I wrote at getodk/central#829 (comment), I don't even know how we'd write our own check. The approach here seems promising to me. Even if it doesn't catch everything, it'd be much better than not checking at all.
| rules: { | ||
| '@intlify/vue-i18n/no-unused-keys': [ 'error', { | ||
| src: 'src/', | ||
| callExpression: '^(\\$t|t|\\$tc|tc|\\$tcn|tcn|\\$tn|tn)$', |
There was a problem hiding this comment.
Once #1026 is complete, there won't be a $tc() or tc() anymore. At that time, we'll probably rename $tcn() to $tn().
Currently, there is $tcn(), and there is tn(); there's no $tn() or tcn(). The asymmetry is because we've had $tcn() for a long time, while tn() was added around the time that we moved to Vue 3 and started using the Composition API. In the context of Vue I18n within the Composition API, tc() is deprecated, because t() now does what $t() and $tc() both used to do.
| asyncArrow: 'always' | ||
| }], | ||
| 'spaced-comment': 'off', | ||
| 'vue/attribute-hyphenation': 'off', |
There was a problem hiding this comment.
You wrote in the PR description:
This PR also seems to introduce a bunch of other new vue eslint rules, which are then disabled in this PR. That doesn't seem ideal; maybe they need to be evaluated and enabled separately.
I wonder if we need to update our version of eslint-plugin-vue. That's probably something I can do as part of #974.
Just based on their names, vue/require-prop-types and vue/no-constant-condition sound potentially useful.
One approach we could take is disable these rules for now, but file a follow-up issue to consider reenabling them.
| /^[\w-]+\.audit\.action\./, | ||
| /^[\w-]+\.audit\.category\./, | ||
| /^[\w-]+\.back\.back$/, | ||
| /^[\w-]+\.back\.title$/, | ||
| /^[\w-]+\.component\.WebFormRenderer\./, // dynamic modals | ||
| /^[\w-]+\.conflict\./, | ||
| /^[\w-]+\.fields\./, | ||
| /^[\w-]+\.oidc.error\./, | ||
| /^[\w-]+\.outdatedVersionHtml\./, // check that file's comments | ||
| /^[\w-]+\.reviewState\./, | ||
| /^[\w-]+\.steps\[0\]\.introduction\[1\]\[0\]$/, | ||
| /^[\w-]+\.tab\./, | ||
| /^[\w-]+\.title\.submissionBacklog\./, | ||
| /^[\w-]+\.title\.updateReviewState\./, | ||
| /^[\w-]+\.type\./, |
There was a problem hiding this comment.
You wrote in the PR description:
Currently the ignore rules are very general, and won't detect e.g. unused
backkeys mentioned in getodk/central#1350. This might be improved by something like intlify/eslint-plugin-vue-i18n#668.
The list of exceptions here is indeed longer than I'd expect. We probably wouldn't have to add to it super often though. I am a little concerned about false negatives though. It seems like the check will ignore a lot of messages. I guess checking most messages is much better than checking no messages though.
It makes sense to me that messages that are accessed via dynamic/constructed keys need to be in this list, e.g., /^[\w-]+\.audit\.action\./. I don't understand some of the other cases though, e.g., /^[\w-]+\.fields\./ or /Modal\.(body|title)$/. If we decide to run with this PR, maybe I can take a closer look at the list and try to figure out if there are ways to ignore fewer messages.
| rules: { | ||
| '@intlify/vue-i18n/no-unused-keys': [ 'error', { | ||
| src: 'src/', | ||
| callExpression: '^(\\$t|t|\\$tc|tc|\\$tcn|tcn|\\$tn|tn)$', |
There was a problem hiding this comment.
Re: being blocked by intlify/eslint-plugin-vue-i18n#643, I have two thoughts:
- Maybe we just ignore
$tcn()andtn()for now. They're not the most common way to access i18n messages. - What if in CI, we modified code files, e.g., replacing all strings containing
/\btn\(/witht(? Would the Vue I18n check work then?
2 participants
Blocked by
Currently the ignore rules are very general, and won't detect e.g. unused
backkeys mentioned in getodk/central#1350. This might be improved by something like intlify/eslint-plugin-vue-i18n#668.Closes getodk/central#829
What has been done to verify that this works as intended?
It doesn't right now.
Why is this the best possible solution? Were any other approaches considered?
An alternative would be to implement something ourselves reusing the code in
bin/transifex.This PR also seems to introduce a bunch of other new vue eslint rules, which are then disabled in this PR. That doesn't seem ideal; maybe they need to be evaluated and enabled separately.
How does this change affect users? Describe intentional changes to behavior and behavior that could have accidentally been affected by code changes. In other words, what are the regression risks?
Might reduce download sizes a tiny bit. Should also prevent wasted time by translators.
Does this change require updates to user documentation? If so, please file an issue here and include the link below.
No.
Before submitting this PR, please make sure you have:
npm run testandnpm run lintand confirmed all checks still pass OR confirm CircleCI build passes