rollup / rollup-plugin-commonjs

This module has moved and is now available at @rollup/plugin-commonjs / https://github.com/rollup/plugins/blob/master/packages/commonjs
MIT License
502 stars 126 forks source link

Illegal reassignment to import 'commonjsHelpers' #166

Closed chief10 closed 5 years ago

chief10 commented 7 years ago

Running into this problem while trying to use this plugin.

Illegal reassignment to import 'commonjsHelpers'
Error: Illegal reassignment to import 'commonjsHelpers'
    at error (/Users/mike/Desktop/ClickTripz/trunk/integrations/node_modules/rollup/src/utils/error.js:2:14)
    at disallowIllegalReassignment (/Users/mike/Desktop/ClickTripz/trunk/integrations/node_modules/rollup/src/ast/nodes/shared/disallowIllegalReassignment.js:9:4)
    at AssignmentExpression.bind (/Users/mike/Desktop/ClickTripz/trunk/integrations/node_modules/rollup/src/ast/nodes/AssignmentExpression.js:12:3)
    at /Users/mike/Desktop/ClickTripz/trunk/integrations/node_modules/rollup/src/ast/Node.js:6:34
    at ExpressionStatement.eachChild (/Users/mike/Desktop/ClickTripz/trunk/integrations/node_modules/rollup/src/ast/Node.js:21:5)
    at ExpressionStatement.bind (/Users/mike/Desktop/ClickTripz/trunk/integrations/node_modules/rollup/src/ast/Node.js:6:8)
    at /Users/mike/Desktop/ClickTripz/trunk/integrations/node_modules/rollup/src/ast/Node.js:6:34
    at IfStatement.eachChild (/Users/mike/Desktop/ClickTripz/trunk/integrations/node_modules/rollup/src/ast/Node.js:21:5)
    at IfStatement.bind (/Users/mike/Desktop/ClickTripz/trunk/integrations/node_modules/rollup/src/ast/Node.js:6:8)
    at Module.bindReferences (/Users/mike/Desktop/ClickTripz/trunk/integrations/node_modules/rollup/src/Module.js:262:9)

It looks like the error is coming from /rollup-plugin-commonjs/src/transform.js.

Here is my rollup.config.js:

import nodeResolve from 'rollup-plugin-node-resolve';
import commonjs from 'rollup-plugin-commonjs';
import ruJson from 'rollup-plugin-json';
import ruSass from 'rollup-plugin-sass';
import ruRiot from 'rollup-plugin-riot';
import ruBabel from 'rollup-plugin-babel';
import ruBuble from 'rollup-plugin-buble';

export default {
  entry: 'work/specific/file.js',
  dest: './rollup.bundle.js',
  format: 'iife',
  plugins: [
    // ruBuble(),
    nodeResolve({ jsNext: true, main: true }),
    commonjs({
      extensions: [".js"]
    }),
    ruJson(),
    ruSass(),
    ruRiot()
  ]
}
Rich-Harris commented 7 years ago

Please include a repro

stalkerg commented 7 years ago

I have this problem too.

stalkerg commented 7 years ago

You can try just import this https://github.com/vigetlabs/gangway .

jchook commented 7 years ago

Here is a repro for this issue: https://github.com/jchook/rollup-plugin-commonjs-166

I am getting the problem when trying to import formidable.

jchook commented 7 years ago

In the case of formidable, I was able to temporarily fix the problem by commenting out all of these:

if (global.GENTLY) require = GENTLY.hijack(require);

It appears the problem is that formidable is reassigning require. Also, @stalkerg, I noticed that gangway requires superagent, which also depends on formidable.

My solution so far is to use the rollup-plugin-replace to remove the offending lines from formidable:

const
  commonjs = require('rollup-plugin-commonjs'),
  replace = require('rollup-plugin-replace'),
  rollup = require('rollup').rollup
;

return rollup({
  entry: 'index.js',
  format: 'iife',
  plugins: [

    resolve({
      jsnext: true,
      main: true,
      preferBuiltins: true,
    }),

    // HACK: removes formidable's attempt to overwrite `require`
    replace({
      include: 'node_modules/formidable/lib/*.js',
      values: {
        'if \\(global\\.GENTLY\\) require = GENTLY\\.hijack\\(require\\);': '',
      }
    }),

    commonjs({
      include: 'node_modules/**',
    }),
  ],
  sourceMap: true
})
.then(function (bundle) {
  return bundle.write({
    format: 'iife',
    dest: 'bundle.js'
  });
}).catch(function(err){
  console.error(err);
});
jchook commented 7 years ago

