Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Browserify error when trying to build bundle with react-router v1.0.0-rc2 #2195

Closed
musbaig opened this issue Oct 8, 2015 · 42 comments
Closed
Labels

Comments

@musbaig
Copy link

musbaig commented Oct 8, 2015

8 Oct 02:03:12 - Browserify error { [Error: Cannot find module 'qs' from '/Users/muslimbaig/github/Elements-Admin/node_modules/react-router/node_modules/history/lib']
  stream: 
   Labeled {
     _readableState: 
      ReadableState {
        highWaterMark: 16,
        buffer: [],
        length: 0,
        pipes: [Object],
        pipesCount: 1,
        flowing: true,
        ended: false,
        endEmitted: false,
        reading: true,
        sync: false,
        needReadable: true,
        emittedReadable: false,
        readableListening: false,
        objectMode: true,
        defaultEncoding: 'utf8',
        ranOut: false,
        awaitDrain: 0,
        readingMore: false,
        decoder: null,
        encoding: null,
        resumeScheduled: false },
     readable: true,
     domain: null,
     _events: 
      { end: [Object],
        error: [Object],
        data: [Function: ondata],
        _mutate: [Object] },
     _eventsCount: 4,
     _maxListeners: undefined,
     _writableState: 
      WritableState {
        highWaterMark: 16,
        objectMode: true,
        needDrain: false,
        ending: true,
        ended: true,
        finished: true,
        decodeStrings: true,
        defaultEncoding: 'utf8',
        length: 0,
        writing: false,
        corked: 0,
        sync: false,
        bufferProcessing: false,
        onwrite: [Function],
        writecb: null,
        writelen: 0,
        buffer: [],
        pendingcb: 0,
        prefinished: true,
        errorEmitted: false },
     writable: true,
     allowHalfOpen: true,
     _options: { objectMode: true },
     _wrapOptions: { objectMode: true },
     _streams: [ [Object] ],
     length: 1,
     label: 'deps' } }
8 Oct 02:03:12 - Browserify error { [Error: Cannot find module 'deep-equal' from '/Users/muslimbaig/github/Elements-Admin/node_modules/react-router/node_modules/history/lib']
  stream: 
   Labeled {
     _readableState: 
      ReadableState {
        highWaterMark: 16,
        buffer: [],
        length: 0,
        pipes: [Object],
        pipesCount: 1,
        flowing: true,
        ended: false,
        endEmitted: false,
        reading: true,
        sync: false,
        needReadable: true,
        emittedReadable: false,
        readableListening: false,
        objectMode: true,
        defaultEncoding: 'utf8',
        ranOut: false,
        awaitDrain: 0,
        readingMore: false,
        decoder: null,
        encoding: null,
        resumeScheduled: false },
     readable: true,
     domain: null,
     _events: 
      { end: [Object],
        error: [Object],
        data: [Function: ondata],
        _mutate: [Object] },
     _eventsCount: 4,
     _maxListeners: undefined,
     _writableState: 
      WritableState {
        highWaterMark: 16,
        objectMode: true,
        needDrain: false,
        ending: true,
        ended: true,
        finished: true,
        decodeStrings: true,
        defaultEncoding: 'utf8',
        length: 0,
        writing: false,
        corked: 0,
        sync: false,
        bufferProcessing: false,
        onwrite: [Function],
        writecb: null,
        writelen: 0,
        buffer: [],
        pendingcb: 0,
        prefinished: true,
        errorEmitted: false },
     writable: true,
     allowHalfOpen: true,
     _options: { objectMode: true },
     _wrapOptions: { objectMode: true },
     _streams: [ [Object] ],
     length: 1,
     label: 'deps' } }
@musbaig
Copy link
Author

musbaig commented Oct 8, 2015

Quick workaround, although not ideal, is to locally install qs and deep-equal.

@moroshko
Copy link

moroshko commented Oct 8, 2015

@websilone
Copy link

