"global" check is naive, and causes really odd bugs

[jakearchibald]

rollup-plugin-commonjs/src/transform.js

Line 15 in 14130a7

const firstpassGlobal = /\b(?:require|module|exports|global)\b/;

As far as I can tell, this is string-searching for "global" and switching behaviour as a result.

My builds started failing because I had a JS comment that contained the word "global".

The error suggested the failure was when it tried to parse an async function - not sure why it failed there, but if I delete "global" from a previous comment everything works again.

Here's my rollup config:

return rollup({
  entry: src,
  sourceMap: true,
  cache,
  format: 'iife',
  plugins: [
    nodeResolve({
      preferBuiltins: false,
      browser: true,
      jsnext: true,
      main: true
    }),
    commonjs(),
    rollupBabel({
      presets: ['stage-3', ['es2015', {modules: false}]],
      plugins: [["transform-react-jsx", {pragma:"h"}], "external-helpers"]
    })
  ]
})

I've added {ignoreGlobal: true} to commonjs to fix things in the meantime.

[Rich-Harris]

The regex is used to quickly determine if a file could be a CommonJS file – if it doesn't contain require, or module, or exports, or global, then clearly it isn't a CommonJS file so the plugin skips it to speed things up.

If one of those words is detected, then we have to parse it (at this stage we're still not sure if it's CommonJS or not). Once we have the AST we can do things like check to see if global is defined locally (if it's used, and isn't in a comment) – and then either transform it or skip it.

But we never get to that point, because of this line. async isn't valid ES6, so Acorn fails to parse it.

Two possible solutions:

• We change 6 to 8, so that async is allowed. Don't see a good reason not to do this – will try and get round to it later
• Transpile the code (with Babel or rollup-plugin-async, or whatever) before this plugin sees it – transforms happen in the order they're listed in the plugins array