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

Please support @shopify/react-i18n/swc #2062

Open
4 of 5 tasks
jasonwilliams opened this issue Oct 25, 2021 · 10 comments
Open
4 of 5 tasks

Please support @shopify/react-i18n/swc #2062

jasonwilliams opened this issue Oct 25, 2021 · 10 comments
Labels
Type: Feature Request 🙌 Request a new feature or changes to an existing one

Comments

@jasonwilliams
Copy link

jasonwilliams commented Oct 25, 2021

Overview

It would be nice if Shopify/react-i18n offered an esbuild/swc plugin alongside the babel one.
The action would be to port the babel plugin to an ESBuild or SWC alternative

...

Type

  • New feature
  • Changes to existing features

Motivation

Applications with the opportunity to migrate to ESBuild for faster build times may not be able to due to there being no esbuild equivalent plugin for react-i18n. For us it's the only blocker.
I think this would allow for much faster build times when compiling translations in a development environment, especially when building many components.

...

Scope

  • Is this issue related to a specific package?

    • Package: @shopify/react-i18n

Checklist

  • Please delete the labels section before submitting your issue
  • I have described this issue in a way that is actionable (if possible)
@jasonwilliams jasonwilliams added the Type: Feature Request 🙌 Request a new feature or changes to an existing one label Oct 25, 2021
@BPScott
Copy link
Member

BPScott commented Oct 25, 2021

ESBuild's plugin system isn't suitable for this. It doesn't expose any AST for JS content and the intent is never to provide one. This means there's not an easy straight port from Babel to ESBuild as we have to create something that can robustly manipulate source JS as plain old string manipulation is way too unreliable. The choice is either:

  • Fork ESBuild and add behaviour to access and transform the AST - gross, doesn't scale well for many transforms (we'll also want transforms for things like @shopify/web-workers)
  • Create a plugin that reads and parses the JS AST all over again - inefficient

Because of this our investigations into next-gen single file transpiling (i.e. what replaces Babel) has been focused on SWC which does have the intent to expose ASTs for use in plugins. @TheMallen and @peter-hamilton can talk more about that.

@jasonwilliams
Copy link
Author

Ok cheers Ben!

@jasonwilliams
Copy link
Author

funnily enough this aligns really well with Next who have just released v12 with SWC support:
https://nextjs.org/blog/next-12

Looks like an SWC plugin would be just as useful also. ill update the title

@jasonwilliams jasonwilliams changed the title Please support @shopify/react-i18n/esbuild Please support @shopify/react-i18n/swc Oct 27, 2021
@jokull
Copy link

jokull commented Nov 1, 2021

I don't know if this is useful but I created a @shopify/react-form minimum failing repo with Next.js 12.

SyntaxError: Named export 'objectSpread2' not found. The requested module '../../_virtual/_rollupPluginBabelHelpers.js' is a CommonJS module, which may not support all module.exports as named exports.
CommonJS modules can always be imported via the default export, for example using:

import pkg from '../../_virtual/_rollupPluginBabelHelpers.js';
const { objectSpread2: _objectSpread2 } = pkg;

@jasonwilliams
Copy link
Author

jasonwilliams commented Nov 1, 2021

@jokull + anyone else here, it would be useful if you can provide your babel config to the Nextjs feedback discussion vercel/next.js#30174

@KevinShiCA
Copy link

(I am Shopify employee, but commenting as an outside user of these packages in personal projects - I do not work on quilt)

@jokull I actually don't think your reproduction repo has to do with SWC specifically. I spent a while tinkering with @shopify/react-i18n which runs into the same problems on Next@12, and this happens even if you disable SWC through a custom babel config file. I believe this specific problem is on Next's end (see issue vercel/next.js#30750).

@jasonwilliams not sure if this will help, but I came across this issue because I was trying to use SWC and upgrade to Next@12, but I couldn't because I had a .babelrc specifically for the @shopify/react-i18n/babel plugin. From the docs, the point of this plugin is to reduce verbosity when calling useI18n():

We recommend that you co-locate your translation files in a ./translations directory and that you include an en.json file (schema specification) in that directory as your fallback. We give preferential treatment to this structure via a babel plugin that will automatically fill in the arguments to useI18n/ withI18n for you.

This doesn't have to be done in babel, I've managed to get rid of this plugin by writing a webpack loader to do the same thing. Here's my exact setup, many parts of this can be tweaked.

  1. I export my translation files in an actual index.ts file, not the autogenerated one (don't use generateTranslationIndexes)
import en from './en.json';
import fr from './fr.json';

export default {en, fr};
  1. Create a custom webpack loader to do the above transformations. This does exactly as the docs say: if there's an instance of useI18n() (I don't use withI18n(), if I did I'd handle that here as well), and there is a translations folder at the same level with at least en.json, then append an import to the default export of the translation index file from above and use it to populate the useI18n() arguments.
const fs = require('fs');
const path = require('path');

function loader(source) {
  if (
    this.resourcePath.includes('node_modules') ||
    !source.includes('useI18n()')
  ) {
    return source;
  }

  const resourceFolder = this.resourcePath.split('/').slice(0, -1).join('/');
  const translationsPath = path.resolve(resourceFolder, 'translations/en.json');
  this.addDependency(translationsPath);

  if (!fs.existsSync(translationsPath)) {
    return source;
  }

  const formatted = formatSource(this.resourcePath, source);

  return `import __shopify_i18n_translations from './translations';\n${formatted}`;
}

function formatSource(path, source) {
  return source.replace(
    'useI18n()',
    `useI18n({id: '${path}', fallback: __shopify_i18n_translations.en, translations(locale) { return __shopify_i18n_translations[locale]; }})`,
  );
}

module.exports = {
  default: loader,
};
  1. Use the loader in your webpack config (or next.config.js if using Next)
    config.module.rules.push({
      test: /\.tsx?$/,
      include: /src/,
      exclude: /node_modules/,
      use: [{loader: path.resolve('./webpack/i18nLoader.js')}],
    });

This will do the exact same thing that the babel plugin does, regardless of babel or SWC. Not sure if babel is needed at all otherwise, this is "working" for me on Next@12 with SWC enabled (not really working because of the named export thing described in the issue I linked above, but that should be unrelated).

@jokull
Copy link

jokull commented Nov 2, 2021

So it looks like my issue with react-forms is just because the ESM build references at least one none .mjs file (which is the _rollupPlguinBabelHelpers.js file).

Can be triggered by

$ node -e "import('@shopify/react-form')"

See this comment over at vercel/next.js.

@BPScott
Copy link
Member

BPScott commented Nov 2, 2021

SWC investigation work is happening in a private repo so i can't link to it here.

The react-form thing is a totally unrelated issue. We've accidentally got a plain .js file that node is treating as commonjs, but its contents is esm. I've documented the root cause of that at rollup/plugins#1031

@jokull
Copy link

jokull commented Nov 22, 2021

Rollup have a new release that merged @BPScott's PR. https://github.com/rollup/rollup/tree/v2.60.1

@BPScott
Copy link
Member

BPScott commented Nov 23, 2021

New versions of all quilt packages that have babel helpers have been released containing a fix for the babel helpers using the wrong extension.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Type: Feature Request 🙌 Request a new feature or changes to an existing one
Projects
None yet
Development

No branches or pull requests

4 participants