@Rich-Harris this issue now has a repro.

fregante commented 7 years ago

For reference, it works in browserify:

npm i formidable
echo "require('formidable')" | browserify -

And this is the clearer error shown in more recent rollup versions:

🚨   Illegal reassignment to import 'commonjsHelpers'
node_modules/formidable/lib/incoming_form.js (29:43)
27: import os from 'commonjs-proxy:os';
28: 
29: if (commonjsHelpers.commonjsGlobal.GENTLY) commonjsHelpers.commonjsRequire = GENTLY.hijack(commonjsHelpers.commonjsRequire);
                                               ^

This is a bit of an edge case. Rollup tries to be smart about require and reassigning it is pulling the rug from under its feet.

It "works" in browserify because browserify just passes require as a variable like node itself:

function(require,module,exports){
(function (global){
if (global.GENTLY) require = GENTLY.hijack(require);
bruce-one commented 7 years ago

Edit: This is a reference to lazy-cache leading to this issue...

Another possible solution is to use unlazy-loader to leverage the fix/workaround done for Webpack, eg

import unlazyLoader from 'unlazy-loader'
import { createFilter } from 'rollup-pluginutils'

export default function unlazy ( options = {} ) {
    const filter = createFilter( options.include, options.exclude )

    return {
        name: 'unlazy'

        , transform ( source, id ) {
            if ( !filter( id ) ) return null
            var res = unlazyLoader(source)
            if(res === source) return
            return res
        }
    }
}

(Might be dodgy etc, but seemed to work well enough for us at this point :-) )

antony commented 7 years ago

In case it helps, another case where this happens is ethereum's web3 js library.

https://github.com/ethereum/web3.js/blob/develop/dist/web3.js

ronanquillevere commented 7 years ago

Same problem for me with superagent and formidable, thanks for the hack @jchook but unfortunatly it does not seem to work for me. I still get like if the replace did not work

[!] Error: Illegal reassignment to import 'commonjsHelpers'
node_modules/formidable/lib/incoming_form.js (29:43)
27: import os from 'commonjs-proxy:os';
28: 
29: if (commonjsHelpers.commonjsGlobal.GENTLY) commonjsHelpers.commonjsRequire = GENTLY.hijack(commonjsHelpers.commonjsRequire);

Ok so I used another plugin which seems to work rollup-plugin-re


import replace from 'rollup-plugin-re';

//...

 replace({
      // ... do replace before commonjs
      patterns: [
        {
          // regexp match with resolved path
          match: /formidable(\/|\\)lib/, 
          // string or regexp
          test: 'if (global.GENTLY) require = GENTLY.hijack(require);', 
          // string or function to replaced with
          replace: '',
        }
      ]
    }),
luisrudge commented 6 years ago

I fixed this for my specific superagent issue. Hopefully it'll help others.

I'm building a browser library, but superagent's "main" file in package.json points to the server bundle. To fix that, I changed this in my config:

before:

import resolve from 'rollup-plugin-node-resolve';

const plugins = [
  resolve()
]

after:

import resolve from 'rollup-plugin-node-resolve';

const plugins = [
  resolve({ browser: true })
]
benjamingr commented 5 years ago

@luisrudge thanks! your fix worked for me and it's a lot less hacky than the alternatives of rewriting the file contents with regex <3

colllin commented 5 years ago

Still a problem these days 🙄. I found this solution in an issue about webpack somewhere:

replace({
    'global.GENTLY': false,
})
shellscape commented 5 years ago

Hey folks (this is a canned reply, but we mean it!). Thanks to everyone who participated in this issue. We're getting ready to move this plugin to a new home at https://github.com/rollup/plugins, and we have to do some spring cleaning of the issues to make that happen. We're going to close this one, but it doesn't mean that it's not still valid. We've got some time yet before the move while we resolve pending Pull Requests, so if this issue is still relevant, please @ me and I'll make sure it gets transferred to the new repo. :beer:

CodeTroopers commented 4 years ago

@shellscape I have the same issue with to-file module where the author (@jonschlinkert) reassign require here Must i open a new issue in the new repository? Must i open issue in to-file module ?