Existing package.json breaks module consumption

[ranweiler]

Hello! I have two small configuration issues to report.

I discovered them when I ran into mui/material-ui#344.

It looks like the current package.json is triggering some bugs for Browserify and even NPM consumers. I've created a repo to provide minimal examples of the issues: https://github.com/ranweiler/poc-react-draggable2

(1) Consuming react-draggable2 using require is broken due to a missing production dependency on react:

$ ./test-require.sh 

module.js:340
    throw err;
          ^
Error: Cannot find module 'react/addons'
    at Function.Module._resolveFilename (module.js:338:15)
    at Function.Module._load (module.js:280:25)
    at Module.require (module.js:364:17)
    at require (module.js:380:17)
    at Object.<anonymous> (/Users/jkr/devel/bugfix/poc-react-draggable2/node_modules/react-draggable2/lib/draggable.js:3:13)
    at Module._compile (module.js:456:26)
    at Object.Module._extensions..js (module.js:474:10)
    at Module.load (module.js:356:32)
    at Function.Module._load (module.js:312:12)
    at Module.require (module.js:364:17)

The fix for this is simply to make the relevant version of react a production dependency in package.json.

(2) The global reactify transform in the package.json of react-draggable2 breaks consumption by Browserify unless the consumer has reactify installed, either globally or in their consuming project. This is because reactify not listed in the dependencies field. If reactify is not in the require path of a consumer, an attempt to bundle a module which requires react-draggable2 results in an error like the following:

$ ./test-browserify.sh 
Error: Cannot find module 'reactify' from '/Users/jkr/devel/bugfix/poc-react-draggable2/node_modules/react-draggable2'
    at /Users/jkr/devel/bugfix/poc-react-draggable2/node_modules/browserify/node_modules/resolve/lib/async.js:46:17
    at process (/Users/jkr/devel/bugfix/poc-react-draggable2/node_modules/browserify/node_modules/resolve/lib/async.js:173:43)
    at ondir (/Users/jkr/devel/bugfix/poc-react-draggable2/node_modules/browserify/node_modules/resolve/lib/async.js:188:17)
    at load (/Users/jkr/devel/bugfix/poc-react-draggable2/node_modules/browserify/node_modules/resolve/lib/async.js:69:43)
    at onex (/Users/jkr/devel/bugfix/poc-react-draggable2/node_modules/browserify/node_modules/resolve/lib/async.js:92:31)
    at /Users/jkr/devel/bugfix/poc-react-draggable2/node_modules/browserify/node_modules/resolve/lib/async.js:22:47
    at Object.oncomplete (fs.js:108:15)

The fix for this is just to remove the global transform from the package.json of react-draggable2. If the global transform really is required for a reason I'm missing, then leaving it in and making reactify a production dependency should be an alternate fix.

I've made these small changes in a fork, and will open a pull request for review. All tests still pass, but my concern is that my changes haven't disrupted the webpack configuration. I'm not a webpack user, so I can't validate that beyond ensuring that the existing tests pass. Thanks!

[mikepb]

Hi @ranweiler !

We don't include react as a production dependency because of react-grid-layout#22

For the reactify issue, see 9d31268

Let me know if that fixes things!

[ranweiler]

Ah, thanks for the quick response and insight about the other issue!

I just tested your commit with my example repo. The build in lib is not updated, so I still get errors due to the unavailability of react, e.g.:

$ ./test-require.sh 

module.js:340
    throw err;
          ^
Error: Cannot find module 'react/addons'
    at Function.Module._resolveFilename (module.js:338:15)
    at Function.Module._load (module.js:280:25)
    at Module.require (module.js:364:17)
    at require (module.js:380:17)
    at Object.<anonymous> (/Users/jkr/devel/bugfix/poc-react-draggable2/node_modules/react-draggable2/lib/draggable.js:3:13)
    at Module._compile (module.js:456:26)
    at Object.Module._extensions..js (module.js:474:10)
    at Module.load (module.js:356:32)
    at Function.Module._load (module.js:312:12)
    at Module.require (module.js:364:17)

If I go into the react-draggable2 that NPM pulled down, inside of my test project's node_modules, and I execute npm install and then npm run-script dist to manually rebuild lib, then both of the test scripts in my test repo work.

However, when I try to wipe the state inside of the installed react-draggable2 folder (via rm -rf node_modules) and do npm install --production, I get the following error from the prepublish script:

$ npm install --production

> react-draggable2@0.5.0-dev prepublish /Users/jkr/devel/bugfix/poc-react-draggable2/node_modules/react-draggable2
> jsx -x jsx --harmony --source-map-inline src lib

sh: jsx: command not found

npm ERR! react-draggable2@0.5.0-dev prepublish: `jsx -x jsx --harmony --source-map-inline src lib`
npm ERR! Exit status 127
npm ERR! 
npm ERR! Failed at the react-draggable2@0.5.0-dev prepublish script.
npm ERR! This is most likely a problem with the react-draggable2 package,
npm ERR! not with npm itself.
npm ERR! Tell the author that this fails on your system:
npm ERR!     jsx -x jsx --harmony --source-map-inline src lib
npm ERR! You can get their info via:
npm ERR!     npm owner ls react-draggable2
npm ERR! There is likely additional logging output above.
npm ERR! System Darwin 13.4.0
npm ERR! command "/Users/jkr/.nvm/v0.10.36/bin/node" "/Users/jkr/.nvm/v0.10.36/bin/npm" "install" "--production"
npm ERR! cwd /Users/jkr/devel/bugfix/poc-react-draggable2/node_modules/react-draggable2
npm ERR! node -v v0.10.36
npm ERR! npm -v 1.4.28
npm ERR! code ELIFECYCLE
npm ERR! 
npm ERR! Additional logging details can be found in:
npm ERR!     /Users/jkr/devel/bugfix/poc-react-draggable2/node_modules/react-draggable2/npm-debug.log
npm ERR! not ok code 0

So it looks like the prepublish script just needs to be fiddled with to succeed in automatically rebuilding what ends up in lib.

[mikepb]

This is expected behavior for npm: https://docs.npmjs.com/misc/scripts

You need to add react to your top-level project dependencies to install react and avoid mzabriskie#22

[ranweiler]

Ah, my tests pass after installing react as a dependency for the test project. This is my first time seeing the deprecation of peerDependencies, and I misunderstood all that was entailed by react's inclusion in devDependencies. I appreciate the pointers!

This fixes the issue for me. Thanks very much!