+1 with webpack :-/

@dmorneau
Copy link

dmorneau commented Oct 8, 2015

+1

3 similar comments
@ericmconnelly
Copy link

+1

@lukemarsh
Copy link

+1

@lucadegasperi
Copy link

+1

@nmqanh
Copy link

nmqanh commented Oct 8, 2015

+1

1 similar comment
@YongX
Copy link

YongX commented Oct 8, 2015

+1

@yurynix
Copy link

yurynix commented Oct 8, 2015

Same issue with webpack

@knowbody knowbody added the bug label Oct 8, 2015
@flootr
Copy link

flootr commented Oct 8, 2015

+1 with webpack

@knowbody
Copy link
Contributor

knowbody commented Oct 8, 2015

we hear you, working to get it fixed :)

@erictsangx
Copy link

+1 with webpack
Cannot find module "deep-equal"
Cannot resolve module 'qs'

1 similar comment
@CrisLi
Copy link

CrisLi commented Oct 8, 2015

+1 with webpack
Cannot find module "deep-equal"
Cannot resolve module 'qs'

@knowbody
Copy link
Contributor

knowbody commented Oct 8, 2015

as @musbaig mentioned the quick workaround for now is to:

npm install qs && npm install deep-equal

@insin
Copy link
Contributor

insin commented Oct 8, 2015

Is this caused by npm/npm#9894?

The .tgz in the npm registry contains a node_modules/history directory:

http://registry.npmjs.org/react-router/-/react-router-1.0.0-rc2.tgz

@jonny-improbable
Copy link

@insin clubuttic

@taion
Copy link
Contributor

taion commented Oct 8, 2015

npm/npm#9894 has nothing to do with this, BTW.

@mjackson
Copy link
Member

mjackson commented Oct 8, 2015

Bah, bad release. Sorry everyone.
On Thu, Oct 8, 2015 at 7:08 AM Jimmy Jia notifications@github.com wrote:

npm/npm#9894 npm/npm#9894 has nothing to do
with this, BTW.


Reply to this email directly or view it on GitHub
#2195 (comment)
.

@srph
Copy link

srph commented Oct 8, 2015

No worries. Great work, by the way!

@mjackson
Copy link
Member

mjackson commented Oct 8, 2015

Looking into this now. I'll have an rc3 out shortly that fixes this issue.

@lolJS
Copy link

lolJS commented Oct 8, 2015

Updating the history to 1.12.3 in package.json for react-router solved my problem however, I'm getting this now, using React 0.14 and rc2:

Warning: Router(...): React component classes must extend React.Component.

@woodb
Copy link

woodb commented Oct 8, 2015

@mjackson if it's helpful at all, installing the deps in PR #2209 seemed to fix the issue for me...

node v0.12.3
npm 3.3.5

@mjackson
Copy link
Member

mjackson commented Oct 8, 2015

npm/npm#9894 has nothing to do with this, BTW.

Are you sure, @taion? If npm is bundling a (potentially) older version of the history package when we publish, couldn't that be at least part of the problem here?

@taion
Copy link
Contributor

taion commented Oct 8, 2015

Huh. I don't know, then. Maybe it's also possible that this could cause CI issues, since nested dependencies also get pulled in?

I guess I assumed it wouldn't cause issues because it doesn't seem like anybody's noticed issues other than with npm shrinkwrap, unless I missed something.

@mjackson
Copy link
Member

mjackson commented Oct 8, 2015

I'm seeing some bizarre behavior here that I believe is related to npm/npm#9894. On 7c15589 (current master):

