Skip to content

Commit

Permalink
[i18n tooling] throw when a variable is passed to template literal in…
Browse files Browse the repository at this point in the history
… default message (elastic#190582)

With the new i18n tooling, we now allow template literals to be used for
the `defaultMessage` however they cannot contain variable substitutions.

This PR adds a check in the parser that throws an error when there is a
variable inside the template literal asking the developer to use
`values` instead.

Previously the parser simply skipped extracting the message which causes
confusion for developers as they expect the CI to fail on errors instead
of just silently allowing it bypassing translations.

Thankfully we dont have any cases in the codebase that are doing direct
substituion yet so this bug fix came at the right time :) thanks
@maryam-saeidi for catching it!
  • Loading branch information
Bamieh authored Aug 15, 2024
1 parent 2c5ae36 commit 9f0108b
Show file tree
Hide file tree
Showing 3 changed files with 37 additions and 0 deletions.
Original file line number Diff line number Diff line change
@@ -0,0 +1,20 @@
/*
* Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one
* or more contributor license agreements. Licensed under the Elastic License
* 2.0 and the Server Side Public License, v 1; you may not use this file except
* in compliance with, at your election, the Elastic License 2.0 or the Server
* Side Public License, v 1.
*/

import { i18n } from '@kbn/i18n';

const e = new Error('Should not work');

i18n.translate('contains_variable', {
defaultMessage: `value passed into literal directly (e: ${e.message})`,
});

i18n.translate('no_variable', {
defaultMessage: `template literal without any variable expressions (e: {errorMessage})`,
values: { errorMessage: e.message },
});
9 changes: 9 additions & 0 deletions src/dev/i18n_tools/extractors/call_expt.ts
Original file line number Diff line number Diff line change
Expand Up @@ -286,6 +286,15 @@ export function extractMessageDescriptor(
break;
}
}
} else if (typescript.isTemplateExpression(initializer)) {
initializer.forEachChild((child) => {
if (typescript.isTemplateSpan(child)) {
const messageText = initializer.getFullText();
throw new Error(
`Template literals with variable substitution is not supported. please pass variables via the 'values' object instead. Message ${messageText}`
);
}
});
}
// {id: `id`}
else if (typescript.isNoSubstitutionTemplateLiteral(initializer)) {
Expand Down
8 changes: 8 additions & 0 deletions src/dev/i18n_tools/extractors/formatjs.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -267,6 +267,14 @@ describe('formatJS Runner', () => {
}
`);
});

it('throws when template literal has a variable', async () => {
await expect(async () =>
formatJsFixtureRunner('template_literal_var.ts')
).rejects.toMatchInlineSnapshot(
`[Error: Error parsing file template_literal_var.ts: Error: Template literals with variable substitution is not supported. please pass variables via the 'values' object instead. Message \`value passed into literal directly (e: \${e.message})\`]`
);
});
});

describe('extraction inside react components', () => {
Expand Down

0 comments on commit 9f0108b

Please sign in to comment.