feat(cli): Extract - Flatten ICU messages #1431

Martin005 · 2023-02-12T11:29:44Z

Description

Based on suggestion from #1186, this PR adds flattening of ICU messages for the extract and extract-template CLI commands.

Types of changes

Bugfix (non-breaking change which fixes an issue)
New feature (non-breaking change which adds functionality)
Breaking change (fix or feature that would cause existing functionality to not work as expected)
Documentation update

Checklist

I have read the CONTRIBUTING and CODE_OF_CONDUCT docs
I have added tests that prove my fix is effective or that my feature works
I have added necessary documentation (if appropriate)

vercel · 2023-02-12T11:29:48Z

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name	Status	Preview	Comments	Updated
js-lingui	✅ Ready (Inspect)	Visit Preview	💬 Add your feedback	Feb 12, 2023 at 0:10AM (UTC)

github-actions · 2023-02-12T11:31:35Z

size-limit report 📦

Path	Size
./packages/core/build/esm/index.js	1.76 KB (0%)
./packages/detect-locale/build/esm/index.js	812 B (0%)
./packages/react/build/esm/index.js	1.79 KB (0%)
./packages/remote-loader/build/esm/index.js	7.29 KB (0%)

codecov · 2023-02-12T11:43:19Z

Codecov Report

Base: 69.24% // Head: 70.01% // Increases project coverage by +0.77% 🎉

Coverage data is based on head (4e4a4ce) compared to base (d4a423d).
Patch coverage: 95.52% of modified lines in pull request are covered.

Additional details and impacted files

@@            Coverage Diff             @@
##             next    #1431      +/-   ##
==========================================
+ Coverage   69.24%   70.01%   +0.77%     
==========================================
  Files          71       73       +2     
  Lines        2019     2081      +62     
  Branches      542      561      +19     
==========================================
+ Hits         1398     1457      +59     
- Misses        499      501       +2     
- Partials      122      123       +1

Impacted Files	Coverage Δ
packages/cli/src/lingui-extract-template.ts	`0.00% <0.00%> (ø)`
packages/cli/src/lingui-extract.ts	`0.00% <0.00%> (ø)`
packages/cli/src/tests.ts	`92.30% <ø> (ø)`
packages/core/src/flatten/printICU.ts	`97.05% <97.05%> (ø)`
packages/cli/src/api/catalog.ts	`88.07% <100.00%> (+0.11%)`	⬆️
packages/core/src/flatten/flattenMessage.ts	`100.00% <100.00%> (ø)`

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

☔ View full report at Codecov.
📢 Do you have feedback about the report comment? Let us know in this issue.

timofei-iatsenko

How this supposed to work in general? When the message would be flattened, it would be a different string. And it would not match to id string from runtime.

timofei-iatsenko · 2023-02-13T07:48:31Z

jest.config.integration.js

-  // Redirect imports to the compiled bundles
-  moduleNameMapper: {},


Without this, integration test become absolutely the same to the "regular" tests. The idea was to check that code works from ./build instead of ./src

What was wrong with that?

timofei-iatsenko · 2023-02-13T07:51:22Z

packages/cli/src/api/catalog.ts

@@ -20,6 +20,7 @@ import extract, { ExtractedMessage } from "./extractors"
 import { CliExtractOptions } from "../lingui-extract"
 import { CliExtractTemplateOptions } from "../lingui-extract-template"
 import { CompiledCatalogNamespace } from "./compile"
+import { flattenMessage } from "@lingui/core/flatten"


Why it's in @lingui/core? Is it supposed to be used on runtime?

timofei-iatsenko · 2023-02-13T07:55:00Z

packages/core/src/flatten/flattenMessage.ts

