Babel fixes

[julienben]

Not ready yet. Just opened this to work on #2330 and potentially other issues with the babel upgrade.

Specifically, jest deps still require babel 6 and it's causing some instability. Looking into it.

[coveralls]

Coverage remained the same at 100.0% when pulling 52abc35 on babel-fix into 1b77c9c on dev.

[jwinn]

Still receiving TypeError [ERR_INVALID_ARG_TYPE]: The "path" argument must be of type string. Received type undefinedError transforming file: app/..., but no babel error (#2330) and the translation files are created.

[julienben]

Is this in reference to #2330? You got this error when running the extract-intl script with this branch checked out?

FYI, this branch isn't trying to fix the issue with the extract-intl script. I just added here a small babel fix (which I later remembered I'd already added in #2329 anyway). My plan is to use this branch to continue adding fixes to the updated Babel setup. Some packages are still dependent on Babel 6 so I'll keep this branch open until we've removed all the kinks with Babel 7 and all our deps have caught up. Jest is the pretty much the only one still on Babel 6 if I'm not mistaken.

[jwinn]

Ok. 👍 Yes, I was referencing #2330, as I wasn't sure, so just checked npm run extract-intl and got the errors and empty translation files, but no babel-related error mentioned in #2330.

Just let me know when you want it reviewed. =)

[julienben]

I gave a first shot at fixing the extract-intl script. It's not there yet but I know where the error is from.

The first issue was that Babel's transform function is now async. I switched to the new transformSync version for now. We can refactor to use the async one later if we want.

The second issue is that our 2 unofficial Babel plugins (styled-components and react-intl) don't seem to be compatible with Babel 7. Or at least not when we use them with the @babel/core Node API. Because they work the rest of the time.

For now I disabled the react-intl plugin (and styled-components), which means the script doesn't fail but it also doesn't extract anything. I'll do some more research when I'm back from my holiday (end of September). Unless someone else can give it a shot in the meantime?

If you do, take a look at lines 23-28 for the plugin setup and line 101 for the error.

I'm not yet sure if this is something we can somehow fix ourselves or if it requires some updates from the relevant packages.

I also refactored a little bit and re-enabled ESLint but this script could use some real maintenance. It needs to use async/await cause it's currently stuck in the age of callback hell.

[julienben]

Link to the relevant babel plugin: https://github.com/yahoo/babel-plugin-react-intl

[julienben]

An alternative solution would be to rewrite the script using this new plugin: https://github.com/akameco/extract-react-intl-messages

[piuccio]

I don't know if you would consider this, but in my projects I've moved away from having babel config in package json and instead added a babel.config.js at the root.

This is my configuration

module.exports = (api) => {
  const baseConfig = {
    presets: [
      ['@babel/env', {
        useBuiltIns: 'entry',
      }],
      '@babel/react',
    ],
    plugins: [
      '@babel/plugin-proposal-class-properties',
      '@babel/plugin-syntax-dynamic-import',
      '@babel/plugin-proposal-object-rest-spread',
      ['import', {
        libraryName: 'antd',
        style: 'css',
      }],
    ],
  };

  if (api.env('production')) {
    baseConfig.plugins.push(
      ['styled-components', {
        displayName: false,
        minify: true,
      }],
      'babel-plugin-transform-react-remove-prop-types',
      '@babel/transform-react-constant-elements',
      '@babel/transform-react-inline-elements'
    );
  }

  if (api.env('development')) {
    baseConfig.plugins.push(
      'styled-components'
    );
  }


  if (api.env('test')) {
    baseConfig.plugins.push(
      'styled-components',
      'dynamic-import-node',
      '@babel/plugin-transform-modules-commonjs'
    );
  }

  if (api.env('intl')) {
    baseConfig.plugins.push(
      'react-intl'
    );
  }

  return baseConfig;
};

I find it easier to read.

[julienben]

@piuccio I like the idea of breaking babel out of package.json. Mostly cause package.json is just too long with all these configs in it and I don't think it does much for us to keep it all in there. We already had to break out ESLint for the prettier integration. Might do the same for Jest and Babel in a different branch.

Closing this PR in favor of the already merged #2350.

[lock]

This thread has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.