$ npm install
$ npm pack
...
react-router-1.0.0-rc2.tgz
$ tar -ztf react-router-1.0.0-rc2.tgz | grep node_modules
package/node_modules/history/package.json
package/node_modules/history/.npmignore
package/node_modules/history/README.md
package/node_modules/history/CHANGES.md
package/node_modules/history/docs/README.md
package/node_modules/history/docs/BasenameSupport.md
package/node_modules/history/docs/ConfirmingNavigation.md
package/node_modules/history/docs/GettingStarted.md
package/node_modules/history/docs/HashHistoryCaveats.md
package/node_modules/history/docs/Location.md
package/node_modules/history/docs/QuerySupport.md
package/node_modules/history/docs/Terms.md
package/node_modules/history/lib/Actions.js
package/node_modules/history/lib/createDOMHistory.js
package/node_modules/history/lib/createHashHistory.js
package/node_modules/history/lib/createHistory.js
package/node_modules/history/lib/createBrowserHistory.js
package/node_modules/history/lib/createMemoryHistory.js
package/node_modules/history/lib/deprecate.js
package/node_modules/history/lib/enableBeforeUnload.js
package/node_modules/history/lib/enableQueries.js
package/node_modules/history/lib/index.js
package/node_modules/history/lib/ExecutionEnvironment.js
package/node_modules/history/lib/runTransitionHook.js
package/node_modules/history/lib/DOMUtils.js
package/node_modules/history/lib/useBasename.js
package/node_modules/history/lib/DOMStateStorage.js
package/node_modules/history/lib/useBeforeUnload.js
package/node_modules/history/lib/AsyncUtils.js
package/node_modules/history/lib/useQueries.js
package/node_modules/history/lib/createLocation.js
package/node_modules/history/modules/Actions.js
package/node_modules/history/modules/createDOMHistory.js
package/node_modules/history/modules/createHashHistory.js
package/node_modules/history/modules/createHistory.js
package/node_modules/history/modules/createBrowserHistory.js
package/node_modules/history/modules/createMemoryHistory.js
package/node_modules/history/modules/deprecate.js
package/node_modules/history/modules/enableBeforeUnload.js
package/node_modules/history/modules/enableQueries.js
package/node_modules/history/modules/index.js
package/node_modules/history/modules/ExecutionEnvironment.js
package/node_modules/history/modules/runTransitionHook.js
package/node_modules/history/modules/DOMUtils.js
package/node_modules/history/modules/useBasename.js
package/node_modules/history/modules/DOMStateStorage.js
package/node_modules/history/modules/useBeforeUnload.js
package/node_modules/history/modules/AsyncUtils.js
package/node_modules/history/modules/useQueries.js
package/node_modules/history/modules/createLocation.js
package/node_modules/history/umd/History.js
package/node_modules/history/umd/History.min.js
package/node_modules/history/node_modules/deep-equal/package.json
package/node_modules/history/node_modules/deep-equal/LICENSE
package/node_modules/history/node_modules/deep-equal/index.js
package/node_modules/history/node_modules/deep-equal/.travis.yml
package/node_modules/history/node_modules/deep-equal/example/cmp.js
package/node_modules/history/node_modules/deep-equal/lib/is_arguments.js
package/node_modules/history/node_modules/deep-equal/lib/keys.js
package/node_modules/history/node_modules/deep-equal/readme.markdown
package/node_modules/history/node_modules/deep-equal/test/cmp.js

So our node_modules/history directory is included in the rc2 tarball. This is a bug in npm, not our fault. :/

If I remove the node_modules directory and only install babel (which I need in order to run npm pack), I get:

$ rm -rf node_modules
$ npm install babel
$ npm pack
...
react-router-1.0.0-rc2.tgz
$ tar -ztf react-router-1.0.0-rc2.tgz | grep node_modules
# nothing here

Now, the history package also has its node_modules directory included in the tarball except it is missing qs, invariant, and warning because these are also deps of react-router, so npm puts them in react-router's node_modules and not in node_modules/history/node_modules. So now, when someone goes and installs react-router, npm looks at node_modules/history and thinks it's already done there–it doesn't need to do anything. But since qs is only a dev dependency of react-router, people who aren't installing dev dependencies (i.e. using the --production flag) won't get it.

What?!

