From 39d8f1a237744eb1f49413e45f349e63fe43c061 Mon Sep 17 00:00:00 2001 From: Timofei Iatsenko Date: Mon, 24 Jun 2024 12:20:04 +0200 Subject: [PATCH] feat: collect and print placeholders in Po files for better translation context --- .../src/index.ts | 102 ++++++++++++++---- .../test/__snapshots__/index.ts.snap | 77 ++++++++++++- .../test/fixtures/js-message-descriptor.js | 18 +++- packages/conf/src/types.ts | 1 + .../src/__snapshots__/extractor.test.ts.snap | 7 ++ .../src/__snapshots__/po.test.ts.snap | 22 ++++ packages/format-po/src/po.test.ts | 27 +++++ packages/format-po/src/po.ts | 36 +++++++ 8 files changed, 265 insertions(+), 25 deletions(-) diff --git a/packages/babel-plugin-extract-messages/src/index.ts b/packages/babel-plugin-extract-messages/src/index.ts index 14e95c691..d48456ff8 100644 --- a/packages/babel-plugin-extract-messages/src/index.ts +++ b/packages/babel-plugin-extract-messages/src/index.ts @@ -5,8 +5,10 @@ import { Node, ObjectExpression, ObjectProperty, + isObjectExpression, } from "@babel/types" import type { PluginObj, PluginPass, NodePath } from "@babel/core" +import {} from "@babel/core" import type { Hub } from "@babel/traverse" type BabelTypes = typeof BabelTypesNamespace @@ -19,6 +21,7 @@ export type ExtractedMessage = { origin?: Origin comment?: string + placeholders?: Record } export type ExtractPluginOpts = { @@ -30,6 +33,7 @@ type RawMessage = { message?: string comment?: string context?: string + placeholders?: Record } export type Origin = [filename: string, line: number, column?: number] @@ -52,6 +56,7 @@ function collectMessage( message: props.message, context: props.context, comment: props.comment, + placeholders: props.placeholders || {}, origin: [ctx.file.opts.filename, line, column], }) } @@ -109,19 +114,67 @@ function getTextFromExpression( } } -function extractFromObjectExpression( +function getNodeSource(fileContents: string, node: Node) { + return fileContents.slice(node.start, node.end) +} + +function valuesObjectExpressionToPlaceholdersRecord( t: BabelTypes, exp: ObjectExpression, - hub: Hub, - keys: readonly string[] + hub: Hub ) { const props: Record = {} + ;(exp.properties as ObjectProperty[]).forEach(({ key, value }, i) => { + let name: string + + if (t.isStringLiteral(key) || t.isNumericLiteral(key)) { + name = key.value.toString() + } else if (t.isIdentifier(key)) { + name = key.name + } else { + console.warn( + hub.buildError( + exp, + `Could not extract values to placeholders. The key #${i} has unsupported syntax`, + SyntaxError + ).message + ) + } + + if (name) { + props[name] = getNodeSource(hub.getCode(), value) + } + }) + + return props +} + +function extractFromObjectExpression( + t: BabelTypes, + exp: ObjectExpression, + hub: Hub +) { + const props: RawMessage = {} + + const textKeys = ["id", "message", "comment", "context"] as const + ;(exp.properties as ObjectProperty[]).forEach(({ key, value }, i) => { const name = (key as Identifier).name - if (!keys.includes(name as any)) return - props[name] = getTextFromExpression(t, value as Expression, hub) + if (name === "values" && isObjectExpression(value)) { + props.placeholders = valuesObjectExpressionToPlaceholdersRecord( + t, + value, + hub + ) + } else if (textKeys.includes(name as any)) { + props[name as (typeof textKeys)[number]] = getTextFromExpression( + t, + value as Expression, + hub + ) + } }) return props @@ -167,12 +220,7 @@ export default function ({ types: t }: { types: BabelTypes }): PluginObj { path: NodePath, ctx: PluginPass ) => { - const props = extractFromObjectExpression(t, path.node, ctx.file.hub, [ - "id", - "message", - "comment", - "context", - ]) + const props = extractFromObjectExpression(t, path.node, ctx.file.hub) if (!props.id) { console.warn( @@ -200,7 +248,7 @@ export default function ({ types: t }: { types: BabelTypes }): PluginObj { return } - const props = attrs.reduce>((acc, item) => { + const props = attrs.reduce((acc, item) => { if (t.isJSXSpreadAttribute(item)) { return acc } @@ -221,6 +269,19 @@ export default function ({ types: t }: { types: BabelTypes }): PluginObj { acc[key] = item.value.expression.value } } + + if ( + key === "values" && + t.isJSXExpressionContainer(item.value) && + isObjectExpression(item.value.expression) + ) { + acc.placeholders = valuesObjectExpressionToPlaceholdersRecord( + t, + item.value.expression, + ctx.file.hub + ) + } + return acc }, {}) @@ -266,7 +327,7 @@ export default function ({ types: t }: { types: BabelTypes }): PluginObj { return } else { // i18n._(id, variables, descriptor) - let props = { + let props: RawMessage = { id: getTextFromExpression( t, firstArgument.node as Expression, @@ -279,16 +340,21 @@ export default function ({ types: t }: { types: BabelTypes }): PluginObj { return } + const secondArgument = path.node.arguments[1] + if (secondArgument && t.isObjectExpression(secondArgument)) { + props.placeholders = valuesObjectExpressionToPlaceholdersRecord( + t, + secondArgument, + ctx.file.hub + ) + } + const msgDescArg = path.node.arguments[2] if (t.isObjectExpression(msgDescArg)) { props = { ...props, - ...extractFromObjectExpression(t, msgDescArg, ctx.file.hub, [ - "message", - "comment", - "context", - ]), + ...extractFromObjectExpression(t, msgDescArg, ctx.file.hub), } } diff --git a/packages/babel-plugin-extract-messages/test/__snapshots__/index.ts.snap b/packages/babel-plugin-extract-messages/test/__snapshots__/index.ts.snap index a121c767e..9b0fcc714 100644 --- a/packages/babel-plugin-extract-messages/test/__snapshots__/index.ts.snap +++ b/packages/babel-plugin-extract-messages/test/__snapshots__/index.ts.snap @@ -11,6 +11,7 @@ exports[`@lingui/babel-plugin-extract-messages CallExpression i18n._() should ex js-call-expression.js, 1, ], + placeholders: {}, }, { comment: description, @@ -21,6 +22,7 @@ exports[`@lingui/babel-plugin-extract-messages CallExpression i18n._() should ex js-call-expression.js, 3, ], + placeholders: {}, }, { comment: undefined, @@ -31,6 +33,7 @@ exports[`@lingui/babel-plugin-extract-messages CallExpression i18n._() should ex js-call-expression.js, 5, ], + placeholders: {}, }, { comment: undefined, @@ -41,6 +44,9 @@ exports[`@lingui/babel-plugin-extract-messages CallExpression i18n._() should ex js-call-expression.js, 7, ], + placeholders: { + param: param, + }, }, { comment: undefined, @@ -51,6 +57,7 @@ exports[`@lingui/babel-plugin-extract-messages CallExpression i18n._() should ex js-call-expression.js, 9, ], + placeholders: {}, }, { comment: My comment, @@ -61,6 +68,7 @@ exports[`@lingui/babel-plugin-extract-messages CallExpression i18n._() should ex js-call-expression.js, 12, ], + placeholders: {}, }, { comment: undefined, @@ -71,6 +79,7 @@ exports[`@lingui/babel-plugin-extract-messages CallExpression i18n._() should ex js-call-expression.js, 19, ], + placeholders: {}, }, { comment: My comment, @@ -81,6 +90,7 @@ exports[`@lingui/babel-plugin-extract-messages CallExpression i18n._() should ex js-call-expression.js, 22, ], + placeholders: {}, }, ] `; @@ -96,6 +106,7 @@ exports[`@lingui/babel-plugin-extract-messages MessageDescriptor should extract js-message-descriptor.js, 1, ], + placeholders: {}, }, { comment: description, @@ -106,6 +117,7 @@ exports[`@lingui/babel-plugin-extract-messages MessageDescriptor should extract js-message-descriptor.js, 3, ], + placeholders: {}, }, { comment: undefined, @@ -116,16 +128,33 @@ exports[`@lingui/babel-plugin-extract-messages MessageDescriptor should extract js-message-descriptor.js, 5, ], + placeholders: {}, }, { comment: undefined, context: undefined, - id: Values {param}, + id: Values {param} {0} {name}, message: undefined, origin: [ js-message-descriptor.js, 7, ], + placeholders: { + 0: user.getName(), + name: "foo", + param: param, + }, + }, + { + comment: undefined, + context: undefined, + id: Values {param} {0}, + message: undefined, + origin: [ + js-message-descriptor.js, + 15, + ], + placeholders: {}, }, { comment: undefined, @@ -134,8 +163,9 @@ exports[`@lingui/babel-plugin-extract-messages MessageDescriptor should extract message: undefined, origin: [ js-message-descriptor.js, - 9, + 17, ], + placeholders: {}, }, ] `; @@ -151,6 +181,9 @@ exports[`@lingui/babel-plugin-extract-messages should extract Plural messages fr jsx-without-trans.js, 2, ], + placeholders: { + count: count, + }, }, { comment: undefined, @@ -161,6 +194,9 @@ exports[`@lingui/babel-plugin-extract-messages should extract Plural messages fr jsx-without-trans.js, 3, ], + placeholders: { + count: count, + }, }, ] `; @@ -176,6 +212,7 @@ exports[`@lingui/babel-plugin-extract-messages should extract all messages from js-with-macros.js, 4, ], + placeholders: {}, }, { comment: undefined, @@ -186,6 +223,7 @@ exports[`@lingui/babel-plugin-extract-messages should extract all messages from js-with-macros.js, 6, ], + placeholders: {}, }, { comment: description, @@ -196,6 +234,7 @@ exports[`@lingui/babel-plugin-extract-messages should extract all messages from js-with-macros.js, 8, ], + placeholders: {}, }, { comment: undefined, @@ -206,6 +245,7 @@ exports[`@lingui/babel-plugin-extract-messages should extract all messages from js-with-macros.js, 13, ], + placeholders: {}, }, { comment: undefined, @@ -216,6 +256,9 @@ exports[`@lingui/babel-plugin-extract-messages should extract all messages from js-with-macros.js, 18, ], + placeholders: { + param: param, + }, }, { comment: undefined, @@ -226,6 +269,7 @@ exports[`@lingui/babel-plugin-extract-messages should extract all messages from js-with-macros.js, 20, ], + placeholders: {}, }, { comment: undefined, @@ -236,6 +280,7 @@ exports[`@lingui/babel-plugin-extract-messages should extract all messages from js-with-macros.js, 25, ], + placeholders: {}, }, { comment: undefined, @@ -246,6 +291,7 @@ exports[`@lingui/babel-plugin-extract-messages should extract all messages from js-with-macros.js, 29, ], + placeholders: {}, }, { comment: undefined, @@ -256,6 +302,7 @@ exports[`@lingui/babel-plugin-extract-messages should extract all messages from js-with-macros.js, 34, ], + placeholders: {}, }, { comment: undefined, @@ -266,6 +313,7 @@ exports[`@lingui/babel-plugin-extract-messages should extract all messages from js-with-macros.js, 39, ], + placeholders: {}, }, { comment: undefined, @@ -276,6 +324,7 @@ exports[`@lingui/babel-plugin-extract-messages should extract all messages from js-with-macros.js, 44, ], + placeholders: {}, }, { comment: undefined, @@ -286,6 +335,7 @@ exports[`@lingui/babel-plugin-extract-messages should extract all messages from js-with-macros.js, 49, ], + placeholders: {}, }, { comment: undefined, @@ -296,6 +346,7 @@ exports[`@lingui/babel-plugin-extract-messages should extract all messages from js-with-macros.js, 54, ], + placeholders: {}, }, { comment: undefined, @@ -306,6 +357,9 @@ exports[`@lingui/babel-plugin-extract-messages should extract all messages from js-with-macros.js, 57, ], + placeholders: { + 0: users.length, + }, }, ] `; @@ -321,6 +375,9 @@ exports[`@lingui/babel-plugin-extract-messages should extract all messages from jsx-with-macros.js, 3, ], + placeholders: { + name: name, + }, }, { comment: undefined, @@ -331,6 +388,7 @@ exports[`@lingui/babel-plugin-extract-messages should extract all messages from jsx-with-macros.js, 4, ], + placeholders: {}, }, { comment: undefined, @@ -341,6 +399,7 @@ exports[`@lingui/babel-plugin-extract-messages should extract all messages from jsx-with-macros.js, 5, ], + placeholders: {}, }, { comment: undefined, @@ -351,6 +410,7 @@ exports[`@lingui/babel-plugin-extract-messages should extract all messages from jsx-with-macros.js, 6, ], + placeholders: {}, }, { comment: undefined, @@ -361,6 +421,7 @@ exports[`@lingui/babel-plugin-extract-messages should extract all messages from jsx-with-macros.js, 7, ], + placeholders: {}, }, { comment: undefined, @@ -371,6 +432,9 @@ exports[`@lingui/babel-plugin-extract-messages should extract all messages from jsx-with-macros.js, 9, ], + placeholders: { + count: count, + }, }, ] `; @@ -386,6 +450,7 @@ exports[`@lingui/babel-plugin-extract-messages should extract all messages from jsx-without-macros.js, 5, ], + placeholders: {}, }, { comment: undefined, @@ -396,6 +461,7 @@ exports[`@lingui/babel-plugin-extract-messages should extract all messages from jsx-without-macros.js, 6, ], + placeholders: {}, }, { comment: undefined, @@ -406,6 +472,7 @@ exports[`@lingui/babel-plugin-extract-messages should extract all messages from jsx-without-macros.js, 7, ], + placeholders: {}, }, { comment: undefined, @@ -416,6 +483,7 @@ exports[`@lingui/babel-plugin-extract-messages should extract all messages from jsx-without-macros.js, 8, ], + placeholders: {}, }, { comment: undefined, @@ -426,6 +494,7 @@ exports[`@lingui/babel-plugin-extract-messages should extract all messages from jsx-without-macros.js, 9, ], + placeholders: {}, }, { comment: undefined, @@ -436,6 +505,7 @@ exports[`@lingui/babel-plugin-extract-messages should extract all messages from jsx-without-macros.js, 10, ], + placeholders: {}, }, { comment: undefined, @@ -446,6 +516,9 @@ exports[`@lingui/babel-plugin-extract-messages should extract all messages from jsx-without-macros.js, 11, ], + placeholders: { + count: count, + }, }, ] `; diff --git a/packages/babel-plugin-extract-messages/test/fixtures/js-message-descriptor.js b/packages/babel-plugin-extract-messages/test/fixtures/js-message-descriptor.js index 70ab1e3b8..55fbf9be9 100644 --- a/packages/babel-plugin-extract-messages/test/fixtures/js-message-descriptor.js +++ b/packages/babel-plugin-extract-messages/test/fixtures/js-message-descriptor.js @@ -1,9 +1,17 @@ -const msg = /*i18n*/{id: 'Message'} +const msg = /*i18n*/ { id: "Message" } -const withDescription = /*i18n*/{id: 'Description', comment: "description"} +const withDescription = /*i18n*/ { id: "Description", comment: "description" } -const withId = /*i18n*/{id: 'ID', message: 'Message with id'} +const withId = /*i18n*/ { id: "ID", message: "Message with id" } -const withValues = /*i18n*/{id: 'Values {param}', values: { param: param }} +const withValues = /*i18n*/ { + id: "Values {param} {0} {name}", + values: { param: param, 0: user.getName(), ["name"]: "foo" }, +} +/** + * With values passed as variable + */ +const values = {} +const withValues2 = /*i18n*/ { id: "Values {param} {0}", values } -const withContext = /*i18n*/{id: 'Some id', context: 'Context1'} +const withContext = /*i18n*/ { id: "Some id", context: "Context1" } diff --git a/packages/conf/src/types.ts b/packages/conf/src/types.ts index 0754406fc..2aaa0b5ff 100644 --- a/packages/conf/src/types.ts +++ b/packages/conf/src/types.ts @@ -36,6 +36,7 @@ export type ExtractedMessageType = { * formatters can store additional data */ extra?: Extra + placeholders?: Record } export type MessageType = ExtractedMessageType & { translation: string diff --git a/packages/extractor-vue/src/__snapshots__/extractor.test.ts.snap b/packages/extractor-vue/src/__snapshots__/extractor.test.ts.snap index 646453824..04603b0c3 100644 --- a/packages/extractor-vue/src/__snapshots__/extractor.test.ts.snap +++ b/packages/extractor-vue/src/__snapshots__/extractor.test.ts.snap @@ -12,6 +12,7 @@ exports[`vue extractor should extract message from functional component 1`] = ` 10, 33, ], + placeholders: {}, }, ] `; @@ -28,6 +29,7 @@ exports[`vue extractor should extract message from vue file 1`] = ` 4, 0, ], + placeholders: {}, }, { comment: undefined, @@ -39,6 +41,7 @@ exports[`vue extractor should extract message from vue file 1`] = ` 19, 20, ], + placeholders: {}, }, { comment: undefined, @@ -50,6 +53,7 @@ exports[`vue extractor should extract message from vue file 1`] = ` 27, 11, ], + placeholders: {}, }, { comment: Message comment, @@ -61,6 +65,7 @@ exports[`vue extractor should extract message from vue file 1`] = ` 29, 10, ], + placeholders: {}, }, { comment: undefined, @@ -72,6 +77,7 @@ exports[`vue extractor should extract message from vue file 1`] = ` 35, 5, ], + placeholders: {}, }, { comment: undefined, @@ -83,6 +89,7 @@ exports[`vue extractor should extract message from vue file 1`] = ` 36, 11, ], + placeholders: {}, }, ] `; diff --git a/packages/format-po/src/__snapshots__/po.test.ts.snap b/packages/format-po/src/__snapshots__/po.test.ts.snap index fbb4d13d4..29b705bcd 100644 --- a/packages/format-po/src/__snapshots__/po.test.ts.snap +++ b/packages/format-po/src/__snapshots__/po.test.ts.snap @@ -72,6 +72,28 @@ msgstr "" `; +exports[`pofile format should print unnamed placeholders as comments 1`] = ` +msgid "" +msgstr "" +"POT-Creation-Date: 2018-08-27 10:00+0000\\n" +"MIME-Version: 1.0\\n" +"Content-Type: text/plain; charset=utf-8\\n" +"Content-Transfer-Encoding: 8bit\\n" +"X-Generator: @lingui/cli\\n" +"Language: en\\n" + +#. js-lingui-explicit-id +#. ph: {0} = getValue() +msgid "static" +msgstr "Static message {0} {name}" + +#. js-lingui-explicit-id +#. ph: {0} = getValue() +msgid "static2" +msgstr "Static message {0} {name}" + +`; + exports[`pofile format should read catalog in pofile format 1`] = ` { obsolete: { diff --git a/packages/format-po/src/po.test.ts b/packages/format-po/src/po.test.ts index 0a83fd111..d5da4a34f 100644 --- a/packages/format-po/src/po.test.ts +++ b/packages/format-po/src/po.test.ts @@ -442,4 +442,31 @@ describe("pofile format", () => { `) }) + + it("should print unnamed placeholders as comments", () => { + const format = createFormatter({ printPlaceholdersInComments: true }) + + const catalog: CatalogType = { + static: { + message: "Static message {0} {name}", + translation: "Static message {0} {name}", + placeholders: { + 0: "getValue()", + name: "user.getName()", + }, + }, + static2: { + message: "Static message {0} {name}", + translation: "Static message {0} {name}", + comments: ["ph: {0} = getValue()"], + placeholders: { + 0: "getValue()", + name: "user.getName()", + }, + }, + } + + const actual = format.serialize(catalog, defaultSerializeCtx) + expect(actual).toMatchSnapshot() + }) }) diff --git a/packages/format-po/src/po.ts b/packages/format-po/src/po.ts index cb34b3841..f8e6764d9 100644 --- a/packages/format-po/src/po.ts +++ b/packages/format-po/src/po.ts @@ -58,6 +58,28 @@ export type PoFormatterOptions = { * @default false */ explicitIdAsDefault?: boolean + + /** + * Print values for unnamed placeholders as comments for each message. + * + * This can give more context to translators for better translations. + * + * Example: + * + * ```js + * t`Hello ${user.name} ${value}` + * ``` + * + * This will be extracted as + * + * ```gettext + * #. ph: {0} = user.name + * msgid "Hello {0} {value}" + * ``` + * + * @default true + */ + printPlaceholdersInComments?: boolean } function isGeneratedId(id: string, message: MessageType): boolean { @@ -121,6 +143,20 @@ const serialize = (catalog: CatalogType, options: PoFormatterOptions) => { item.msgid = id } + if (options.printPlaceholdersInComments) { + item.extractedComments = item.extractedComments.filter( + (comment) => !comment.startsWith("ph:") + ) + + if (message.placeholders) { + Object.entries(message.placeholders).forEach(([name, value]) => { + if (/^\d+$/.test(name)) { + item.extractedComments.push(`ph: {${name}} = ${value}`) + } + }) + } + } + if (message.context) { item.msgctxt = message.context }