Repository: lingui/js-lingui
Pull request #1296 (2022-10-28 13:08:01)

feat: Support GetText template files support during compile
This PR brings full e2e support for gettext *.pot files into linguijs. 

## What is a template or *.pot file? 
Template is a file where all messages extracted from sourcecode is stored. It doesn't have any translations. It treated as build artifact and could be added to gitignore. 

## What's wrong with current support of *.pot files? 
Short version: 
They are not taken into account during compiling messages.

Long version:
Standard gettext flow looks like the following
- Change source code and potentially change messages
- Run extractor and produce *.pot file
- Send *.pot file together with translations (en.po, pl.po) to the translators. 
- Translators upload .pot and *.po files into theirs translation software which merges them
- Translators translate
- Translators return translated files. 

There are few things which important to notice. 
Merging new messages to the translations should happen outside of build process. It's the responsibility of translation software (such as PoEditor or modern cloud services like crowdin) to do so, not the linguijs extractor.

In current linguijs flow, extractor tries to merge new messages (and delete old) into translation files. 

What's wrong with this? Translations files are usually under version control. Running a build with NODE_ENV=production without fresh extracted messages will cause that messages where ICU is used would be corrupted and this force developers to always extract messages before build in CI process (for test/staging builds, for production builds we assume that 100% messages are translated).

Every time we run a development build on CI, our translation files marked as changed. 

This might not be a big deal for regular repositories, but could be a dealbreaker for monorepos where some strategies for incremental builds are used. Usually tools such as turborepo / rush / ultra-runner trying to be smart and detect which project inside monorepo was changed and rebuild only this one. With linguijs this doesn't work because every build touches translations and therefore bursting the cache. 

But lingui has a command to `extract-template`, you can use this, you may say.  Actually not.

What happend if we generate template file using `lingui extract-template`? 
- The template would be generated and translations files would not be touched. That's good. 
- You build your application and realize that all plurals expressions doesn't work because they was not compiled because they are not in the translation files - That sucks. 
- Your understand that in current stage, extracting *.pot files are useless in linguijs. 

## How this PR changes it? 
Instead of merging new messages into catalogs during extraction phase, it merges template with requested catalog during compile time. If you have actual template - your translations catalogs are also actual.