The temporary workaround here for me I think is to publish an rc3 that doesn't include any node_modules. I can do that manually, but this is ultimately a problem that needs to be fixed in npm or else one of us is bound to cut another "bad" release in the future.

@taion
Copy link
Contributor

taion commented Oct 8, 2015

This is an awful workaround, but you can also downgrade to npm2, and make sure you never run npm ddp.

ETA: It's awful because it's liable to break again the next time history's dependencies change.

@taion
Copy link
Contributor

taion commented Oct 8, 2015

You could also put rimraf node_modules/history in the prepublish script. It'll inconvenience anybody doing local development (i.e. who is running npm install on history from a local path, mostly), but it would protect against someone accidentally publishing a broken release pending the upstream NPM fix, which might be worth it.

@mjackson
Copy link
Member

mjackson commented Oct 8, 2015

That's a great idea, @taion. I'll do that for now.

@knowbody
Copy link
Contributor

knowbody commented Oct 8, 2015

If I'll ever need npm master I'll message @taion 😄

@mjackson
Copy link
Member

mjackson commented Oct 8, 2015

WHY ON GOD'S GREEN EARTH DOES NPM RUN PREPUBLISH WHEN I RUN NPM INSTALL 😫

related: npm/npm#3059

@taion
Copy link
Contributor

taion commented Oct 8, 2015

I did mention that caveat:

It'll inconvenience anybody doing local development (i.e. who is running npm install on history from a local path, mostly)

I can't really think of anything better. I don't know if you can fix this in postpublish.

@mjackson
Copy link
Member

mjackson commented Oct 8, 2015

I think I'm just going to fix this in the release script. If I rimraf node_modules/history in prepublish then I won't ever be able to actually npm install history because npm install also runs the prepublish script.

@mjackson
Copy link
Member

mjackson commented Oct 8, 2015

Related, I think I'm just going to stop using prepublish altogether. It just doesn't do what it says it does.

@taion
Copy link
Contributor

taion commented Oct 8, 2015

👍

I think npm install only runs prepublish when installing locally - for smaller packages that build quickly enough, it's convenient to automatically run build when you do npm install ../path/to/package or whatever, but it is a little annoying that there isn't actually a separate actual pre-publish hook.

The release script definitely looks like the best way to go, though. Sorry about that.

@mjackson
Copy link
Member

mjackson commented Oct 8, 2015

No problem, @taion. Your discovery of npm/npm#9894 proved invaluable in finding the root cause of this issue, so thank you for that ;)

@mjackson
Copy link
Member

mjackson commented Oct 8, 2015

Ok, just released rc3 which fixes this issue. Everything seems ok:

$ npm install --production react-router@1.0.0-rc3
$ ls -al node_modules/react-router/node_modules/history/node_modules/
total 0
drwxr-xr-x   4 michael  staff  136 Oct  8 11:10 .
drwxr-xr-x  11 michael  staff  374 Oct  8 11:10 ..
drwxr-xr-x  10 michael  staff  340 Oct  8 11:10 deep-equal
drwxr-xr-x  13 michael  staff  442 Oct  8 11:10 qs

Please open another issue if you're still having problems.

@brandondurham
Copy link

Works beautifully. Thanks so much for the super quick turnaround on this! 👏

@keeth
Copy link

keeth commented Oct 8, 2015

👍

@musbaig
Copy link
Author

musbaig commented Oct 8, 2015

Awesome guys, thank you for the timely fix. Great work!!

@websilone
Copy link

Thank you guys for your reactivity !!
Great work!

@tamzinblake
Copy link

Good news! npm/fstream-npm#15

Boelensman1 pushed a commit to Boelensman1/championpick2 that referenced this issue Dec 7, 2015
Tomoya32 added a commit to Tomoya32/Redux-Universal-Hot-Example that referenced this issue Oct 31, 2018
@lock lock bot locked as resolved and limited conversation to collaborators Jan 22, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.