Skip to content

MPDX-9934 - Ensure {{ name }} and not {name} in <Trans> #1977

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.

Sign up for GitHub

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

Open
zweatshirt wants to merge 2 commits into
base: main
Choose a base branch
from
Open

MPDX-9934 - Ensure {{ name }} and not {name} in <Trans> #1977

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
26249cd
Add ESLint rules for <Trans> component to enforce t prop and prevent …
zweatshirt Aug 11, 2026
944a6da
Refactor Trans component usage to include translation function for im…
zweatshirt Aug 11, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
21 changes: 21 additions & 0 deletions .eslintrc.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -64,6 +64,27 @@ module.exports = {
ignoreMemberSort: false,
},
],
'no-restricted-syntax': [

@canac canac Aug 12, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I love that we can do this with the ESLint selectors instead of needing to write a custom code-based rule!

'error',
{
selector:
"JSXOpeningElement[name.name='Trans']:not(:has(JSXAttribute[name.name='t']))",
message:
'<Trans> must be passed a t={t} prop from useTranslation() so it resolves keys against the component i18n instance.',
},
{
/* i18next interpolates {{ name }}, not {name}. A bare {name} child is a
* plain value at runtime, so it lands in the extracted key and the
* lookup can never match.
* Includes Trans and children of Trans, and determines if the descendent matches an
* Identifier (e.g. {name}), MemberExpression (e.g. {user.name}), or CallExpression (e.g. {getName()})
*/
selector:
":matches(JSXElement[openingElement.name.name='Trans'], JSXElement[openingElement.name.name='Trans'] JSXElement) > JSXExpressionContainer > :matches(Identifier, MemberExpression, CallExpression)",

@zweatshirt zweatshirt Aug 12, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[Medium] **This selector has 2 proven false positives and 9 proven false-negative classes.** Nine separate agent findings consolidated here — they share one root cause: esquery cannot distinguish JSX *children* from *attribute* subtrees, so the descendant combinator walks into `components={{...}}`.

All reproduced against this exact config with the real ESLint 8.57.0 + @typescript-eslint/parser:

probe result
components={{ bold: <strong>{count}</strong> }} wrongly flagged
<Trans {...props}> (spread supplies t) wrongly flagged (rule 1)
{user?.name} — optional chaining missed (ChainExpression breaks the > match)
{`hi ${n}`}, {n + '!'}, {c ? a : b}, {[n]} missed ×4
{n as string}, {n!} missed ×2
<Trans t={t}><>{name}</></Trans> missed (JSXFragment is not JSXElement)
t only on a nested element inside components missed (rule 1's :has() is a descendant match)
controls: {name} / {' '} flagged / clean ✓

The components false positive is not latent in practice: no-restricted-syntax is never autofixable and .husky/pre-commit runs lint-staged → eslint --cache --fix, so it becomes a hard commit block with no autofix path — and it lands squarely on components, the escape hatch this PR itself adopts in getLimitedText.tsx. Optional chaining is the worst miss, since ?. is pervasive here and <Trans t={t}>Hello {contact?.name}</Trans> is exactly the bug this rule exists to prevent.

Verified drop-in replacement — passes all 16 probes and produces 0 violations across */**/*.{js,ts,tsx}, so the PR's headline invariant survives:

selector:
  ":matches(JSXElement[openingElement.name.name='Trans'], JSXElement[openingElement.name.name='Trans'] JSXElement:not(JSXAttribute JSXElement), JSXElement[openingElement.name.name='Trans'] JSXFragment) > JSXExpressionContainer > :not(Literal, JSXEmptyExpression, ObjectExpression, JSXElement, JSXFragment, LogicalExpression)",

Three changes: :not(JSXAttribute JSXElement) excludes components subtrees; a JSXFragment branch closes the fragment escape; allowlist→denylist closes the seven missed node types at once. Literal must stay excluded to preserve the {' '} separator used in 30+ files; ObjectExpression preserves correct {{ name }}; LogicalExpression preserves {cond && <B/>}.

Rule 1 (line 70-71) is not fixable in esquery — :has(> JSXAttribute[name.name='t']) is a hard syntax error (no leading combinator inside :has()). That check needs a custom ESLint rule, or the two bad shapes need to stay on the manual review checklist.

message:
'Single-brace {name} inside <Trans> becomes part of the extracted key, so the lookup never matches. Use {{ name }} which only typechecks as a direct child of <Trans>, so if this sits inside a nested element, move that element outside the <Trans> instead.',

@canac canac Aug 12, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Use {{ name }} which only typechecks as a direct child of

From my research, upgrading our i18n libraries might fix this restriction.

@zweatshirt zweatshirt Aug 12, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[Medium] **This message steers authors toward the one `` form that can silently fail — and the blocker in this PR is proof.**

"…so if this sits inside a nested element, move that element outside the <Trans> instead" mandates children-based <Trans>, whose key is reconstructed twice — once by i18next-parser at extract time, once by react-i18next at render time. Whenever a propless br/strong/i/p child is present those two disagree (<N></N> vs <tag/>), and the key becomes permanently unresolvable. That is exactly what happened at finish.page.tsx:46, and there are 7 such sites repo-wide.

The defaults="…" + components={{…}} form this same PR adopts in getLimitedText.tsx has no such failure mode: the author writes one string that both the parser and the runtime consume verbatim, so no two-serializer agreement is required. Confirming evidence from a fresh extract: the only keys carrying literal <i>/<p>/<strong> markup are authored-string keys, and all 7 unmatchable-key sites are children-based.

Two suggestions, in order of value:

  1. Lead with the safe form in this message — defaults= + values= + components= handles interpolation inside nested markup without DOM surgery. AboutForm.tsx:93-112 in this PR had to invert <Trans>/<Box> nesting and hoist a <Box> out just to satisfy the current advice, when a props-only fix existed.
  2. Add a third selector banning propless br/strong/i/p as <Trans> children. That class is mechanically checkable, it is only 7 sites to migrate, and it would have caught this PR's blocker at lint time.

},
],

@zweatshirt zweatshirt Aug 12, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[Medium] **The single-brace ban is now a hard CI error documented nowhere.** `CLAUDE.md` § Localization never mentions `` at all, and `.claude/rules/code-review.md` records only the `t`-prop item, not this one. The ESLint message is currently the sole documentation — and per the two comments above, its stated guarantees diverge from its actual behavior on 11 proven counts, so it cannot carry that job alone.

Suggested additions:

CLAUDE.md § Localization — use t() with interpolation by default; use <Trans> only when inline elements matter; inside <Trans> interpolate as {{ name }} and only as a direct child; prefer defaults= + values= over children; never put a propless basic HTML node (br/strong/i/p) in <Trans> children.

.claude/rules/code-review.md § Localization — annotate the existing t-prop item as machine-enforced, with the two shapes still needing a manual check (<Trans {...props}> false positive, and t supplied only on a nested element inside components — the latter unfixable in esquery). Add the single-brace item with its known misses, and add the propless-basic-HTML rule.

curly: 'error',
eqeqeq: 'error',
'no-console': 'error',
Expand Down
96 changes: 96 additions & 0 deletions __tests__/eslintrules/eslintTransRules.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,96 @@
import { Linter } from 'eslint';
import eslintConfig from '../../.eslintrc';

interface RestrictedSyntaxOption {
selector: string;
message: string;
}

// Assert against the real rules so these tests survive wording changes and fail
// if a selector is narrowed.
const restrictedSyntax = eslintConfig.rules['no-restricted-syntax'] as [

@zweatshirt zweatshirt Aug 12, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[Medium] **This cast hard-codes exactly two options, and TypeScript never length-checks it.** Add a third `no-restricted-syntax` entry and you get no compile error and no test failure — the new rule is active in `verify()` but never asserted on. Insert one *before* the existing two and the positional destructure on line 16 silently mis-binds, so the tests would assert the wrong messages while still passing. (Reordering the current two *is* caught, since the expected messages are read from the config.)

Bind by selector text instead of index, and pin the count:

const restrictedSyntax = eslintConfig.rules['no-restricted-syntax'] as [
  'error',
  ...RestrictedSyntaxOption[],
];
const options = restrictedSyntax.slice(1) as RestrictedSyntaxOption[];

expect(options).toHaveLength(2); // a new rule must add its own coverage

const missingTRule = options.find((option) =>
  option.selector.includes("JSXAttribute[name.name='t']"),
)!;
const singleBraceRule = options.find((option) =>
  option.selector.includes('JSXExpressionContainer'),
)!;

Reading the rules from the live config was the right call — it survives message rewording. This just closes the one hole in that approach.

'error',
RestrictedSyntaxOption,
RestrictedSyntaxOption,
];
const [, missingTRule, singleBraceRule] = restrictedSyntax;

const linter = new Linter();

@zweatshirt zweatshirt Aug 12, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[Suggestion] Pin the config type: `new Linter({ configType: 'eslintrc' })`.

This is correct today — the repo is on ESLint 8.57.0 (package.json:123, yarn.lock:10250), where new Linter() defaults to configType: 'eslintrc' and the { parserOptions, rules } shape passed to verify() is right. On ESLint 9 the default flips to flat config and this exact call shape stops working. Being explicit makes the upgrade fail loudly instead of subtly.

(For the record: import eslintConfig from '../../.eslintrc' resolves correctly for both Jest and TypeScript — moduleFileExtensions includes js, plus allowJs: true, moduleResolution: "bundler", and esModuleInterop: true. Verified with tsc --traceResolution; no implicit any.)


const lintTrans = (body: string): string[] =>
linter
.verify(`export const Probe = ({ name, url, t }) => (\n ${body}\n);`, {
parserOptions: {
ecmaVersion: 2020,
sourceType: 'module',
ecmaFeatures: { jsx: true },
},
rules: { 'no-restricted-syntax': restrictedSyntax },
})
.filter((message) => message.ruleId === 'no-restricted-syntax')

@zweatshirt zweatshirt Aug 12, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[Medium] **This filter drops fatal parse errors, so all five negative tests can pass vacuously — and the harness lints a different language than CI does.** Two defects with one fix.

1. Parse errors are swallowed. ESLint reports them with ruleId: null, which this filter discards, so any probe that fails to parse yields [] — exactly what the toEqual([]) tests assert. Proven with a deliberate syntax error:

probe: '<Trans t={t}>oops</Tran'
  ALL msgs:  [{"ruleId":null,"fatal":true,"msg":"Parsing error: Unexpected token )"}]
  after filter: []

So accepts a <Trans> that is passed t, accepts double-brace interpolation, accepts a string literal child…, accepts an expression in a nested element attribute, and does not flag nested double-brace interpolation are all one typo away from green-but-meaningless.

2. Wrong parser. .eslintrc.js:3 sets parser: '@typescript-eslint/parser'; verify() here passes none, so it validates against espree's AST instead. Combined with defect 1 this is a blind spot the harness structurally cannot see:

plain {name}                  | espree: FLAG        | tsParser: FLAG
TS as-cast {name as string}   | espree: PARSE_ERR→[] | tsParser: [] (real miss)
TS non-null {name!}           | espree: PARSE_ERR→[] | tsParser: [] (real miss)

Fix:

const linter = new Linter({ configType: 'eslintrc' });
linter.defineParser('ts', require('@typescript-eslint/parser'));

const lintTrans = (body: string): string[] => {
  const messages = linter.verify(
    `export const Probe = ({ name, url, t }) => (\n  ${body}\n);`,
    {
      parser: 'ts',
      parserOptions: {
        ecmaVersion: 2020,
        sourceType: 'module',
        ecmaFeatures: { jsx: true },
      },
      rules: { 'no-restricted-syntax': restrictedSyntax },
    },
  );

  // A parse error reports ruleId: null, which the filter below would drop,
  // turning every toEqual([]) expectation into a false pass.
  const fatal = messages.find((message) => message.fatal);
  if (fatal) {
    throw new Error(`Probe failed to parse: ${fatal.message}`);
  }

  return messages
    .filter((message) => message.ruleId === 'no-restricted-syntax')
    .map((message) => message.message);
};

Untested branches worth adding (each verified against the real config): optional chaining {user?.name}, template literal, conditional expression, MemberExpression (claimed in the config comment, never asserted), spread-only attributes, and the components-prop case from the .eslintrc.js:83 comment. If you widen the selector as suggested there, the first four flip from missed to flagged.

.map((message) => message.message);

describe('<Trans> no-restricted-syntax rules', () => {
it('flags a <Trans> with no t prop', () => {
expect(lintTrans('<Trans>All set</Trans>')).toEqual([missingTRule.message]);
});

it('flags a self-closing <Trans> with no t prop', () => {
expect(
lintTrans('<Trans defaults="Hello {{name}}" values={{ name }} />'),
).toEqual([missingTRule.message]);
});

it('accepts a <Trans> that is passed t', () => {
expect(lintTrans('<Trans t={t}>All set</Trans>')).toEqual([]);
});

it('flags a single-brace interpolation', () => {
expect(lintTrans('<Trans t={t}>Hello {name}</Trans>')).toEqual([
singleBraceRule.message,
]);
});

it('flags a single-brace interpolation nested inside a child element', () => {
expect(lintTrans('<Trans t={t}><Box>Hello {name}</Box></Trans>')).toEqual([
singleBraceRule.message,
]);
});

it('flags a t() call used as a child', () => {
expect(lintTrans(`<Trans t={t}>{t('Hello')}</Trans>`)).toEqual([
singleBraceRule.message,
]);
});

it('accepts double-brace interpolation', () => {
expect(
lintTrans('<Trans t={t} values={{ name }}>Hello {{ name }}</Trans>'),
).toEqual([]);
});

/*
* react-i18next supports nested JSX/interpolation: nodesToString recursively
* serializes nested nodes, and the Trans rendering path resolves interpolation
* values recursively. The JSX form {{ name }} is different: it is parsed as an
* object literal ({ name }) inside a JSX expression container, so it is not a
* ReactNode and TypeScript rejects it when <Box>'s children prop is ReactNode.
*/
it('does not flag nested double-brace interpolation, which only TypeScript rejects', () => {
expect(
lintTrans('<Trans t={t}><Box>Hello {{ name }}</Box></Trans>'),
).toEqual([]);
});

it('accepts a string literal child such as a whitespace separator', () => {
expect(lintTrans(`<Trans t={t}>Hello{' '}<b>there</b></Trans>`)).toEqual(
[],
);
});

it('accepts an expression in a nested element attribute', () => {
expect(
lintTrans('<Trans t={t}><Link href={url}>Docs</Link></Trans>'),
).toEqual([]);
});
});
6 changes: 3 additions & 3 deletions pages/accountLists/[accountListId]/setup/finish.page.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -43,10 +43,10 @@ const FinishPage: React.FC = () => {
</Head>
<SetupPage
title={
<Trans>
{t('Congratulations!')}
<Trans t={t}>

@zweatshirt zweatshirt Aug 12, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[High] **This `` key can never resolve, and it orphans two keys that are translated in 22 locales.**

Verified by execution, not inference — three agents each ran the real extractor into a scratchpad and rendered the real react-i18next@11.18.6:

  • yarn extract writes: Congratulations!<1></1>You're all set!
  • react-i18next requests: Congratulations!<br/>You're all set!

i18next-parser 9.0.2 serializes a propless <br /> as an indexed node; react-i18next keeps it as a tag (br is in the default transKeepBasicHtmlNodesFor, and nothing overrides it). The two strings can never be equal, so no state of the locale files reachable by Crowdin makes this render translated — proven by seeding the translated extracted key into the fr bundle and re-rendering: still English. public/locales/fr/translation.json:265 shows a human translator already localized inside this code artifact on the previous form, and it still never rendered.

Why the old code worked: {t('Congratulations!')} / {t("You're all set!")} were translated before Trans built its key, so the outer lookup missed and Trans fell back to rendering its own children — which were already translated. Removing the t() calls makes that fallback literal English.

Impact: both inner keys have real translations in 22 locales (en:862/:3349, fr:808/:3119, es-419 "¡Felicitaciones!"/"¡Está todo listo!", …). The composite key exists in none. Every non-English user reaching the last screen of onboarding sees an English <h2> over translated body copy. Silent: yarn eslint on this file exits 0, and finish.page.test.tsx never asserts the title.

Fix — satisfies both new lint rules, keeps every existing translation, no Crowdin round-trip needed:

// line 5: drop Trans from the import
import { useTranslation } from 'react-i18next';

// lines 45-51
title={
  <>
    {t('Congratulations!')}
    <br />
    {t("You're all set!")}
  </>
}

Worth adding the assertion that would have caught this, too:

it('renders the congratulations title', () => {
  const { getByText } = render(<TestComponent />);

  expect(getByText(/Congratulations!/)).toBeInTheDocument();
  expect(getByText(/You're all set!/)).toBeInTheDocument();
});

Congratulations!
<br />
{t("You're all set!")}
You&apos;re all set!
</Trans>
}
>
Expand Down
8 changes: 4 additions & 4 deletions src/components/HrTools/AdditionalSalaryRequest/RequestPage/Helper/SplitCapSubContent.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -54,10 +54,10 @@ export const SplitCapSubContent: React.FC<SplitCapSubContentProps> = ({
<>
<Trans t={t} values={{ spouseName }} parent="span">
Please make adjustments to your request to continue. You may make a
separate request up to {spouseName}&apos;s maximum allowable salary if
desired. After using both you and {spouseName}&apos;s maximum allowable
salary, any additional requests can be submitted online but will require
approval through our{' '}
separate request up to {{ spouseName }}&apos;s maximum allowable salary
if desired. After using both you and {{ spouseName }}&apos;s maximum
allowable salary, any additional requests can be submitted online but
will require approval through our{' '}
<Link
href={progressiveApprovalsLink}
target="_blank"
Expand Down
2 changes: 1 addition & 1 deletion ...components/HrTools/AdditionalSalaryRequest/RequestPage/Helper/SpouseOverCapSubContent.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ export const SpouseOverCapSubContent: React.FC<
return (
<Trans t={t} values={{ spouseName }} parent="span">
Please consider submitting your request at your maximum allowable salary
to reduce the amount on {spouseName}&apos;s request, which may avoid
to reduce the amount on {{ spouseName }}&apos;s request, which may avoid
requiring approval through our{' '}
<Link
href={progressiveApprovalsLink}
Expand Down
10 changes: 6 additions & 4 deletions src/components/HrTools/MinisterHousingAllowance/MainPages/EligibleDisplay.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import { Box } from '@mui/material';
import { Trans } from 'react-i18next';
import { Trans, useTranslation } from 'react-i18next';

export interface EligibleDisplayProps {
isPending: boolean;
Expand All @@ -10,24 +10,26 @@ export const EligibleDisplay: React.FC<EligibleDisplayProps> = ({
isPending,
isEditable,
}) => {
const { t } = useTranslation();

return (
<Box>
{isPending ? (
<p style={{ lineHeight: 1.5 }}>
<Trans>
<Trans t={t}>
Our records indicate that you have an MHA request{' '}
<strong>waiting to be processed</strong>. To view your MHA request,
click on the &quot;View Current MHA&quot; button below.
</Trans>
{isEditable && (
<Trans>
<Trans t={t}>
If you would like to make changes to your request, click on the
&quot;Edit Request&quot; button below.
</Trans>
)}
</p>
) : (
<Trans>
<Trans t={t}>
<p style={{ lineHeight: 1.5 }}>
Our records indicate that you have an approved MHA amount. To view
your MHA amount, click on the &quot;View Current MHA&quot; button
Expand Down
34 changes: 17 additions & 17 deletions src/components/HrTools/MinisterHousingAllowance/Steps/StepOne/AboutForm.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -57,7 +57,7 @@ export const AboutForm: React.FC<AboutFormProps> = ({
<Typography variant="h5">{t('About this Form')}</Typography>
</Box>
<p style={{ lineHeight: 1.5 }}>
<Trans>
<Trans t={t}>
A Minister&apos;s Housing Allowance Request is a form ministers
complete to designate part of their compensation as tax-free housing
allowance. To complete this form for the {{ nextYear }} tax year,
Expand Down Expand Up @@ -90,26 +90,26 @@ export const AboutForm: React.FC<AboutFormProps> = ({
</StyledListItem>
</List>
</Box>
<Trans values={{ boardDateFormatted, availableDateFormatted }}>
<Box sx={{ mt: 2 }}>
The next time the board will approve MHA Requests is {after} and your
approved annual MHA amount will appear on your{' '}
<Box sx={{ mt: 2 }}>
<Trans t={t} values={{ after, approval }}>
The next time the board will approve MHA Requests is {{ after }} and
your approved annual MHA amount will appear on your{' '}
<Link href={salaryLink} target="_blank">
Salary Calculation Form
</Link>
{approval} Once approved by the board, keep a copy for your tax
{{ approval }} Once approved by the board, keep a copy for your tax
records.
</Box>
<Box sx={{ mt: 4 }}>
<OpenInNew
fontSize="medium"
sx={{ verticalAlign: 'middle', opacity: 0.56 }}
/>{' '}
<Link component="button" type="button">
What expenses can I claim on my MHA?
</Link>
</Box>
</Trans>
</Trans>
</Box>
<Box sx={{ mt: 4 }}>
<OpenInNew
fontSize="medium"
sx={{ verticalAlign: 'middle', opacity: 0.56 }}
/>{' '}
<Link component="button" type="button">
{t('What expenses can I claim on my MHA?')}
</Link>
</Box>
<DirectionButtons
formTitle={t('MHA Request')}
handleNextStep={handleNextStep}
Expand Down
4 changes: 2 additions & 2 deletions ...nents/HrTools/MinisterHousingAllowance/Steps/StepThree/CalcComponents/FairRentalValue.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,7 @@ export const FairRentalValue: React.FC<FairRentalValueProps> = ({ schema }) => {
{t('Monthly market rental value of your home.')}
</Typography>
<Box sx={{ color: 'text.secondary' }}>
<Trans>
<Trans t={t}>
The best way to determine this amount is to have an appraiser or
rental real estate specialist provide you with a written
estimate of the monthly rental value. If this is not possible,
Expand Down Expand Up @@ -72,7 +72,7 @@ export const FairRentalValue: React.FC<FairRentalValueProps> = ({ schema }) => {
)}
</Typography>
<Box sx={{ color: 'text.secondary' }}>
<Trans>
<Trans t={t}>
This is a reasonable amount by which the monthly rental of your
home would increase if it were furnished.
</Trans>
Expand Down
8 changes: 4 additions & 4 deletions ...ts/HrTools/MinisterHousingAllowance/Steps/StepThree/CalcComponents/RequestSummaryCard.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -85,10 +85,10 @@ export const RequestSummaryCard: React.FC<RequestSummaryCardProps> = ({
<b>{t('Your Annual MHA Total')}</b>
</Typography>
<Box sx={{ color: 'text.secondary' }}>
<Trans values={{ above }}>
This is calculated from your {above} responses and is the
lower of the Annual Fair Rental Value or the Annual Cost of
Providing a Home.
<Trans t={t} values={{ above }}>

@canac canac Aug 12, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you research values more? It seems redundant, if not actively harmful. Maybe we want a follow-up lint rule to ensure that we don't pass values to Trans.

@zweatshirt zweatshirt Aug 13, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Making a separate PR for disallowing both values and defaults

This is calculated from your {{ above }} responses and is
the lower of the Annual Fair Rental Value or the Annual Cost
of Providing a Home.
</Trans>
</Box>
</TableCell>
Expand Down
6 changes: 3 additions & 3 deletions src/components/HrTools/MinisterHousingAllowance/Steps/StepThree/Calculation.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -278,7 +278,7 @@ export const Calculation: React.FC<CalculationProps> = ({
<PersonInfo />
) : actionRequired ? (
<p style={{ lineHeight: 1.5 }}>
<Trans values={{ after, approval }}>
<Trans t={t} values={{ after, approval }}>
Please review the Annual MHA Request that you have submitted
for Board approval and make any changes necessary here. The
board will review this {{ after }} and you will receive notice
Expand All @@ -287,7 +287,7 @@ export const Calculation: React.FC<CalculationProps> = ({
</p>
) : (
<p style={{ lineHeight: 1.5 }}>
<Trans values={{ after, approval }}>
<Trans t={t} values={{ after, approval }}>
Please enter dollar amounts for each category below to
calculate your Annual MHA. The board will review this{' '}
{{ after }} and you will receive notice of your {{ approval }}
Expand All @@ -302,7 +302,7 @@ export const Calculation: React.FC<CalculationProps> = ({
sx={{ verticalAlign: 'middle', opacity: 0.56 }}
/>{' '}
<Link component="button" type="button">
What expenses can I claim on my MHA?
<Trans t={t}>What expenses can I claim on my MHA?</Trans>
Comment thread
zweatshirt marked this conversation as resolved.
</Link>
</Box>
</SimpleScreenOnly>
Expand Down
5 changes: 3 additions & 2 deletions ...ts/HrTools/SalaryCalculator/EffectiveDateStep/EffectiveDateBanner/EffectiveDateBanner.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ import React from 'react';
import { Alert, Typography } from '@mui/material';
import { styled } from '@mui/material/styles';
import { DateTime } from 'luxon';
import { Trans } from 'react-i18next';
import { Trans, useTranslation } from 'react-i18next';
import { navBarHeight } from 'src/components/Layouts/Primary/Primary';

const StyledAlert = styled(Alert)({
Expand All @@ -25,6 +25,7 @@ interface EffectiveDateBannerProps {
export const EffectiveDateBanner: React.FC<EffectiveDateBannerProps> = ({
onClose,
}) => {
const { t } = useTranslation();
const thisYear = DateTime.now().year;
const nextYear = thisYear + 1;

Expand All @@ -36,7 +37,7 @@ export const EffectiveDateBanner: React.FC<EffectiveDateBannerProps> = ({
data-testid="effective-date-banner-text"
>
<Typography fontWeight="bold" textAlign="center">
<Trans values={{ thisYear, nextYear }}>
<Trans t={t} values={{ thisYear, nextYear }}>
Comment thread
zweatshirt marked this conversation as resolved.
Dates for {'{{nextYear}}'} are unavailable at this time while we
update salary level tables. By December 15, {'{{thisYear}}'} you will
be able to request a salary change for {'{{nextYear}}'}.
Expand Down
1 change: 1 addition & 0 deletions src/components/Settings/integrations/Google/GoogleAccordion.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -236,6 +236,7 @@
color={theme.palette.mpdxGrayDark.main}
>
<Trans
t={t}

Check warning on line 239 in src/components/Settings/integrations/Google/GoogleAccordion.tsx

View check run for this annotation

CodeScene Delta Analysis / CodeScene Code Health Review (main)

❌ Getting worse: Large Method 

GoogleAccordion:React.FC<AccordionProps> increases from 190 to 191 lines of code, threshold = 120

Large functions with many lines of code are generally harder to understand and lower the code health.
Avoid adding more lines to this function.
defaults="When you add a Google account to {{appName}}, Google will ask you what {{appName}} should be allowed to access. <bold>Please select ALL of the checkboxes.</bold><br/><br/>Otherwise, {{appName}} may not work properly."
shouldUnescape
values={{ appName }}
Expand Down
7 changes: 5 additions & 2 deletions src/components/Shared/Filters/NullState/NullState.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ interface CreateButtonProps {
}

const CreateButton: React.FC<CreateButtonProps> = ({ page }) => {
const { t } = useTranslation();
const { openTaskModal, preloadTaskModal } = useTaskModal();
const [contactsDialogOpen, setContactsDialogOpen] = useState(false);

Expand Down Expand Up @@ -51,7 +52,7 @@ const CreateButton: React.FC<CreateButtonProps> = ({ page }) => {
backgroundColor: theme.palette.mpdxBlue.main,
}}
>
<Trans defaults="Add new {{page}}" values={{ page }} />
<Trans t={t} defaults="Add new {{page}}" values={{ page }} />

@canac canac Aug 12, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Another possible rule would be to reject defaults as well and require the text to be a child of <Trans>.

</Button>
{renderDialog(
AddMenuItemsEnum.NewContact,
Expand Down Expand Up @@ -104,19 +105,21 @@ const NullState: React.FC<Props> = ({
<>
<Typography variant="h5">
<Trans
t={t}
defaults="Looks like you haven't added any {{page}}s yet"
values={{ page }}
/>
</Typography>
<Typography>
<Trans
t={t}
defaults="You can import {{page}}s from another service or add a new {{page}}."
values={{ page }}
/>
</Typography>
<Box display="flex" mt={1}>
<Button variant="contained">
<Trans defaults="Import {{page}}s" values={{ page }} />
<Trans t={t} defaults="Import {{page}}s" values={{ page }} />
</Button>
<CreateButton page={page} />
</Box>
Expand Down
4 changes: 1 addition & 3 deletions src/components/Shared/LimitedAccess/LimitedAccess.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,9 +21,7 @@ export const LimitedAccess: React.FC<LimitedAccessProps> = ({
<Link
href="mailto:support@mpdx.org"
style={{ color: theme.palette.primary.main, fontWeight: 'bold' }}
>
support@mpdx.org
</Link>
/>
);

const { title, content } = getLimitedText({
Expand Down
Loading
Loading