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

[SIMPLE PATCH READY] Simple hack for compatibility with Turbopack (NextJS) #1854

Open
Palid opened this issue Feb 21, 2024 · 17 comments
Open

Comments

@Palid
Copy link

Palid commented Feb 21, 2024

Hi! 👋

Firstly, thanks for your work on this project! 🙂

Today I used patch-package to patch @lingui/[email protected] for the project I'm working on.

I've been trying to setup turbopack with my project due to enormous compilation times, and figured out why things do not work as expected - it's the loader expecting webpack environment, even though it's not really doing anything with the module definitions.
With a simple change, the loader is now turbopack-swc-compliant.
Maybe there's a better way to figure out

Just for documentation reason, I'm additionally including my next.config.js, so that documentation for NextJS+SWC loader+Turbopack can be improved in the future. :)

/** @type {import('next').NextConfig} */
const nextConfig = {
  reactStrictMode: true,
  experimental:
  {
    swcPlugins: [["@lingui/swc-plugin", {}]],
    turbo: {
      resolveExtensions: [
        ".mdx",
        ".tsx",
        ".ts",
        ".jsx",
        ".js",
        ".mjs",
        ".json",
        ".po",
      ],
      rules: {
        "*.po": ["@lingui/loader"],
      },
    },
  }
};

module.exports = nextConfig;

The usage differs a little bit after using Turbopack, as it's going to be just direct imports like shown below.
You no longer need the prefix of @lingui/loader! before your imported .po files!

async function loadLinguiMessages(lang: Lang) {
  if (process.env.NODE_ENV === "development") {
    return (
      (await import(
        `src/locales/${lang}/messages.po`
      )) as MessagesFile
    ).messages;
  } else {
    return translations[lang].messages;
  }
}

Package versions, but most likely any nextjs version with turbopack will have the same problem:
"next": "^14.1.0",
"@lingui/loader": "^4.6.0",

I might try to solve this with a proper PR, but adding a simple patch like this is definitely better than just leaving it be!

Here is the diff that solved my problem:

diff --git a/node_modules/@lingui/loader/dist/index.cjs b/node_modules/@lingui/loader/dist/index.cjs
index e478911..75c8e0a 100644
--- a/node_modules/@lingui/loader/dist/index.cjs
+++ b/node_modules/@lingui/loader/dist/index.cjs
@@ -45,7 +45,7 @@ Please check that \`catalogs.path\` is filled properly.
   const strict = process.env.NODE_ENV !== "production";
   return api.createCompiledCatalog(locale, messages, {
     strict,
-    namespace: this._module.type === "json" ? "json" : "es",
+    namespace: '_module' in this ? this._module.type === "json" ? "json" : "es" : 'es',
     pseudoLocale: config.pseudoLocale
   });
 };

This issue body was partially generated by patch-package.

(I have decided to leave this template, as it's pretty slick and I think more people could use it. 😄 )

@Palid Palid changed the title Simple hack for compatibility with Turbopack (NextJS) [SIMPLE PATCH READY] Simple hack for compatibility with Turbopack (NextJS) Feb 21, 2024
@timofei-iatsenko
Copy link
Collaborator

Thanks for the patch. Do they have documentation about compatibility of turborepo with webpack loaders?

As far as I understand from your patch when loader is executed by turborepo, there is no this._module variable, and it probably crashes when it accesses this._module.type. Am I right?

Just for context:

condition:

namespace: this._module.type === "json" ? "json" : "es",

Made to overcome the issue with webpack. Webpack doesn't allow (at least with public methods) switching type of the module in the loader. It means if you're loading "*.json" module through loader webpack expects json output from this loader. When you load any unknown to webpack module such as *.po or *.png file, webpack expects a valid ES module as output.

So this poses a problem, lingui user may load and compile catalog using *.json extension (lingui or minimal catalog formats) and using *.po format. Loader should work for both of the cases. That's the purpose of this condition.

@Palid
Copy link
Author

Palid commented Mar 1, 2024

@thekip It does not break compat with webpack by checking if we have this._module though. If we do - just go through with webpack part, if we don't we're probably in different bundler (e.g. turbopack).

It's just a quick&dirty hack to make sure this works, as turbopack makes compilation so much quicker.

@Palid
Copy link
Author

Palid commented Oct 9, 2024

@timofei-iatsenko I think the problem is that for turbopack you need to define your extensions explicitly, and it indeed is missing this._module, so if you do not define supporting extensions in the config (see first post) it will not be imported.

Do I have a green light to add a PR for this, with a comment that this path is explicitly added for solving problems with different builders than webpack (e.g. turbopack)?

@timofei-iatsenko
Copy link
Collaborator

@Palid yes please, create a PR with this patch. Also it would be good to add a doc somewhere about how to setup for turbopack, but i haven't found a good place. We have a React Server Components page, which is basically describing nextjs setup, but due to the name of this article no one will expect turbo pack configuration tips there.

@andrii-bodnar may be you can suggest a good place for that doc? Maybe just create a separate article "Turbopack Integration"

@andrii-bodnar
Copy link
Contributor

What about creating a new page Turbopack Project within the Getting Started section? Right after the Vite Project item in the sidebar.

@mrmckeb
Copy link

mrmckeb commented Oct 17, 2024

Hi @Palid, this isn't working for me in the latest RC (RC2), I'm getting "ReferenceError: i18n is not defined". Have you managed to test the RC out yet?

Note that the config should now be:

export default {
  reactStrictMode: true,
  experimental: {
    swcPlugins: [["@lingui/swc-plugin", {}]],
    turbo: {
      rules: {
        "*.po": {
          loaders: ["@lingui/loader"],
          as: "*.js",
        },
      },
    },
  },
};

@Palid
Copy link
Author

Palid commented Oct 17, 2024

@mrmckeb I'll have a look today, using this config at work and it works properly. I'll double check what's missing and get a PR ready next week.

@timofei-iatsenko
Copy link
Collaborator

Guessing from the error message it might be not related to loader itself, because there are no i18n variable inside. It rather might be related to macro and runtime execution. But it's just a guess.

@mrmckeb
Copy link

mrmckeb commented Oct 22, 2024

I've just tested this with Next.js 15, which released yesterday and I'm getting the same issue.

I think it may be to do with the macro, but I haven't dug in yet sorry.

Error:

ReferenceError: i18n is not defined

Line from the error (where t is from "@lingui/macro"):

  111 |     <div>
  112 |       <Head>
> 113 |         <title key="title">{t`Translated text`}</title>
      |          ^
  114 |         <meta
  115 |           key="description"
  116 |           name="description"

This thread indicates that there may be a bug with the plugin, perhaps an issue with how Turbopack loads SWC plugins?

@Palid
Copy link
Author

Palid commented Oct 22, 2024

@mrmckeb could you try to get me a repro repo? There's so many moving parts in here!

@andrii-bodnar
Copy link
Contributor

What about creating a new page Turbopack Project within the Getting Started section? Right after the Vite Project item in the sidebar.

Since the setup section has been heavily refactored and changed for Lingui v5 (#2060), I'd suggest adding a new section to the Installation and Setup > Build Tools documentation.

@carstenblt
Copy link

With NextJS 15 being released, a lot more people will run into problems with turbopack.

@glekner
Copy link

glekner commented Oct 29, 2024

can we get a status here? is this patch safe to use with turbopack? or should we wait for v5?

@Trosterud
Copy link

This thread is nr 1 ranking in google when searching "lingui turbopack" 👍

@timofei-iatsenko
Copy link
Collaborator

timofei-iatsenko commented Oct 29, 2024

We've released @lingui/[email protected] which is compatible with Next 15.

I prepared an example with Turborepo and swc in next 15. You can check the changes here #2073

The turborepo in that example works without any additional changes from the lingui loader side.

@mrmckeb
Copy link

mrmckeb commented Nov 11, 2024

Hi @timofei-iatsenko, we've finally had a chance to test this and got it working. Thanks so much for your hard work on this.

We have one issue that I've managed to workaround for now: t and plural macros no longer have a reference to i18n.

<button
  // This worked before, but now fails with `ReferenceError: i18n is not defined`.
  x={t`Message X`}
  // Passing in `i18n` makes it work. This is not a custom `i18n` instance.
  y={t(i18n)`Message Y`}
/>

In compiled code, I can see the issue - i18n is not pointing to the actual imported module - but I'm not sure what the fix is. Perhaps the plugin is running after Turbopack processes imports?

x: i18n._({
  id: "xiNIwB",
  message: "Message X"
}),
y: __TURBOPACK__imported__module__$5b$project$5d2f$node_modules$2f2e$pnpm$2f40$lingui$2b$core$40$4$2e$14$2e$0$2f$node_modules$2f40$lingui$2f$core$2f$dist$2f$index$2e$mjs__$5b$client$5d$__$28$ecmascript$29$__["i18n"]._({
  id: "iUN2yT",
  message: "Message Y"
})

@timofei-iatsenko
Copy link
Collaborator

@mrmckeb Please crate a minimal repro. I've just tried to reproduce this in the example project and wasn't successful.

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

No branches or pull requests

7 participants