refactor(macro): macro to plugin

[timofei-iatsenko]

Description

There are few noticeable changes in this PR:

1. Macro is re-written as regular babel plugin and hooked up to babel-macro-plugin
2. Babel's scopes functionality is used to determine references between macro imports and usages
3. Runtime imports inserted taking to account variables in current and all children scopes and should not make any collisions with existing bindings.

These changes make a transformation more robust, reliable and ready for more complicated cases which may appear for example after code bundling.

The original motivation was to avoid complications related to babel-macros architecture, but that it turned out that it will help with fixing few of the opened bugs (#1844 #1848 ) and fix broken experimental deps extractor

The problem with babel-macro-plugin for lingui macro:

Macro written for babel-macro-plugin works a little bit different way then usual babel plugins. Instead of defining a Visitor (object which enters each node of AST) it receives a list of references for nodes used from the macro package.

import {t, plural} from "@lingui/macro";

t`Message`
t`Hello ${plural()}`

For this snippet babel-macro-plugin will invoke our macro function one time with an object:

{
  t: [Node, Node],
  plural: [Node]
}

Then our macro should take each node and somehow process it.
The problem here lays in the nested macro calls:

t`Hello ${plural()}`

Which should be turned in a single i18n call:

i18n._("Hello {plural, ...}")

We receive 2 nodes for this one expression one node in t key another in plural, and we don't have any information about how these nodes are correlated to each other.

The previous macro used quite simple approach, we collect all nodes from all keys into one single array

const references = {
  t: [Node, Node],
  plural: [Node]
}
const jsNodes = [...references.t, ...references.plural]

And then add a condition before we start processing each node which recursively goes and check do each node has parent declared in this array. Let's call this "parent matching method"

jsNodes.forEach((node) => {
   if (!node.findParent((parent) => jsNodes.includes(parent))) {
     // transform node, only if its "root" node, not a nested node in another macro
   }
})

And this worked up to the point. The code inside transformation was written "imperatively", every possible case is handled manually by additional conditions in the flow. Although there are still cases which were not anticipated in the transformation code, such as:

t`Field ${t`First Name`} is required`

But since it's a rear case that wasn't a problem for a long time.

However, things start biting again during implementation of useLingui hook:

import {useLingui, plural} from "@lingui/macro"
const {t} = useLingui()
t`Hello ${plural()}`

Only useLingui and plural are imported from @lingui/macro, and we have only theirs nodes in the references, all usages of t are in the "gray" zone for the macro.

const references = {
  useLingui: [Node],
  plural: [Node],
}

So when we have a nested macro call it get transformed by it's own, not as part of outer expression.

This is solvable, and was solved in the original PR, but this feels bad, its fragile, hard to understand and probably not very performant (i didn't test though).

When things start complicating

So far, i covered simple, human written code. But that's not all.

Imagine the case:

import { t } from "@lingui/macro";
import { plural } from "@lingui/macro";

t`Hello ${plural()}`

This a completely valid code which may be even written by a person. Due to how babel-macro-plugin works our macro function will be called 2 times (for each import statement) with references belonging to each import:

// first invocation
const references = {
  t: [Node],
}

// second invocation
const references = {
  plural: [Node]
}

If you followed all my writings you can already guess, that our "parent matching method" described above will not work, since each invocation would have it's own array of nodes and mixing and matching macros referenced from one import with another would be treat as standalone macro calls and wouldn't be turned in one runtime i18n statement.

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

// would print 2 separate i18n statements
i18n._({
  message: 'Hello {0}',
  variables: {
   0: i18n._("{plural, ...}")
 }
})

// instead of single i18n statement:
i18n._("Hello {plural, ...}")

However, this hasn't happened often for users, or at least they don't open tickets for that.

But that is the case for experimental deps extractor. Look to the code generated by esbuild:

Esbuild bundle

// src/pages/index.page.tsx
import { Plural as Plural2, t as t2, Trans as Trans3 } from "@lingui/macro";
import path from "path";
import Head from "next/head";

// src/components/AboutText.tsx
import { Trans } from "@lingui/macro";
function AboutText() {
  return <p>
    <Trans>Hello, world</Trans>
    <br />
    <Trans id="message.next-explanation">Next.js is an open-source React front-end development web framework that enables functionality such as server-side rendering and generating static websites for React based web applications. It is a production-ready framework that allows developers to quickly create static and dynamic JAMstack websites and is used widely by many large companies.</Trans>
  </p>;
}

// src/components/Developers.tsx
import { useState } from "react";
import { Trans as Trans2, Plural } from "@lingui/macro";
function Developers() {
  const [selected, setSelected] = useState("1");
  return <div>
    <p><Trans2>Plural Test: How many developers?</Trans2></p>
    <div style={{ display: "flex", justifyContent: "space-evenly" }}>
      <select
        value={selected}
        onChange={(evt) => setSelected(evt.target.value)}
      >
        <option value="1">1</option>
        <option value="2">2</option>
      </select>
      <p><Plural value={selected} one="Developer" other="Developers" /></p>
    </div>
  </div>;
}

// src/components/Switcher.tsx
import { useRouter } from "next/router";
import { useState as useState2 } from "react";
import { msg } from "@lingui/macro";
import { useLingui } from "@lingui/react";
var languages = {
  en: msg`English`,
  sr: msg`Serbian`,
  es: msg`Spanish`
};
function Switcher() {
  const router = useRouter();
  const { i18n: i18n2 } = useLingui();
  const [locale, setLocale] = useState2(
    router.locale.split("-")[0]
  );
  function handleChange(event) {
    const locale2 = event.target.value;
    setLocale(locale2);
    router.push(router.pathname, router.pathname, { locale: locale2 });
  }
  return <select value={locale} onChange={handleChange}>{Object.keys(languages).map((locale2) => {
    return <option value={locale2} key={locale2}>{i18n2._(languages[locale2])}</option>;
  })}</select>;
}

// src/pages/index.page.tsx
import styles from "../styles/Index.module.css";

// src/utils.ts
import { i18n } from "@lingui/core";
import { useRouter as useRouter2 } from "next/router";
import { useEffect, useState as useState3 } from "react";
async function loadCatalog(locale, pathname) {
  if (pathname === "_error") {
    return {};
  }
  const catalog = await import(`@lingui/loader!./locales/src/pages/${pathname}.page/${locale}.po`);
  return catalog.messages;
}

// src/pages/index.page.tsx
import { useLingui as useLingui2 } from "@lingui/react";
var getStaticProps = async (ctx) => {
  const fileName = __filename;
  const cwd = process.cwd();
  const { locale } = ctx;
  const pathname = path.relative(cwd, fileName).replace(".next/server/pages/", "").replace(".js", "");
  const translation = await loadCatalog(locale || "en", pathname);
  return {
    props: {
      translation
    }
  };
};
var Index = () => {
  useLingui2();
  return <div className={styles.container}>
    <Head>
      {
        /*
         The Next Head component is not being rendered in the React
         component tree and React Context is not being passed down to the components placed in the <Head>.
         That means we cannot use the <Trans> component here and instead have to use `t` macro.
        */
      }
      <title>{t2`Translation Demo`}</title>
      <link rel="icon" href="/favicon.ico" />
    </Head>
    <main className={styles.main}>
      <Switcher />
      <h1 className={styles.title}><Trans3>
        {"Welcome to "}
        <a href="https://nextjs.org">Next.js!</a>
      </Trans3></h1>
      <h2><Trans3>Plain text</Trans3></h2>
      <h2>{t2`Plain text`}</h2>
      <h2><Trans3>
        <a href="https://nextjs.org">Next.js</a>
        {" say hi."}
      </Trans3></h2>
      <h2><Trans3>
        {"Wonderful framework "}
        <a href="https://nextjs.org">Next.js</a>
        {" say hi."}
      </Trans3></h2>
      <h2><Trans3>
        {"Wonderful framework "}
        <a href="https://nextjs.org">Next.js</a>
        {" say hi. And "}
        <a href="https://nextjs.org">Next.js</a>
        {" say hi."}
      </Trans3></h2>
      <div className={styles.description}><AboutText /></div>
      <Developers />
      <div>
        <Plural2 value={1} one="# Person" other="# Persons" />
        <br />
        <Plural2 value={2} one="# Person" other="# Persons" />
      </div>
    </main>
  </div>;
};
var index_page_default = Index;
export {
  index_page_default as default,
  getStaticProps
};

Due to how bundler works, there might be multiple import statements in one file and usages of macro could be mixed and matched from different imports.

Solution

I decided to stop fighting with babel-macro-plugin and just write a good-old babel plugin using macro just as hook to run my plugin.

function macro({ state, babel, config }: MacroParams) {
    const plugin = linguiPlugin(babel)

    const { enter, exit } = plugin.visitor.Program as VisitNodeObject<
      any,
      Program
    >

    enter(state.file.path, state)
    state.file.path.traverse(plugin.visitor, state)
    exit(state.file.path, state)
  }

  return { keepImports: true }
}

Due to Visitor and DFS tree traversal we always start from the root node and go deeper processing every nested macro it might have. Also even if we forgot to handle some case, thanks to Visitor pattern this node could be picked up and processed by plugin even if didn't manually cover this case.

Additionally, plugin heavily relies on babel built-in methods to work with scopes/bindings, and instead of "naive" matching lingui's macros by it's name, now plugin uses referencesImport() which consider all scope bindings in the file. Let's look on example:

import { t as myT } from "@lingui/macro"; // <- already smart enought to understand renaming

myT`Hello`;

function MyFunction() {
  let myT;

  myT`Hello`; // <-- Oops, this inner scope has binding with the same name, but we not smart enought to understand this 
}

Now this is not the case and plugin correctly understand scopes and shadowing of bindings.

Clash of inserted variables and existing ones

Previous implementation was so dumb about scopes and bindings that it may broke the runtime code entirely in specific circumstances:

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

function MyFunction() {
  let i18n;

  t`Hello`;
}

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

function MyFunction() {
  let i18n;

  i18n._("Hello"); // <- Broken! since the i18n already exsits in current scope
}

Now the plugin creates a unique identifier for every inserted binding and transformed code looks like:

// AFTER
import { i18n as _i18n1 } from "@lingui/core";

function MyFunction() {
  let i18n;

  _i18n1._("Hello"); // <- Works! no naming collisions
}

What's next?

In next iterations, i'm going to make this plugin publically available and start promoting it as a recomended way to setup lingui. So users which have access to the babel config may not need to install an additional babel-macro-plugin dependency and could use this plugin directly.

The macro hook would still be available for that users which doesn't have access to babel config, such as CRA or gatsby but would be less promoted.

Currently, the plugin is private and not exposed publically from the package.

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
• Examples update

Fixes # (issue)

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 the necessary documentation (if appropriate)

[vercel]

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

Name	Status	Preview	Comments	Updated (UTC)
js-lingui	✅ Ready (Inspect)	Visit Preview	💬 Add feedback	Mar 5, 2024 4:26pm

[github-actions]

size-limit report 📦

Path	Size
./packages/core/dist/index.mjs	2.86 KB (0%)
./packages/detect-locale/dist/index.mjs	723 B (0%)
./packages/react/dist/index.mjs	1.67 KB (0%)
./packages/remote-loader/dist/index.mjs	7.26 KB (0%)

[codecov]

Codecov Report

Attention: Patch coverage is 97.00000% with 6 lines in your changes are missing coverage. Please review.

Project coverage is 77.23%. Comparing base (dd43fb0) to head (30c0a04).
Report is 2 commits behind head on next.

Files	Patch %	Lines
packages/macro/src/macroJs.ts	97.36%	1 Missing and 2 partials ⚠️
packages/macro/src/macroJsx.ts	92.30%	1 Missing and 1 partial ⚠️
packages/macro/src/index.ts	90.00%	0 Missing and 1 partial ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             next    #1867      +/-   ##
==========================================
+ Coverage   76.66%   77.23%   +0.56%     
==========================================
  Files          81       83       +2     
  Lines        2083     2126      +43     
  Branches      532      549      +17     
==========================================
+ Hits         1597     1642      +45     
+ Misses        375      374       -1     
+ Partials      111      110       -1     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

[timofei-iatsenko]

@andrii-bodnar rebased. PR is ready for review.

[andrii-bodnar]

@thekip the extractor-experimental-1797 test case looks extremely redundant, it contains a lot of files that are probably not needed for this. Can we isolate this case into a minimal set of required files and code?

[timofei-iatsenko]

the extractor-experimental-1797 test case looks extremely redundant, it contains a lot of files that are probably not needed for this. Can we isolate this case into a minimal set of required files and code?

Well, I cover every separate issue by its own test in macro / extractor. So yeah, it's probably redundant. From the other side, this is a real test, not synthetic, which potentially could catch problems after updating esbuild, or other dependencies.

[andrii-bodnar]

Well, I cover every separate issue by its own test in macro / extractor. So yeah, it's probably redundant. From the other side, this is a real test, not synthetic, which potentially could catch problems after updating esbuild, or other dependencies.

Understand. As I can see, we already have some similar tests for the experimental extractor. Let's follow them and limit the new test to the minimum amount of code and files needed. It probably doesn't need so many strings to test, a language switcher, styles, utils, tsconfig, and package.json.

[timofei-iatsenko]

they all needed unfortunately, package.json is needed to determine "externals", such as next/header, tsconfig might be deleted... The rest of files are needed to simulate real-use case when many files bundled and esbuild crawl them in specific order (so there are appears multiple macro imports).

Styles needed also nice to have, to check that esbuild don't try to parse them.

All this is fixtures, we don't compare these files. We compare only resulting catalogs.

[andrii-bodnar]

I'm trying to avoid committing excessive code. Let's reduce this test to the minimum size needed - delete unnecessary files and make the rest files as small as possible (we probably don't need 14 messages, 50+ lines of styles, or a 100-line component for the test).

[timofei-iatsenko]

i don't have time to checking how changes in the imports / files structure after "cleaning" will affect bundled output of esbuild and will it still reproduce the issue or not. I reverted it.

[andrii-bodnar]

@thekip thank you!

[andrii-bodnar]

@thekip are you planning to commit anything soon? I would like to make a new release if not

[timofei-iatsenko]

@andrii-bodnar no you can go ahead. Make it as next as well, since change is a quite big, i want to test it on real projects before

[andrii-bodnar]

https://github.com/lingui/js-lingui/releases/tag/v4.8.0-next.1

[timofei-iatsenko]

Tested on my project and on https://github.com/brave/ads-ui

works well, catalogs did not changed after extracting with new version and no errors during build.

[andrii-bodnar]

Awesome!