+function cloneDeep<T>(obj: T): T {
+  if (Array.isArray(obj)) {
+    // @ts-expect-error
+    return [...obj.map(cloneDeep)]
+  }
+  if (obj !== null && typeof obj === "object") {
+    // @ts-expect-error
+    return Object.keys(obj).reduce((cloned, k) => {
+      cloned[k] = cloneDeep(obj[k])
+      return cloned
+    }, {})
+  }
+  return obj


Please, avoid @ts-expect-error

For clone deep you can use simpler JSON.parse(JSON.stringify()) but, better is avoiding mutating data in flattenTokens

timofei-iatsenko · 2023-02-13T07:57:21Z

packages/core/src/flatten/flattenMessage.ts

+  for (let i = 0; i < tokens.length; i++) {
+    const token = tokens[i]
+    if (isPluralOrSelect(token)) {
+      const cloned = cloneDeep(token)
+      const { cases } = cloned
+      cloned.cases = cases.reduce((all, k, index) => {
+        const newValue = flattenTokens([
+          ...tokens.slice(0, i),
+          ...cases[index].tokens,
+          ...tokens.slice(i + 1),
+        ])
+        all[index] = { ...k, tokens: newValue }
+        return all
+      }, [])
+
+      return [cloned]
+    }
+  }
+  return tokens


Why not a usual forEach()?

tokens.forEach((token, index) => { /* … */ })

timofei-iatsenko · 2023-02-13T08:47:58Z

packages/core/src/flatten/flattenMessage.ts

+      const cloned = cloneDeep(token)
+      const { cases } = cloned
+      cloned.cases = cases.reduce((all, k, index) => {
+        const newValue = flattenTokens([
+          ...tokens.slice(0, i),
+          ...cases[index].tokens,
+          ...tokens.slice(i + 1),
+        ])
+        all[index] = { ...k, tokens: newValue }
+        return all
+      }, [])
+
+      return [cloned]


Without reduce, mutating data and cloneDeep:

return [{ ...token, cases: token.cases.map((k, index) => { const newValue = flattenTokens([ ...tokens.slice(0, i), ...cases[index].tokens, ...tokens.slice(i + 1) ]) return {...k, tokens: newValue} }) }]

timofei-iatsenko · 2023-02-13T15:24:36Z

@Martin005

How this supposed to work in general? When the message would be flattened, it would be a different string. And it would not match to id string from runtime.

I see here a couple ways how this could be done:

This logic should be applied in the macro and not in extractor. This way macro will generate flatten messages, left them in the bundle where extractor would pick them up. But we should consider SWC version of macro, and porting these changes to the SWC would be hard (because it depends on @message-parser). So i would not recommend this option.
Implement it together with hash based id proposal. This way a hash would be generated by macro from the original, untouched message. And extractor with --flat would process only "messages" not ids. The only one problem here - this will not work for users who not used macro and not use explicit ids. Actually, for v4 we can force explicit ids to be used for non-macro usage.

Martin005 · 2023-02-14T13:53:41Z

@thekip I have been thinking about this and I believe that the second option would be the best way to go.
Looking at formatjs, if you don't specify id, it will throw an Error. But in their babel config, you can specify that an id can be generated with a specified pattern.
This makes a lot of sense for me. What do you think?

timofei-iatsenko · 2023-02-14T14:07:31Z

Yes, this is exactly how i'm see implementation of hash-based ids with macro.

Two cases:

Without macro, i18n._() call, id is required. Message would not be used as substitution.
Macro t calls will generate hash based ids from message if explicit ids is not specified, and this behavior would be enabled as default.

feat(cli): Extract - Flatten ICU messages

dfa86e3

Martin005 requested review from timofei-iatsenko and andrii-bodnar February 12, 2023 11:29

Martin005 changed the title ~~Merge pull request #1399 from lingui/main~~ feat(cli): Extract - Flatten ICU messages Feb 12, 2023

Fix lint issues

4e4a4ce

Martin005 linked an issue Feb 12, 2023 that may be closed by this pull request

Suggestion: option to flatten ICU Messages #1186

Closed

vercel bot deployed to Preview February 12, 2023 12:10 View deployment

andrii-bodnar approved these changes Feb 13, 2023

View reviewed changes

timofei-iatsenko requested changes Feb 13, 2023

View reviewed changes

timofei-iatsenko reviewed Feb 13, 2023

View reviewed changes

andrii-bodnar modified the milestone: v4 Feb 13, 2023

Martin005 marked this pull request as draft February 14, 2023 14:14

andrii-bodnar added the v4 label Feb 15, 2023

timofei-iatsenko mentioned this pull request Feb 17, 2023

feat: Hash based id #1440

Merged

14 tasks

andrii-bodnar force-pushed the next branch 2 times, most recently from 1b9469e to 4aa80de Compare February 20, 2023 12:33

andrii-bodnar removed the v4 label Feb 24, 2023

andrii-bodnar closed this Mar 1, 2023

andrii-bodnar deleted the feat/cli-extract-flatten-icu branch April 26, 2023 09:15

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat(cli): Extract - Flatten ICU messages #1431

feat(cli): Extract - Flatten ICU messages #1431

Martin005 commented Feb 12, 2023 •

edited

Loading

vercel bot commented Feb 12, 2023 •

edited

Loading

github-actions bot commented Feb 12, 2023 •

edited

Loading

codecov bot commented Feb 12, 2023 •

edited

Loading

timofei-iatsenko left a comment

timofei-iatsenko Feb 13, 2023

timofei-iatsenko Feb 13, 2023

timofei-iatsenko Feb 13, 2023

timofei-iatsenko Feb 13, 2023

timofei-iatsenko Feb 13, 2023

timofei-iatsenko commented Feb 13, 2023

Martin005 commented Feb 14, 2023 •

edited

Loading

timofei-iatsenko commented Feb 14, 2023

		// Redirect imports to the compiled bundles
		moduleNameMapper: {},

feat(cli): Extract - Flatten ICU messages #1431

feat(cli): Extract - Flatten ICU messages #1431

Conversation

Martin005 commented Feb 12, 2023 • edited Loading

Description

Types of changes

Checklist

vercel bot commented Feb 12, 2023 • edited Loading

github-actions bot commented Feb 12, 2023 • edited Loading

size-limit report 📦

codecov bot commented Feb 12, 2023 • edited Loading

Codecov Report

timofei-iatsenko left a comment

Choose a reason for hiding this comment

timofei-iatsenko Feb 13, 2023

Choose a reason for hiding this comment

timofei-iatsenko Feb 13, 2023

Choose a reason for hiding this comment

timofei-iatsenko Feb 13, 2023

Choose a reason for hiding this comment

timofei-iatsenko Feb 13, 2023

Choose a reason for hiding this comment

timofei-iatsenko Feb 13, 2023

Choose a reason for hiding this comment

timofei-iatsenko commented Feb 13, 2023

Martin005 commented Feb 14, 2023 • edited Loading

timofei-iatsenko commented Feb 14, 2023

Martin005 commented Feb 12, 2023 •

edited

Loading

vercel bot commented Feb 12, 2023 •

edited

Loading

github-actions bot commented Feb 12, 2023 •

edited

Loading

codecov bot commented Feb 12, 2023 •

edited

Loading

Martin005 commented Feb 14, 2023 •

edited

Loading