Potentionally incorrect instructions in babel plugin

[apennell]

• Rollup Plugin Name: @rollup/plugin-babel
• Rollup Plugin Version: I have v5.3.0 installed

Documentation Is:

• Missing
• Needed
• Confusing
• Not Sure?

Please Explain in Detail...

The babel plugin docs state in the "Using With @rollup/plugin-commonjs" that commonjs has to come before babel in the plugins array if you're using both. This would be very helpful information if true, however I think following the guidance caused issues for me instead.

Here is what my config was, based on the docs:

import { babel } from '@rollup/plugin-babel'
import { nodeResolve } from '@rollup/plugin-node-resolve'
import commonjs from '@rollup/plugin-commonjs'

import pkg from './package.json'
const input = './src/index'

const config = {
  input,
  output: {
    file: pkg.main,
    format: 'cjs'
  },
  plugins: [
    nodeResolve(),
    commonjs(),
    babel({ babelHelpers: 'runtime', exclude: 'node_modules/**' })
  ]
}

export default config

And running rollup -c resulted in a syntax error (which shouldn't be an issue):

[!] (plugin commonjs) SyntaxError: Unexpected token (13:2) in /Users/anniepennell/Code/all-turtles/mmhmm-component-library/src/icons/Spinner.js
src/icons/Spinner.js (13:2)
11: 
12: const Spinner = ({ color, ...rest }) => (
13:   <svg
      ^
14:     width='24px'
15:     height='24px'

When I switched the order and put babel first, it successfully built.

Your Proposal for Changes

If confirmed that this is true, this portion should be updated to say the opposite of what it does now, so:

it's important to note that @rollup/plugin-commonjs must be placed after this plugin in the plugins array for the two to work together properly

[shellscape]

@Andarist ☝️

[Andarist]

This note has actually been added recently in response to this issue.

Basically, you can end up with different problems both ways - maybe, a better recommendation would be to use this:

const config = {
  plugins: [
    commonjs({ include: /node_modules/ }),
    babel({ babelHelpers: 'bundled' })
  ],
};

WDYT? Usually, people don't author CJS files anymore and commonjs aims to convert 3rd party code.

[apennell]

Thanks so much for the context @Andarist! I didn't do much looking into the what or why of it, just tried adjusting the order and noticed that my error went away when babel came first.

I tried your suggestion of putting commonjs first and adding { include: /node_modules/ } and that also worked. Is the explanation of this that I only need to run commonjs on my node-modules, because the code I wrote is ESM and the CJS support is just for legacy code found in my dependencies? Is setting babel-helpers: 'bundled' related, or will this still work with runtime?

[Andarist]

Is the explanation of this that I only need to run commonjs on my node-modules, because the code I wrote is ESM and the CJS support is just for legacy code found in my dependencies?

Yes.

Is setting babel-helpers: 'bundled' related, or will this still work with runtime?

This is a separate thing and should work either way.

[apennell]

Great, that did work for me. If this is a more universal solution, then I think this would be better to include in the docs.

[stale]

Hey folks. This issue hasn't received any traction for 60 days, so we're going to close this for housekeeping. If this is still an ongoing issue, please do consider contributing a Pull Request to resolve it. Further discussion is always welcome even with the issue closed. If anything actionable is posted in the comments, we'll consider reopening it. ⓘ