From 90f19f6164121bb2d76c12a8c83206e2ed146b76 Mon Sep 17 00:00:00 2001 From: Patrik Oldsberg Date: Wed, 18 Feb 2026 23:04:32 +0100 Subject: [PATCH] cli: address review feedback on translations commands - Fix Windows path separators in import by splitting on path.sep - Make extractTranslations test less brittle by not pinning exact message strings or total count - Remove unnecessary `as any` cast for devDependencies - Validate pattern in export command to fail fast on invalid patterns Signed-off-by: Patrik Oldsberg Co-authored-by: Cursor --- .../modules/translations/commands/export.ts | 8 +++++++- .../modules/translations/commands/import.ts | 14 ++++++------- .../translations/lib/discoverPackages.ts | 2 +- .../lib/extractTranslations.test.ts | 20 ++++++++----------- .../translations/lib/messageFilePath.ts | 2 +- 5 files changed, 24 insertions(+), 22 deletions(-) diff --git a/packages/cli/src/modules/translations/commands/export.ts b/packages/cli/src/modules/translations/commands/export.ts index cf6630ae39..96b3ceb0c6 100644 --- a/packages/cli/src/modules/translations/commands/export.ts +++ b/packages/cli/src/modules/translations/commands/export.ts @@ -26,7 +26,11 @@ import { extractTranslationRefsFromSourceFile, TranslationRefInfo, } from '../lib/extractTranslations'; -import { DEFAULT_LANGUAGE, formatMessagePath } from '../lib/messageFilePath'; +import { + DEFAULT_LANGUAGE, + formatMessagePath, + validatePattern, +} from '../lib/messageFilePath'; interface ExportOptions { output: string; @@ -34,6 +38,8 @@ interface ExportOptions { } export default async (options: ExportOptions) => { + validatePattern(options.pattern); + const targetPackageJson = await readTargetPackage( paths.targetDir, paths.targetRoot, diff --git a/packages/cli/src/modules/translations/commands/import.ts b/packages/cli/src/modules/translations/commands/import.ts index cab2030d1f..7275da4c24 100644 --- a/packages/cli/src/modules/translations/commands/import.ts +++ b/packages/cli/src/modules/translations/commands/import.ts @@ -19,7 +19,7 @@ import fs from 'fs-extra'; import { resolve as resolvePath, relative as relativePath, - posix as posixPath, + sep, } from 'node:path'; import { readTargetPackage } from '../lib/discoverPackages'; import { @@ -141,12 +141,12 @@ export default async (options: ImportOptions) => { const translationEntries = entries .sort((a, b) => a.lang.localeCompare(b.lang)) .map(({ lang, relPath }) => { - const jsonRelPath = posixPath.normalize( - relativePath( - resolvePath(outputPath, '..'), - resolvePath(inputDir, relPath), - ), - ); + const jsonRelPath = relativePath( + resolvePath(outputPath, '..'), + resolvePath(inputDir, relPath), + ) + .split(sep) + .join('/'); return ` ${JSON.stringify(lang)}: () => import('./${jsonRelPath}'),`; }) .join('\n'); diff --git a/packages/cli/src/modules/translations/lib/discoverPackages.ts b/packages/cli/src/modules/translations/lib/discoverPackages.ts index 32c54da297..c7a7602a7e 100644 --- a/packages/cli/src/modules/translations/lib/discoverPackages.ts +++ b/packages/cli/src/modules/translations/lib/discoverPackages.ts @@ -92,7 +92,7 @@ export async function discoverFrontendPackages( ) { const deps: Record = { ...packageJson.dependencies, - ...(includeDevDeps ? (packageJson as any).devDependencies : {}), + ...(includeDevDeps ? packageJson.devDependencies ?? {} : {}), }; for (const depName of Object.keys(deps)) { diff --git a/packages/cli/src/modules/translations/lib/extractTranslations.test.ts b/packages/cli/src/modules/translations/lib/extractTranslations.test.ts index 8b46c63eff..27b67db052 100644 --- a/packages/cli/src/modules/translations/lib/extractTranslations.test.ts +++ b/packages/cli/src/modules/translations/lib/extractTranslations.test.ts @@ -44,21 +44,17 @@ describe('extractTranslations', () => { exportName: 'orgTranslationRef', }); - // Verify a subset of messages - expect(refs[0].messages).toMatchObject({ - 'groupProfileCard.groupNotFound': 'Group not found', - 'membersListCard.title': 'Members', - 'membersListCard.subtitle': 'of {{groupName}}', - 'userProfileCard.userNotFound': 'User not found', - }); + expect(refs[0].messages).toBeDefined(); + expect(Object.keys(refs[0].messages)).not.toHaveLength(0); + + // Verify some well-known keys exist without pinning exact wording + expect(refs[0].messages).toHaveProperty(['groupProfileCard.groupNotFound']); + expect(refs[0].messages).toHaveProperty(['membersListCard.title']); // Verify interpolation placeholders are preserved - expect(refs[0].messages['membersListCard.paginationLabel']).toBe( - ', page {{page}} of {{nbPages}}', + expect(refs[0].messages['membersListCard.subtitle']).toContain( + '{{groupName}}', ); - - // Verify total message count - expect(Object.keys(refs[0].messages).length).toBe(26); }); it('ignores non-TranslationRef exports', () => { diff --git a/packages/cli/src/modules/translations/lib/messageFilePath.ts b/packages/cli/src/modules/translations/lib/messageFilePath.ts index 0eb2547759..eb3dc8ea5c 100644 --- a/packages/cli/src/modules/translations/lib/messageFilePath.ts +++ b/packages/cli/src/modules/translations/lib/messageFilePath.ts @@ -64,7 +64,7 @@ export function patternHasSubdirectories(pattern: string): boolean { return pattern.includes('/'); } -function validatePattern(pattern: string) { +export function validatePattern(pattern: string) { if (!pattern.includes('{id}')) { throw new Error( `Invalid message file pattern: must contain {id} placeholder. Got: ${pattern}`,