codecov[bot]: # [Codecov](https://codecov.io/gh/lingui/js-lingui/pull/1296?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=lingui) Report
Base: **81.67**% // Head: **82.14**% // Increases project coverage by **`+0.46%`** :tada:
> Coverage data is based on head [(`794d270`)](https://codecov.io/gh/lingui/js-lingui/pull/1296?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=lingui) compared to base [(`da070ba`)](https://codecov.io/gh/lingui/js-lingui/commit/da070ba294435484fde10d82ecb895d2410ec1b7?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=lingui).
> Patch coverage: 81.81% of modified lines in pull request are covered.

<details><summary>Additional details and impacted files</summary>


```diff
@@            Coverage Diff             @@
##             main    #1296      +/-   ##
==========================================
+ Coverage   81.67%   82.14%   +0.46%     
==========================================
  Files          56       56              
  Lines        1779     1781       +2     
  Branches      495      498       +3     
==========================================
+ Hits         1453     1463      +10     
+ Misses        196      188       -8     
  Partials      130      130              
```


| [Impacted Files](https://codecov.io/gh/lingui/js-lingui/pull/1296?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=lingui) | Coverage Δ | |
|---|---|---|
| [packages/cli/src/api/catalog.ts](https://codecov.io/gh/lingui/js-lingui/pull/1296/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=lingui#diff-cGFja2FnZXMvY2xpL3NyYy9hcGkvY2F0YWxvZy50cw==) | `85.38% <81.81%> (+3.82%)` | :arrow_up: |

Help us with your feedback. Take ten seconds to tell us [how you rate us](https://about.codecov.io/nps?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=lingui). Have a feature suggestion? [Share it here.](https://app.codecov.io/gh/feedback/?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=lingui)

</details>

[:umbrella: View full report at Codecov](https://codecov.io/gh/lingui/js-lingui/pull/1296?src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=lingui).   
:loudspeaker: Do you have feedback about the report comment? [Let us know in this issue](https://about.codecov.io/codecov-pr-comment-feedback/?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=lingui).


Patch:
diff --git a/packages/cli/src/api/catalog.ts b/packages/cli/src/api/catalog.ts
index 3e13f7eb..4af1ccd6 100644
--- a/packages/cli/src/api/catalog.ts
+++ b/packages/cli/src/api/catalog.ts
@@ -31,6 +31,7 @@ export type ExtractedMessageType = {
   obsolete?: boolean
   flags?: string[]
   context?: string
+  defaults?: string
 }
 
 export type MessageType = ExtractedMessageType & {
@@ -254,50 +255,55 @@ export class Catalog {
 
   getTranslations(locale: string, options: GetTranslationsOptions) {
     const catalogs = this.readAll()
+    const template = this.readTemplate() || {}
+
+
     return R.mapObjIndexed(
       (_value, key) => this.getTranslation(catalogs, locale, key, options),
-      catalogs[locale]
+      { ...template, ...catalogs[locale] },
     )
   }
 
   getTranslation(
-    catalogs: Object,
+    catalogs: AllCatalogsType,
     locale: string,
     key: string,
     { fallbackLocales, sourceLocale }: GetTranslationsOptions
   ) {
-    if (!catalogs[locale].hasOwnProperty(key)) {
+    const catalog = catalogs[locale] || {}
+
+    if (!catalog.hasOwnProperty(key)) {
       console.error(`Message with key ${key} is missing in locale ${locale}`)
     }
 
-    const getTranslation = (locale) => {
+    const getTranslation = (_locale: string) => {
       const configLocales = this.config.locales.join('", "')
-      const localeCatalog = catalogs[locale]
+      const localeCatalog = catalogs[_locale] || {}
 
       if (!localeCatalog) {
         console.warn(`
-        Catalog "${locale}" isn't present in locales config parameter
-        Add "${locale}" to your lingui.config.js:
+        Catalog "${_locale}" isn't present in locales config parameter
+        Add "${_locale}" to your lingui.config.js:
         {
-          locales: ["${configLocales}", "${locale}"]
+          locales: ["${configLocales}", "${_locale}"]
         }
       `)
         return null
       }
       if (!localeCatalog.hasOwnProperty(key)) {
-        console.error(`Message with key ${key} is missing in locale ${locale}`)
+        console.error(`Message with key ${key} is missing in locale ${_locale}`)
         return null
       }
 
-      if (catalogs[locale]) {
-        return catalogs[locale][key].translation
+      if (catalogs[_locale]) {
+        return catalogs[_locale][key].translation
       }
 
       return null
     }
 
-    const getMultipleFallbacks = (locale) => {
-      const fL = fallbackLocales && fallbackLocales[locale]
+    const getMultipleFallbacks = (_locale: string) => {
+      const fL = fallbackLocales && fallbackLocales[_locale]
 
       // some probably the fallback will be undefined, so just search by locale
       if (!fL) return null
@@ -319,16 +325,13 @@ export class Catalog {
       // We search in fallbackLocales as dependent of each locale
       getMultipleFallbacks(locale) ||
       // Get translation in fallbackLocales.default (if any)
-      (fallbackLocales &&
-        fallbackLocales.default &&
-        getTranslation(fallbackLocales.default)) ||
+      (fallbackLocales?.default && getTranslation(fallbackLocales.default as string)) ||
       // Get message default
-      catalogs[locale][key].defaults ||
+      catalog[key]?.defaults ||
       // If sourceLocale is either target locale of fallback one, use key
       (sourceLocale && sourceLocale === locale && key) ||
       (sourceLocale &&
-        fallbackLocales &&
-        fallbackLocales.default &&
+        fallbackLocales?.default &&
         sourceLocale === fallbackLocales.default &&
         key) ||
       // Otherwise no translation is available
@@ -398,7 +401,7 @@ export class Catalog {
     return this.format.read(filename)
   }
 
-  readAll() {
+  readAll(): AllCatalogsType {
     return R.mergeAll(
       this.locales.map((locale) => ({
         [locale]: this.read(locale),
@@ -406,7 +409,7 @@ export class Catalog {
     ) as AllCatalogsType
   }
 
-  readTemplate() {
+  readTemplate(): CatalogType {
     const filename = this.templateFile
     if (!fs.existsSync(filename)) return null
     return this.format.read(filename)
@@ -456,9 +459,9 @@ export class Catalog {
 /**
  * Parse `config.catalogs` and return a list of configured Catalog instances.
  */
-export function getCatalogs(config: LinguiConfig) {
+export function getCatalogs(config: LinguiConfig): Catalog[] {
   const catalogsConfig = config.catalogs
-  const catalogs = []
+  const catalogs: Catalog[] = []
 
   catalogsConfig.forEach((catalog) => {
     // Validate that `catalogPath` doesn't end with trailing slash
diff --git a/packages/cli/src/api/fixtures/pot-template/messages.pot b/packages/cli/src/api/fixtures/pot-template/messages.pot
new file mode 100644
index 00000000..d2a32f73
--- /dev/null
+++ b/packages/cli/src/api/fixtures/pot-template/messages.pot
@@ -0,0 +1,5 @@
+msgid "Hello World"
+msgstr ""
+
+msgid "Test String"
+msgstr ""
diff --git a/packages/cli/src/api/fixtures/pot-template/pl.po b/packages/cli/src/api/fixtures/pot-template/pl.po
new file mode 100644
index 00000000..67adfd62
--- /dev/null
+++ b/packages/cli/src/api/fixtures/pot-template/pl.po
@@ -0,0 +1,2 @@
+msgid "Hello World"
+msgstr "Cześć świat"
diff --git a/packages/cli/src/lingui-extract-template.ts b/packages/cli/src/lingui-extract-template.ts
index f4ebf153..4f0fd4cf 100644
--- a/packages/cli/src/lingui-extract-template.ts
+++ b/packages/cli/src/lingui-extract-template.ts
@@ -35,7 +35,7 @@ export default async function command(
 
   await Promise.all(catalogs.map(async (catalog) => {
     await catalog.makeTemplate({
-      ...options,
+      ...options as CliExtractTemplateOptions,
       orderBy: config.orderBy,
       projectType: detect(),
     })
diff --git a/packages/cli/src/lingui-extract.ts b/packages/cli/src/lingui-extract.ts
index 949ce254..6c86d63c 100644
--- a/packages/cli/src/lingui-extract.ts
+++ b/packages/cli/src/lingui-extract.ts
@@ -43,7 +43,7 @@ export default async function command(
   const catalogStats: { [path: string]: AllCatalogsType } = {}
   for (let catalog of catalogs) {
     await catalog.make({
-      ...options,
+      ...options as CliExtractOptions,
       orderBy: config.orderBy,
       extractors: config.extractors,
       projectType: detect(),
diff --git a/packages/loader/package.json b/packages/loader/package.json
index 581423b5..b223f684 100644
--- a/packages/loader/package.json
+++ b/packages/loader/package.json
@@ -33,8 +33,7 @@
     "@babel/runtime": "^7.11.2",
     "@lingui/cli": "^3.14.0",
     "@lingui/conf": "^3.14.0",
-    "loader-utils": "^2.0.0",
-    "ramda": "^0.27.1"
+    "loader-utils": "^2.0.0"
   },
   "devDependencies": {
     "memory-fs": "^0.5.0"
diff --git a/packages/loader/src/webpackLoader.ts b/packages/loader/src/webpackLoader.ts
index b6cf178c..74160c79 100644
--- a/packages/loader/src/webpackLoader.ts
+++ b/packages/loader/src/webpackLoader.ts
@@ -1,5 +1,4 @@
 import path from "path"
-import * as R from "ramda"
 import { getConfig } from "@lingui/conf"
 import {
   createCompiledCatalog,
@@ -61,15 +60,11 @@ export default function (source) {
     catalogRelativePath,
     getCatalogs(config)
   )
-  const catalogs = catalog.readAll()
-  const messages = R.mapObjIndexed(
-    (_, key) =>
-      catalog.getTranslation(catalogs, locale, key, {
-        fallbackLocales: config.fallbackLocales,
-        sourceLocale: config.sourceLocale,
-      }),
-    catalogs[locale]
-  )
+
+  const messages = catalog.getTranslations(locale, {
+    fallbackLocales: config.fallbackLocales,
+    sourceLocale: config.sourceLocale,
+  })
 
   // In production we don't want untranslated strings. It's better to use message
   // keys as a last resort.
diff --git a/packages/macro/src/index.ts b/packages/macro/src/index.ts
index ed638e1b..11e7808c 100644
--- a/packages/macro/src/index.ts
+++ b/packages/macro/src/index.ts
@@ -32,6 +32,16 @@ function macro({ references, state, babel }) {
   const jsNodes = []
   let needsI18nImport = false
 
+  const alreadyVisitedCache = new WeakSet()
+  const alreadyVisited = (path) => {
+    if (alreadyVisitedCache.has(path)) {
+      return true
+    } else {
+      alreadyVisitedCache.add(path)
+      return false
+    }
+  }
+
   Object.keys(references).forEach((tagName) => {
     const nodes = references[tagName]
     const macroType = getMacroType(tagName)
@@ -121,17 +131,6 @@ function isRootPath(allPath) {
     })(node)
 }
 
-const alreadyVisitedCache = []
-
-function alreadyVisited(path) {
-  if (alreadyVisitedCache.includes(path)) {
-    return true
-  } else {
-    alreadyVisitedCache.push(path)
-    return false
-  }
-}
-
 function getMacroType(tagName) {
   switch (tagName) {
     case "defineMessage":


Test patch:
diff --git a/packages/cli/src/api/__snapshots__/catalog.test.ts.snap b/packages/cli/src/api/__snapshots__/catalog.test.ts.snap
index 26224950..7a19a761 100644
--- a/packages/cli/src/api/__snapshots__/catalog.test.ts.snap
+++ b/packages/cli/src/api/__snapshots__/catalog.test.ts.snap
@@ -1,5 +1,19 @@
 // Jest Snapshot v1, https://goo.gl/fbAQLP
 
+exports[`Catalog POT Flow Should get translations from template if locale file not presented 1`] = `
+Object {
+  Hello World: Hello World,
+  Test String: Test String,
+}
+`;
+
+exports[`Catalog POT Flow Should merge source messages from template if provided 1`] = `
+Object {
+  Hello World: Cześć świat,
+  Test String: Test String,
+}
+`;
+
 exports[`Catalog collect should extract messages from source files 1`] = `
 Object {
   Component A: Object {
diff --git a/packages/cli/src/api/catalog.test.ts b/packages/cli/src/api/catalog.test.ts
index 2f58cbe4..1ad7e38f 100644
--- a/packages/cli/src/api/catalog.test.ts
+++ b/packages/cli/src/api/catalog.test.ts
@@ -131,6 +131,61 @@ describe("Catalog", function () {
     })
   })
 
+  describe("POT Flow", function () {
+    it('Should merge source messages from template if provided', () => {
+      const catalog = new Catalog(
+        {
+          name: "messages",
+          path: path.resolve(
+            __dirname,
+            path.join("fixtures", "pot-template", "{locale}")
+          ),
+          include: [],
+          exclude: [],
+        },
+        mockConfig({
+          locales: ['en', 'pl'],
+        })
+      )
+
+      const translations = catalog.getTranslations('pl', {
+        sourceLocale: 'en',
+        fallbackLocales: {
+          default: 'en'
+        }
+      });
+
+      expect(translations).toMatchSnapshot()
+    })
+
+    it('Should get translations from template if locale file not presented', () => {
+      const catalog = new Catalog(
+        {
+          name: "messages",
+          path: path.resolve(
+            __dirname,
+            path.join("fixtures", "pot-template", "{locale}")
+          ),
+          include: [],
+          exclude: [],
+        },
+        mockConfig({
+          locales: ['en', 'pl'],
+        })
+      )
+
+      const translations = catalog.getTranslations('en', {
+        sourceLocale: 'en',
+        fallbackLocales: {
+          default: 'en'
+        }
+      });
+
+      console.log(translations);
+      expect(translations).toMatchSnapshot()
+    })
+  })
+
   describe("collect", function () {
     it("should extract messages from source files", async function () {
       const catalog = new Catalog(