multi-bundle fails to resolve peer dependencies

[anvaka]

I noticed browserify cdn is failing to resolve peer dependencies, even when they are part of multi-bundle.

E.g.:

curl 'http://wzrd.in/multi' --data-binary '{"dependencies":{"an":"latest","typeahead.an":"latest"}}'

This gives me:

Error: "browserify exited with code 1"

code: 1
stderr: Error: module "an" not found from "/tmp/typeahead.an114229-2259-1zh50r/node_modules/typeahead.an/index.js"

dirPath: /tmp/typeahead.an114229-2259-1zh50r

Here typeahead.an has an as a peer dependency.

[jfhbrook]

Interesting. Possibly this is a bug in the version of browserify that is on the server? @substack @maxogden

[max-mapper]

If typeahead.an requires an to function then it should be a dependency and
not a peerdependency

On Friday, March 28, 2014, Joshua Holbrook notifications@github.com wrote:

Interesting. Possibly this is a bug in the version of browserify that is
on the server? @substack https://github.com/substack @maxogdenhttps://github.com/maxogden

—
Reply to this email directly or view it on GitHubhttps://github.com//issues/73#issuecomment-38984558
.

[max-mapper]

sorry the for terseness, I was on my phone earlier. I believe this is a misunderstanding of how peerDeps are supposed to be used. feel free to re-open if I'm wrong

[KoryNunn]

As discussed here: max-mapper/requirebin#45 this issue is not about code style, but about parity with npm+browserify.

if module A has module B as a peerDependency, and it require()'s B, it will work in a normal environment, with npm and browserify.

There are two different arguments:

• The opinion that modules with peerDependencies should not require() them.
• The fact that this does work normally, and only fails with browserify-cdn.

[STRML]

Unfortunately, due to issues like facebook/react#2402, it is often necessary for plugin authors to put react in a peerDep to avoid this hard-to-debug bug.

I understand what people are trying to do with making peerDependencies stricter but it appears we're caught between a rock and a hard place with this one. Damned if we put it in dependencies, and damned (in terms of compat with projects like this) if we don't.

If browserify-cdn installed peerDependencies it would be a big help. It is the npm 2.x behavior so it is technically "correct", even if the auto-installation (but not peerDependencies) will be deprecated in npm 3. For a service like this which should just be trying to compile the module as best it can, it makes sense to install these deps.

[voronianski]

@STRML @jfhbrook @maxogden @anvaka I see this is still an issue, related to #114

[jfhbrook]

It's also a wontfix

[voronianski]

@jfhbrook is this happening because of npm v3 on wzrd.in server?

[voronianski]

I will just leave it here for some time https://github.com/jfhbrook/wzrd.in/blob/master/bundler/install.js#L12

[jfhbrook]

We're currently using npm@2.

[voronianski]

@jfhbrook hmm, so I don't understand why peer deps are not installed then..

[voronianski]

@jfhbrook I just investigated this issue a bit and found that npm install runs without errors but npm install --production results in this weird error npm ERR! Cannot read property 'react' of undefined.. probably will raise an issue in react-dom repo.

[voronianski]

@jfhbrook @maxogden maybe it's time to upgrade to npm@3?! it looks like there's no such error on that version and peer dependencies are not installed as well.

[Risto-Stevcev]

@maxogden So... is this an issue with browserify? because it's clear that wzrd.in multi bundles do not work the same way that npm does. This example wouldn't fail if the dependencies section were part of a package.json that gets npm installed and then browserifyd.

In all fairness to the lib devs that use peerDependency in this way, it's not clear which side is right. From the nodejs blog:

What we need is a way of expressing these "dependencies" between plugins and their host package. Some way of saying, "I only work when plugged in to version 1.2.x of my host package, so if you install me, be sure that it's alongside a compatible host." We call this relationship a peer dependency.

What's clear about the quote is that the plugin with the peerDependency will not work without the peerDependency being installed along side it. What's also clear is that npm1 and 2 will auto-install this dependency (and npm3 will give a warning). What isn't clear is whether or not a module import could fail if the peerDependency isn't installed. Some interpret that it could, other's don't, and it isn't clear who is right. But if it isn't explicitly mentioned in the docs, then it's not unreasonable to assume that the module import could fail without it's peerDepedency installed. And if I had to guess one practical reason (interpretations aside) why these libs (like motorcyclejs and cyclejs) use peerDependency in this way, is because npm1 and 2 will not try to flatten the dependency tree.

But ambiguity aside, I do think that wzrd.in should work like npm does to avoid any unnecessary headaches. Lib devs will write their code to work with npm, so if npm starts behaving like this then the lib devs will write it as a dependency rather than a peerDependency. But since it doesn't break on npm, they keep writing it this way. It really should work like npm does.

[jfhbrook]

wzrd.in ultimately uses npm to install and bundle. Anything you're seeing here is a consequence of where the code is placed inside an install tree when browserifying.

EDIT: In the case of multibundles, we build each dependency separately. This is probably the source of a few of your problems.

If you can find a way to make peerDeps Just Work, then by all means. To me, it's such an antipattern that I don't even really care.

[Risto-Stevcev]

EDIT: In the case of multibundles, we build each dependency separately

I think that might be the source of the problem. Why is it bundling each dependency separately?

Regardless of whether or not you think it's an antipattern (I agree that it is), it should be consistent with how npm + browserify works locally if it's calling itself browserify as a service.

More specifically, what commands is wzrd.in running that makes the following fail but works when doing it locally?

{
  "dependencies": {
    "@motorcycle/core": "^1.2.0",
    "@motorcycle/dom": "^1.3.0"
  }
}

$ npm install
$ ./node_modules/.bin/browserify -r @motorcycle/dom -t [ babelify --presets [ es2015 ] ] -s DOM -o dom.js

^ works locally ^

[jfhbrook]

I told you--- @motorcycle/core and @motorcycle/dom are built with entirely different builds, because multibundles code is just a convenience layer on top of singular bundles. Changing this would involve a large refactor and probably breakage.

If you want to take a crack at fixing it, go nuts.

[Risto-Stevcev]

Ugh.. oh well. That's a real bummer but I guess it is what it is... I'm not sure I would go through the trouble of supporting peerDependency if it's not a small fix either.

I posted an issue on their side so hopefully they'll get rid of the peerDependency and I don't have to resort to hacks.