feat(macro): support JSX macro inside conditional expressions #1436

thekip · 2023-02-14T15:11:47Z

Description

This PR adding syntax support for ConditionalExpressions in JSX Macro:

import { Trans } from '@lingui/macro'
<Trans>Hello, {props.world ? <Trans>world</Trans> : <Trans>guys</Trans>}</Trans>

To make reviewing easier, i split changes into 2 commits.

First commit refactoring JSX macro to operate with babel's path instead of node
Second commit fix actual issue.

We need path because babel doesn't allow traversing over plain node. And manual traversing wasn't a case here because there might be a syntax which we didn't expect. (multiple nested ternaries or function call which accepts jsx node, etc)

Also issue in SWC version repo: lingui/swc-plugin#16

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

fixes: #1175

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-14T15:12:24Z

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 15, 2023 at 10:39AM (UTC)

github-actions · 2023-02-14T15:14:20Z

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%)

packages/macro/src/macroJsx.test.ts

codecov · 2023-02-14T15:22:34Z

Codecov Report

Base: 70.92% // Head: 71.02% // Increases project coverage by +0.09% 🎉

Coverage data is based on head (3a00d7a) compared to base (77d1c08).
Patch coverage: 89.15% of modified lines in pull request are covered.

❗ Current head 3a00d7a differs from pull request most recent head b24690d. Consider uploading reports for the commit b24690d to get more accurate results

Additional details and impacted files

@@            Coverage Diff             @@
##             next    #1436      +/-   ##
==========================================
+ Coverage   70.92%   71.02%   +0.09%     
==========================================
  Files          75       75              
  Lines        1940     1943       +3     
  Branches      525      526       +1     
==========================================
+ Hits         1376     1380       +4     
+ Misses        452      451       -1     
  Partials      112      112

Impacted Files	Coverage Δ
packages/macro/test/jsx-trans.ts	`100.00% <ø> (ø)`
packages/macro/src/macroJsx.ts	`92.10% <87.83%> (+0.49%)`	⬆️
packages/macro/src/index.ts	`83.92% <100.00%> (+0.05%)`	⬆️

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.

packages/macro/src/macroJsx.ts

thekip · 2023-02-15T07:49:07Z

packages/macro/test/jsx-trans.ts

+        ;<Trans
+            id={"Hello, {0}"}
+            values={{
+              0: props.world ? <Trans id={'world'} /> : <Trans id={'guys'} />
+            }}
+          />


I'm thinking that we can automatically generate from 1 level ternary an ICU select expression:

Hello, { props.world, select, =true {world} =false {guys} }

Or i'm overengineering...

Select needs to always have the other case, so I don't really see a way how to do that.

Hello, { props.world, select, =true {world} other {guys} }

But it's overengineering anyway

thekip · 2023-02-15T07:50:14Z

packages/macro/tsconfig.json

+    "lib": [
+      "ES2019.Array"
+    ],


This is for Array.prototype.flat / Array.prototype.flatMap
Supported started from Node.js 11 AFAIK

thekip · 2023-02-15T07:51:26Z

packages/macro/src/macroJsx.test.ts

+          one: [
+            {
+              type: "arg",
              name: "gender",
-              type: "Identifier",
-            }),
-            format: "select",
-            options: {
-              male: "he",
-              female: "she",
-              other: "they",
+              value: expect.objectContaining({
+                name: "gender",
+                type: "Identifier",
+              }),
+              format: "select",
+              options: {
+                male: "he",
+                female: "she",
+                other: "they",
+              },
            },
-          },
+          ],


Was a single element - become an array

packages/macro/src/index.ts

fixes: lingui#1175

…ences from babel-macro-plugin

* refactor(macro): pass down and manipulate with `path` instead of `node` * feature(macro): support JSX macro inside conditional expressions * refactor(macro): remove alreadyVisitedCache in favor of dedupe references from babel-macro-plugin

thekip requested review from Martin005 and andrii-bodnar February 14, 2023 15:11

vercel bot deployed to Preview February 14, 2023 15:13 View deployment

thekip force-pushed the fix/trans-macro-conditional-expression branch from b34528b to dad400c Compare February 14, 2023 15:18

thekip changed the title ~~Fix/trans macro conditional expression~~ feature(macro): support JSX macro inside conditional expressions Feb 14, 2023

