Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

macro(babel) : There is a problem with transforming the embedded(nested) t-function #1745

Closed
T9-Forever opened this issue Aug 17, 2023 · 9 comments

Comments

@T9-Forever
Copy link

Describe the bug
macro(babel) : There is a problem with transforming the embedded t-function

To Reproduce
Steps to reproduce the behavior, possibly with minimal code sample, e.g:

import { t as $t } from "@lingui/macro";

const a = $t`wrapper ${$t`nested`}`

const b = $t({
  message: `wrapper ${$t`nested`}`,
})

In babel-macro, it will be transformed

import { i18n } from "@lingui/core";
      const a = i18n._(
      /*i18n*/
      {
        id: "rqkBxe",
        message: "wrapper {0}",
        values: {
          0: $t`nested`
        }
      });
      const b = i18n._(
      /*i18n*/
      {
        values: {
          0: $t`nested`
        },
        message: "wrapper {0}",
        id: "rqkBxe"
      });

Expected behavior
This is the result of the macro-swc-plugin conversion (and also the code for the target)

const a = i18n._({
    id: "rqkBxe",
    message: "wrapper {0}",
    values: {
        0: i18n._({
            id: "lV+268",
            message: "nested"
        })
    }
});
const b = i18n._({
    id: "rqkBxe",
    message: "wrapper {0}",
    values: {
        0: i18n._({
            id: "lV+268",
            message: "nested"
        })
    }
});

As you can see, the swc macro plugin and the babel macro plugin transformations don't match, and babel is missing the embedded t function transformation

Additional context
Add any other context about the problem here.

  • jsLingui version lingui --version 4.3.0
  • Babel version npm list @babel/core 7.21.4
  • Macro support:
  • Project using @lingui/swc-plugin
  • Cli(for extract) using babel-macro-plugin
  • Your Babel config (e.g. .babelrc) or framework you use (Create React App, NextJs, Vite)
@T9-Forever
Copy link
Author

I pulled the js-lingui (packages/macro) repository locally, and used jest to test my case, and it was also wrong.

testFilePath: packages/macro/test/test.tsx:

import { t as $t } from "@lingui/macro";

const a = $t`wrapper ${$t`nested`}`

const b = $t({
  message: `wrapper ${$t`nested`}`,
})

Transformed code(in console):

import { i18n } from "@lingui/core";

const a = i18n._(
 /*i18n*/
 {
   id: "rqkBxe",
   message: "wrapper {0}",
   values: {
     0: $t`nested`
   }
});
 
const b = i18n._(
 /*i18n*/
 {
   values: {
     0: $t`nested`
   },
   message: "wrapper {0}",
   id: "rqkBxe"
 });

@thekip
Copy link
Collaborator

thekip commented Aug 17, 2023

Yeah, SWC plugin works differently then babel-macro. The discovering of macro function calls in babel version is delegated to babel-macro-plugin and in SWC plugin it has written in the plugin itself.

The main question, what is the real-world scenario for this use case? Honestly speaking, i don't really want to invest time for fixing possibly rear use case in babel version of plugin.

@T9-Forever
Copy link
Author

T9-Forever commented Aug 17, 2023

The project is compiled using the swc plugin, similar to the following code

import { t as $t } from '@lingui/macro'
const columnsLen = 10
$t`Remove table ${columnsLen ? $t`columns` : $t`column`}`

It works fine locally because swc is able to parse t functions inline. It is normal for the developer to open the page locally
Now lingui-cli extracts the code and goes to publish it, and we see that there's a problem on the production

Guaranteeing 100% extraction is important because development simply doesn't realize errors locally (we expect local use to be safe)


I know that issues like the above can be bypassed with icu format like select, but in general, the average developer is still used to template string conditional judgments (we can set up the specification, but there's no lint-like plugin to circumvent these writings, and once the rest of the team writes it that way, accidents happen)


Please fix the issue
Thank you so much.!!!

@T9-Forever
Copy link
Author

T9-Forever commented Aug 17, 2023

swc plugin will remove the default message on the production, so if the cli extraction is incomplete,page will render hashId (But our users don't notice locally.)
It's so dangerous to be in the actual program

@thekip
Copy link
Collaborator

thekip commented Aug 17, 2023

I agree that inconsistency in results between extractor which uses babel and swc which is used for actual program could lead to potentialy hard to catch errors. This is indeed should be fixed but i don't know when i will have time to look into this.

I'm happy to review a PR if you feel brave to help with that 🙂

@T9-Forever
Copy link
Author

/**
 * Filtering nested macro calls
 *
 * <Macro>
 *   <Macro /> <-- this would be filtered out
 * </Macro>
 */
function isRootPath(allPath: NodePath[]) {
  return (node: NodePath) =>
    (function traverse(path): boolean {
      if (!path.parentPath) {
        return true
      } else {
        return !allPath.includes(path.parentPath) && traverse(path.parentPath)
      }
    })(node)
}

I know it's this line that talks about embedded t-function filtering, but I don't understand why we need to filter before.

For increase performance?
Removing it shouldn't have much impact, because there are actually very few internal nodes like t({})

Probably for jsx, the lift is more pronounced because Trans has many layers of nesting within it


I would love to be involved in this project because lingui is very cool!

I plan to go through the lingui-swc source code first and compare the differences between the two implementations (I'm trying to learn rust today)

@thekip
Copy link
Collaborator

thekip commented Aug 18, 2023

The difference is very big, actually. SWC version written from scratch, zero lines from babel implementation are used.

All this filtering is needed because of babel-macro-plugin integration instead of just plugin as it's done for SWC.

How it works for babel-macro:

  1. Babel macro plugins traverse AST and collect all symbols related to macro. Such as identifiers.
  2. Then it calls lingui macro code with a flat list of these identifiers.
  3. From that point our macro should understand how these identifiers are related to each other (for example nested) and do the transformation. The problem when we do the transformation we also traversing the AST from the point where given identifier is mounted. So such constructions as t`I have ${plural(count, '# apple', '# apples')}` should be transformed only once as one sentence, but macro code actually receives 2 identifiers t and plural

How SWC plugin works:

We do traverse from the Root of AST and compile nodes when they appear in the tree. It means if we found any node which is belongs to Lingui whole that AST branch would be processed and replaced in one pass.

So as you see babel macro plugin actually bringing more mess into implementation, because the lingui usecase is not so straightforward.

@T9-Forever
Copy link
Author

Thanks for the explanation!I've learned the key to the problem.

@thekip
Copy link
Collaborator

thekip commented Mar 6, 2024

fixed by #1867

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants