Repository: lingui/js-lingui
Pull request #1309 (2022-12-06 12:16:37)

feat: allow extract to work with i18n._ calls not created from macro
This change updates babel-plugin-extract-messages so that it will pick up usages of the i18n._ method from @lingui/core directly.

This solves the issues mentioned in https://github.com/lingui/js-lingui/issues/1178 and https://github.com/lingui/js-lingui/issues/1224#issuecomment-1317265270 without needing the workaround listed in 1178 (prefixing the id with `/*i18n*/`).

The main driver for this is to allow use of Lingui without macros, so that sites running on Next.js can use the SWC compiler instead of babel.

Aware that this is effectively adding back extract behavior that was removed previously, but hopefully this isn't too big of a deal - let me know if you have any feedback.

vercel[bot]: **The latest updates on your projects**. Learn more about [Vercel for Git ↗︎](https://vercel.link/github-learn-more)

| Name | Status | Preview | Updated |
| :--- | :----- | :------ | :------ |
| **js-lingui** | 🔄 Building ([Inspect](https://vercel.com/lingui/js-lingui/7oqTBogUb1hUeLf8AmeZEjHPA9AB)) |  | Dec 6, 2022 at 0:16AM (UTC) |



Patch:
diff --git a/packages/babel-plugin-extract-messages/src/index.ts b/packages/babel-plugin-extract-messages/src/index.ts
index 91d9f6a1..358529bc 100644
--- a/packages/babel-plugin-extract-messages/src/index.ts
+++ b/packages/babel-plugin-extract-messages/src/index.ts
@@ -109,6 +109,28 @@ function extractStringContatentation(t, node, error): string {
   }
 }
 
+function extractCommentString(t, path, valuePath, valueObj): string {
+  if (t.isStringLiteral(valueObj)) {
+    // Comment is a single line string
+    return valueObj.value;
+  }
+
+  // Comment is a multi-line string.
+  const errorIfNotAString = path
+    .get(valuePath)
+    .buildCodeFrameError("Only strings are supported as comments.")
+
+  if (t.isBinaryExpression(valueObj)) {
+    return extractStringContatentation(
+      t,
+      valueObj,
+      errorIfNotAString
+    )
+  } else {
+    throw errorIfNotAString
+  }
+}
+
 export default function ({ types: t }) {
   let localTransComponentName
 
@@ -220,25 +242,32 @@ export default function ({ types: t }) {
             )
           }
         )
-        if (!hasComment) return
-
+        
+        const isNonMacroI18n = isI18nMethod(path.node.callee) && !hasComment && path.node.arguments[0] && !path.node.arguments[0].leadingComments;
+        if (!hasComment && !isNonMacroI18n) return;
         const props = {
-          id: path.node.arguments[0].value,
-        }
+          id: path.node.arguments[0].value
+        };
 
         if (!props.id) {
-          console.warn("Missing message ID, skipping.")
+          console.warn("Missing message ID, skipping.");
           console.warn(generate(path.node).code)
-          return
+          return;
         }
 
         const copyOptions = ["message", "comment", "context"]
 
         if (t.isObjectExpression(path.node.arguments[2])) {
-          path.node.arguments[2].properties.forEach((property) => {
-            if (!copyOptions.includes(property.key.name)) return
+          path.node.arguments[2].properties.forEach(({key, value}, i) => {
+            if (!copyOptions.includes(key.name)) return
+
+            let valueToExtract = value.value;
+
+            if (key.name === "comment") {
+              valueToExtract = extractCommentString(t, path, `arguments.2.properties.${i}.value`, value);
+            }
 
-            props[property.key.name] = property.value.value
+            props[key.name] = valueToExtract
           })
         }
 
@@ -298,21 +327,8 @@ export default function ({ types: t }) {
             // By default, the value is just the string value of the object property.
             let valueToExtract = value.value;
 
-            if (key.name === "comment" && !t.isStringLiteral(value)) {
-              // Comments can be single or multi-line strings.
-              const errorIfNotAString = path
-                .get(`properties.${i}.value`)
-                .buildCodeFrameError("Only strings are supported as comments.")
-
-              if (t.isBinaryExpression(value)) {
-                valueToExtract = extractStringContatentation(
-                  t,
-                  value,
-                  errorIfNotAString
-                )
-              } else {
-                throw errorIfNotAString
-              }
+            if (key.name === "comment") {
+              valueToExtract = extractCommentString(t, path, `properties.${i}.value`, value);
             } else if (key.name === "id") {
                 const isIdLiteral = !value.value && t.isTemplateLiteral(value)
                 if (isIdLiteral) {
diff --git a/packages/cli/src/api/catalog.ts b/packages/cli/src/api/catalog.ts
index 4af1ccd6..6a5b6f5f 100644
--- a/packages/cli/src/api/catalog.ts
+++ b/packages/cli/src/api/catalog.ts
@@ -99,8 +99,9 @@ export class Catalog {
     this.format = getFormat(config.format)
   }
 
-  async make(options: MakeOptions) {
+  async make(options: MakeOptions): Promise<boolean> {
     const nextCatalog = await this.collect(options)
+    if (!nextCatalog) return false
     const prevCatalogs = this.readAll()
 
     const catalogs = this.merge(prevCatalogs, nextCatalog, {
@@ -125,18 +126,21 @@ export class Catalog {
     } else {
       this.writeAll(sortedCatalogs)
     }
+    return true
   }
 
-  async makeTemplate(options: MakeTemplateOptions) {
+  async makeTemplate(options: MakeTemplateOptions): Promise<boolean> {
     const catalog = await this.collect(options)
+    if (!catalog) return false
     const sort = order(options.orderBy) as (catalog: CatalogType) => CatalogType
     this.writeTemplate(sort(catalog as CatalogType))
+    return true
   }
 
   /**
    * Collect messages from source paths. Return a raw message catalog as JSON.
    */
-  async collect(options: CollectOptions) {
+  async collect(options: CollectOptions): Promise<CatalogType | undefined> {
     const tmpDir = path.join(os.tmpdir(), `lingui-${process.pid}`)
 
     if (fs.existsSync(tmpDir)) {
@@ -153,15 +157,18 @@ export class Catalog {
         paths = paths.filter((path: string) => regex.test(path))
       }
 
+      let catalogSuccess = true
       for (let filename of paths) {
-        await extract(filename, tmpDir, {
+        const fileSuccess = await extract(filename, tmpDir, {
           verbose: options.verbose,
           configPath: options.configPath,
           babelOptions: this.config.extractBabelOptions,
           extractors: options.extractors,
           projectType: options.projectType,
         })
+        catalogSuccess &&= fileSuccess
       }
+      if (!catalogSuccess) return undefined
 
       return (function traverse(directory) {
         return fs
diff --git a/packages/cli/src/api/extractors/index.ts b/packages/cli/src/api/extractors/index.ts
index 530d5767..849b3b48 100644
--- a/packages/cli/src/api/extractors/index.ts
+++ b/packages/cli/src/api/extractors/index.ts
@@ -45,18 +45,18 @@ export default async function extract(
 
     try {
       await ext.extract(filename, targetPath, options)
+
+      if (options.verbose && spinner) spinner.succeed()
+      return true
     } catch (e) {
       if (options.verbose && spinner) {
         spinner.fail(e.message)
       } else {
         console.error(`Cannot process file ${e.message}`)
       }
-      return true
+      return false
     }
-
-    if (options.verbose && spinner) spinner.succeed()
-    return true
   }
 
-  return false
+  return true
 }
diff --git a/packages/cli/src/lingui-extract.ts b/packages/cli/src/lingui-extract.ts
index 6c86d63c..e7d4d06e 100644
--- a/packages/cli/src/lingui-extract.ts
+++ b/packages/cli/src/lingui-extract.ts
@@ -41,8 +41,9 @@ export default async function command(
 
   const catalogs = getCatalogs(config)
   const catalogStats: { [path: string]: AllCatalogsType } = {}
+  let commandSuccess = true
   for (let catalog of catalogs) {
-    await catalog.make({
+    const catalogSuccess = await catalog.make({
       ...options as CliExtractOptions,
       orderBy: config.orderBy,
       extractors: config.extractors,
@@ -50,6 +51,7 @@ export default async function command(
     })
 
     catalogStats[catalog.path] = catalog.readAll()
+    commandSuccess &&= catalogSuccess
   }
 
   Object.entries(catalogStats).forEach(([key, value]) => {
@@ -80,7 +82,7 @@ export default async function command(
       .catch(err => console.error(`Can't load service module ${moduleName}`, err))
   }
 
-  return true
+  return commandSuccess
 }
 
 if (require.main === module) {
diff --git a/packages/macro/index.d.ts b/packages/macro/index.d.ts
index 298b979d..d95374d5 100644
--- a/packages/macro/index.d.ts
+++ b/packages/macro/index.d.ts
@@ -175,6 +175,7 @@ export type TransProps = {
   children?: React.ReactNode
   component?: React.ComponentType<TransRenderProps>
   render?: (props: TransRenderProps) => ReactElement<any, any> | null
+  i18n?: I18n
 }
 
 export type ChoiceProps = {


Test patch:
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 bbb46d84..da852e90 100644
--- a/packages/babel-plugin-extract-messages/test/__snapshots__/index.ts.snap
+++ b/packages/babel-plugin-extract-messages/test/__snapshots__/index.ts.snap
@@ -123,6 +123,72 @@ Object {
 }
 `;
 
+exports[`@lingui/babel-plugin-extract-messages should extract all messages from JS files (without macros or i18n comments) 1`] = `
+Object {
+  Context1: Object {
+    Some id: Object {
+      extractedComments: Array [],
+      origin: Array [
+        Array [
+          js-without-macros-or-comments.js,
+          11,
+        ],
+      ],
+    },
+  },
+  Description: Object {
+    extractedComments: Array [
+      description,
+    ],
+    origin: Array [
+      Array [
+        js-without-macros-or-comments.js,
+        3,
+      ],
+    ],
+  },
+  ID: Object {
+    extractedComments: Array [],
+    message: Message with id,
+    origin: Array [
+      Array [
+        js-without-macros-or-comments.js,
+        7,
+      ],
+    ],
+  },
+  Message: Object {
+    extractedComments: Array [],
+    origin: Array [
+      Array [
+        js-without-macros-or-comments.js,
+        1,
+      ],
+    ],
+  },
+  Multiline: Object {
+    extractedComments: Array [
+      this is 2 lines long,
+    ],
+    origin: Array [
+      Array [
+        js-without-macros-or-comments.js,
+        5,
+      ],
+    ],
+  },
+  Values {param}: Object {
+    extractedComments: Array [],
+    origin: Array [
+      Array [
+        js-without-macros-or-comments.js,
+        9,
+      ],
+    ],
+  },
+}
+`;
+
 exports[`@lingui/babel-plugin-extract-messages should extract all messages from JS files 1`] = `
 Object {
   Context1: Object {
@@ -321,3 +387,30 @@ Object {
   },
 }
 `;
+
+exports[`@lingui/babel-plugin-extract-messages should handle duplicate ids 1`] = `
+Object {
+  msg: Object {
+    extractedComments: Array [],
+    message: Hello World!,
+    origin: Array [
+      Array [
+        duplicate-id-valid.js,
+        5,
+      ],
+      Array [
+        duplicate-id-valid.js,
+        8,
+      ],
+      Array [
+        duplicate-id-valid.js,
+        11,
+      ],
+      Array [
+        duplicate-id-valid.js,
+        14,
+      ],
+    ],
+  },
+}
+`;
diff --git a/packages/babel-plugin-extract-messages/test/fixtures/js-without-macros-or-comments.js b/packages/babel-plugin-extract-messages/test/fixtures/js-without-macros-or-comments.js
new file mode 100644
index 00000000..e73602cc
--- /dev/null
+++ b/packages/babel-plugin-extract-messages/test/fixtures/js-without-macros-or-comments.js
@@ -0,0 +1,11 @@
+const msg = i18n._('Message')
+
+const withDescription = i18n._('Description', {}, { comment: "description"});
+
+const withMultilineComment = i18n._('Multiline', {}, { comment: "this is " + "2 lines long" });
+
+const withId = i18n._('ID', {}, { message: 'Message with id' });
+
+const withValues = i18n._('Values {param}', { param: param });
+
+const withContext = i18n._('Some id', {},{ context: 'Context1'});
diff --git a/packages/babel-plugin-extract-messages/test/index.ts b/packages/babel-plugin-extract-messages/test/index.ts
index 3a7fc29b..f7db9aab 100644
--- a/packages/babel-plugin-extract-messages/test/index.ts
+++ b/packages/babel-plugin-extract-messages/test/index.ts
@@ -113,6 +113,11 @@ describe("@lingui/babel-plugin-extract-messages", function () {
     ).toBeFalsy()
   })
 
+  testCase(
+    "should handle duplicate ids",
+    "duplicate-id-valid.js"
+  )
+
   testCase(
     "should extract all messages from JSX files",
     "jsx-without-macros.js"
@@ -134,4 +139,9 @@ describe("@lingui/babel-plugin-extract-messages", function () {
     "should extract all messages from JS files (macros)",
     "js-with-macros.js"
   )
+
+  testCase(
+    "should extract all messages from JS files (without macros or i18n comments)",
+    "js-without-macros-or-comments.js"
+  )
 })
diff --git a/packages/cli/src/api/__snapshots__/catalog.test.ts.snap b/packages/cli/src/api/__snapshots__/catalog.test.ts.snap
index 7a19a761..69d5e117 100644
--- a/packages/cli/src/api/__snapshots__/catalog.test.ts.snap
+++ b/packages/cli/src/api/__snapshots__/catalog.test.ts.snap
@@ -93,7 +93,7 @@ Object {
 }
 `;
 
-exports[`Catalog collect should handle errors 1`] = `Object {}`;
+exports[`Catalog collect should handle errors 1`] = `undefined`;
 
 exports[`Catalog make should collect and write catalogs 1`] = `
 Object {