thekip marked this pull request as ready for review February 14, 2023 15:18

vercel bot deployed to Preview February 14, 2023 15:19 View deployment

thekip commented Feb 14, 2023

View reviewed changes

packages/macro/src/macroJsx.test.ts Outdated Show resolved Hide resolved

thekip force-pushed the fix/trans-macro-conditional-expression branch from dad400c to 2555690 Compare February 14, 2023 15:22

vercel bot deployed to Preview February 14, 2023 15:24 View deployment

thekip changed the title ~~feature(macro): support JSX macro inside conditional expressions~~ feat(macro): support JSX macro inside conditional expressions Feb 14, 2023

thekip mentioned this pull request Feb 14, 2023

Trans macro inside ConditionalExpressions are not processed lingui/swc-plugin#16

Open

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

JSX macros aren't working inside JSX expressions in JSX macros #1175

Closed

Martin005 reviewed Feb 14, 2023

View reviewed changes

packages/macro/src/macroJsx.ts Outdated Show resolved Hide resolved

thekip force-pushed the fix/trans-macro-conditional-expression branch from 2555690 to fbdcc68 Compare February 14, 2023 18:03

vercel bot deployed to Preview February 14, 2023 18:06 View deployment

thekip force-pushed the fix/trans-macro-conditional-expression branch from fbdcc68 to 69c9278 Compare February 14, 2023 18:15

vercel bot deployed to Preview February 14, 2023 18:17 View deployment

thekip commented Feb 15, 2023

View reviewed changes

vercel bot deployed to Preview February 15, 2023 08:06 View deployment

andrii-bodnar added the v4 label Feb 15, 2023

Martin005 approved these changes Feb 15, 2023

View reviewed changes

andrii-bodnar reviewed Feb 15, 2023

View reviewed changes

packages/macro/src/index.ts Outdated Show resolved Hide resolved

thekip added 2 commits February 15, 2023 11:37

refactor(macro): pass down and manipulate with path instead of node

612aed7

feature(macro): support JSX macro inside conditional expressions

24d3258

fixes: lingui#1175

refactor(macro): remove alreadyVisitedCache in favour of dedupe refer…

b24690d

…ences from babel-macro-plugin

thekip force-pushed the fix/trans-macro-conditional-expression branch from 3a00d7a to b24690d Compare February 15, 2023 10:37

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

andrii-bodnar approved these changes Feb 15, 2023

View reviewed changes

andrii-bodnar merged commit 44f8360 into lingui:next Feb 15, 2023

thekip deleted the fix/trans-macro-conditional-expression branch February 15, 2023 12:34

thekip mentioned this pull request Feb 15, 2023

JSX macros aren't working inside JSX expressions in JSX macros #1175

Closed

thekip mentioned this pull request Jan 23, 2024

Lingui doesn't extract msg within a msg #1841

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat(macro): support JSX macro inside conditional expressions #1436

feat(macro): support JSX macro inside conditional expressions #1436

thekip commented Feb 14, 2023 •

edited

Loading

vercel bot commented Feb 14, 2023 •

edited

Loading

github-actions bot commented Feb 14, 2023 •

edited

Loading

codecov bot commented Feb 14, 2023 •

edited

Loading

thekip Feb 15, 2023

Martin005 Feb 15, 2023

thekip Feb 15, 2023

thekip Feb 15, 2023

thekip Feb 15, 2023

feat(macro): support JSX macro inside conditional expressions #1436

feat(macro): support JSX macro inside conditional expressions #1436

Conversation

thekip commented Feb 14, 2023 • edited Loading

Description

Types of changes

Checklist

vercel bot commented Feb 14, 2023 • edited Loading

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

size-limit report 📦

codecov bot commented Feb 14, 2023 • edited Loading

Codecov Report

thekip Feb 15, 2023

Choose a reason for hiding this comment

Martin005 Feb 15, 2023

Choose a reason for hiding this comment

thekip Feb 15, 2023

Choose a reason for hiding this comment

thekip Feb 15, 2023

Choose a reason for hiding this comment

thekip Feb 15, 2023

Choose a reason for hiding this comment

thekip commented Feb 14, 2023 •

edited

Loading

vercel bot commented Feb 14, 2023 •

edited

Loading

github-actions bot commented Feb 14, 2023 •

edited

Loading

codecov bot commented Feb 14, 2023 •

edited

Loading