Skip to content
This repository has been archived by the owner on Aug 11, 2022. It is now read-only.

peerDependencies (plugins) #1400

Closed
isaacs opened this issue Sep 14, 2011 · 54 comments
Closed

peerDependencies (plugins) #1400

isaacs opened this issue Sep 14, 2011 · 54 comments

Comments

@isaacs
Copy link
Contributor

isaacs commented Sep 14, 2011

New package.json field, "peerDependencies"

Just like "dependencies" hash, but:

  1. Each entry must be installed in the family (at the same level or higher) as the package. (If the package is the top target, and not global, then install as normal deps, maybe, just for testing/development purposes?)
  2. If not found, then fail, unless --force flag is set.
  3. Never ever include in the package when publishing.

This enables express middleware and other plugin-type things to specify that they depend on a certain version of express being in-play, but not bundled underneath them.

Open questions:

Should peerDeps be installed by default if missing? Leaning towards no, since this brings up all kinds of cleanup issues on uninstallation.

@dfellis
Copy link

dfellis commented Sep 14, 2011

What about taking a page from apt-get and print something like

express-route-util RECOMMENDS the following packages:
    express

and then have something like --recommended to auto-install these packages, and perhaps display a warning message like:

The following RECOMMENDED applications must be uninstalled manually:
    express

@isaacs
Copy link
Contributor Author

isaacs commented Sep 14, 2011

Yeah, a flag to install them automatically would be good. Then at least it'll feel like a separate step.

However, I'd really like to make the installation actually fail if they're missing, and require the use of -f, since it probably means that the plugin or whatever actually won't work.

@dfellis
Copy link

dfellis commented Sep 14, 2011

That makes sense; express-route-util isn't actually recommending express; it really is depending on it.

A RECOMMENDS type of thing would be the other way around; a package stating that it can optionally use the library if present (by wrapping require() in a try-catch block, for instance).

@dfellis
Copy link

dfellis commented Sep 14, 2011

Perhaps --peers would be the right flag name?

@indexzero
Copy link
Contributor

Additional discussion is at #930

@indexzero
Copy link
Contributor

@isaacs Any progress here? What can I do to help?

@domenic
Copy link
Contributor

domenic commented May 22, 2012

Ping.

We've run into this while working on plugins for Chai. The plugins need to be able to specify that they will only work with (e.g.) Chai >= 1.0.1.

@drewfish
Copy link

Ditto. We're trying to get plugins for mojito going as well.

The only thought we'd had so far was to have the plugin specify the host (i.e., express, mojito) as an"engines", as in

"engines": {
    "mojito": "0.3.x"
}

Since (I believe) NPM doesn't check the engines (like "dependencies") the host would need to do that itself. Not a clean approach :(

@indexzero
Copy link
Contributor

@drewfish That doesn't work. See the discussion on #930. @isaacs correct me if I'm wrong, but we've basically decided on the package.json API, which for your case @drewfish would be:

  {
    "peerDependencies": {
      "mojito": "0.3.x"
    }
  }

@drewfish
Copy link

Sorry, yeah, just read #930.

As for discoverability, mojito right now walks the app's node_modules/ directory looking for modules that are extensions to mojito. (We never thought that this was a great solution, but it works for now.) If npm had some kind of API that a host could use to list its plugins, then the host doesn't care where those plugins are installed.

@indexzero
Copy link
Contributor

@drewfish Cool. Anyone on the mojito team up to taking on implementing this?

@domenic
Copy link
Contributor

domenic commented May 25, 2012

@indexzero, @isaacs I'd be happy to give this a shot over the weekend. I'd implement the proposal from the OP without the parenthetical for point 1 and without install-by-default-if-missing. Sound good?

@indexzero
Copy link
Contributor

@domenic Sounds good. A couple of things:

  1. The parenthetical from point one is actually much simpler than it sounds. Just use semver.satisfies instead of semver.gt.
  2. install-by-default-if-missing is critical for anything we merge in, but if it gets implemented later that's not a problem

@domenic
Copy link
Contributor

domenic commented May 25, 2012

@indexzero by the parenthetical I meant

(If the package is the top target, and not global, then install as normal deps, maybe, just for testing/development purposes?)

Is that semver related?

@indexzero
Copy link
Contributor

@domenic No. I misunderstood. Look forward to seeing what you put together :-D

@aseemk
Copy link

aseemk commented Jun 6, 2012

Sweet! Have been wondering about this too. (And like @drewfish, used engines to specify it informally.)

@vojtajina
Copy link

👍 Totally support this effort, it would be great for plugins.

@carlos8f
Copy link

I've needed this feature for a long time. Going to give it a shot.

@carlos8f
Copy link

carlos8f commented Sep 1, 2012

Regarding point 3: does that really need addressing? Isn't it required to list things in bundledDependencies to include them when publishing? If so, wouldn't you have to include the package in both peer and bundled deps to have that problem?

Also re: install by default if missing, I would be wary. Mostly because plugins often share the same state of the parent by require()ing something in the parent's family, and if there is a different/local copy of that module in the plugin's family, it breaks the state sharing.

@indexzero
Copy link
Contributor

@domenic @isaacs Did this ever get a full-on PR somewhere?

@domenic
Copy link
Contributor

domenic commented Nov 29, 2012

@indexzero no, it's waiting on npm/read-installed#6, which has a review comment I need to take care of.

You can see the installation code here, but it's not really ready to be a proper PR until I have a version of read-installed I can work with. It probably also needs a new test to take care of the review comment.

@mfncooper
Copy link
Contributor

Once we have peerDependencies support in npm, the next step would be the host auto-discovering its peer-installed plugins. The host could certainly be told, by the app, which plugins to use, but it seems like it would be nice to be able to (perhaps optionally) auto-discover them.

Let's say I have an application that uses some-framework and some-cool-framework-plugin, and the latter declares a peer dependency on the former. What we need is some-framework to walk the packages that are its peers (i.e. in the same node_modules folder), and discover those that declare a peer dependency on it.

Unfortunately, it doesn't seem that easy to robustly locate the package's containing node_modules folder within the app. For one thing, the package may have been symlinked in there, using npm link, so it's not simply the parent folder. It'd be possible for the framework to walk module.paths, looking for itself, but that seems messy. And readInstalled() still needs to be told where the root is.

Thoughts? Apologies if this seems a tad off-topic, but it seems to me that it's part of a complete "we have plugin support" story, so it'd be good to think it through before we commit to a peer dependencies mechanism for npm.

@rvagg
Copy link
Contributor

rvagg commented Dec 16, 2012

@mfncooper this is something we've considered for Ender. I'd like to be able to scan all accessible modules and see if any are defined (somehow, something in package.json I suppose) as Ender-compatible plugins to extend the core CLI functionality. Having to discover the current node_modules directory and perhaps the global one(s) is probably not the most robust method and it'd be neat to have a way to get npm to help us out.

I think Grunt is even more of a candidate for this kind of thing at the moment though.

@balupton
Copy link

balupton commented Jan 2, 2013

Sweet. This will be really useful for abstracting out components out of https://github.com/bevry/docpad

Currently we have our own plugin loader that checks the plugin's package.json file and scans the engines and verifies them ourselves. https://github.com/bevry/docpad/blob/master/src/lib/plugin-loader.coffee#L133-L136 and https://github.com/docpad/docpad-plugin-livereload/blob/master/package.json#L31-L34 - so what @drewfish suggested.

However, having this peerDependencies would be amazing. Our use case would be for DocPad Plugins, DocPad Modules, as well as Backbone.js extensions.

@gregerolsson
Copy link

Is there any work ongoing with regards to the question @mfncooper raised; an npm linked package cannot find peer dependencies since it is symlinked from another part of the file system? I wouldn't mind giving it a stab unless someone is already working on it.

@domenic
Copy link
Contributor

domenic commented Feb 8, 2013

@gregerolsson I think the point @mfncooper raised is not terribly relevant to our current peer dependencies implementation. The idea of a host package auto-discovering its plugins is not really npm's responsibility IMO. Although, if someone wanted to write something that automatically crawled siblings, inspected their package.json, and thus compiled a list of plugins, maybe host packages would start using it.

That said, if there are issues related to npm link that make it not work with peer dependencies when peer dependencies are used in the normal way, I'm all for fixing it. But I'd think that if a peer-depends on b, and x depends on a and b, this should work:

$ cd b; npm link; cd ..
$ cd a; npm link; cd ..
$ cd x; npm link a; npm link b

@gregerolsson
Copy link

Right, the problem is actually with Node (module.js) when you only link a which peerDepends on b. I described it more thoroughly in this gist. We might just be using npm link in a way that was not intended, but we actually found us in this situation.

We have an Express-based host application that depends on a bunch of smaller "mountable" Express-apps that we develop ourselves as reusable packages. These apps peerDepend on Express (and a few others) that they expect the host application to provide. And if we need to make changes to an "app" we just npm link that particular app to a development version of it, but it still executes in the host for the final tests. Once linked, the app cannot find its peerDependencies.

Again, we might be doing things the wrong way here.

@mfncooper
Copy link
Contributor

I agree with @domenic that the auto-discovery code isn't part of npm's purview. I have working code that includes dealing with npm linked packages; I just need to pull it out of the app it's currently embedded in, and make it a reusable module. Will try to do that soon ...

BTW, @domenic, did that blog post happen? Link, please?

@edef1c
Copy link
Contributor

edef1c commented Feb 10, 2013

@mfncooper Yep, the blog post happened. http://domenic.me/2013/02/08/peer-dependencies/

@domenic
Copy link
Contributor

domenic commented Feb 11, 2013

@gregerolsson nice gist, that does explain the problem. It seems like a limitation of how npm link and Node's module system interact, sadly. Can't think of a nice solution, and can't think of anything you're doing wrong :(. Probably the dev-time copying is what you need to do.

Oh npm link, so close yet so far...

@briandipalma
Copy link

Is there a --save-peer command for installing a peerDependency? It doesn't seem to work atm.

@kenany
Copy link
Contributor

kenany commented Jan 5, 2014

@briandipalma #3994

@briandipalma
Copy link

@kenany Thank you, I see why --save-peer may not make sense but it's still a handy piece of functionality...

I'm struggling to get versions to work as I wish when using a github URL as my installing version...I guess you can't have versioning unless you publish to npm...

@briandipalma
Copy link

If you want to make npm the best JS package manager out there (and I hope you can) this inability to easily save a peerDependency should be fixed. In the front end I would prefer to have a flat dependency tree so when I install using --save I open up my package.json and move the newly installed dependency (let's us say a Web Component) from dependencies to peerDependencies.

This is a unfortunate overhead..

Unless people would recommend npm dedupe instead? That again seems like an overhead...after every install you would need to remember to run it and all new devs would have to be taught this...and I don't think dedupe would pull up dependencies that aren't duplicates so still leaving me with a deep tree in many cases...

@domenic
Copy link
Contributor

domenic commented Jan 13, 2014

Yeah, you definitely should not use peerDependencies for a flat dependency tree; that is going to break a lot of things. They are meant for plugins, not artificially flattening your dependency tree. Use dedupe for that.

@rlidwka
Copy link
Contributor

rlidwka commented Jan 13, 2014

Unless people would recommend npm dedupe instead?

Yes. I'd rather see it done automatically though.

@briandipalma
Copy link

I know it's not meant for that but it seems the only way to create a flat dependency tree, dedupe won't flatten unless there are two instances of a dependency...umm...maybe this is not an issue if using an ES6 module loader like I'm doing atm as I won't be creating script tags to import JS code any longer (Praise the Lord!).

I've added npm dedupe as a postinstall script, I'll see how that works out I guess.

@domenic
Copy link
Contributor

domenic commented Jan 13, 2014

Flat dependency trees are not inherently virtuous; npm doesn't want to support them just to keep the files flat. Non-duplicated dependencies do have independent virtues, though, so npm does put in work to make that possible. Peer dependencies are pretty unrelated to this whole thing though.

@briandipalma
Copy link

Sorry @domenic, I meant non-duplicated dependencies, we don't want to send the same dependency version to the browser twice that would be a performance issue as far as I'm concerned but two versions of the same dependency might be OK, at first I was against it but I can see that it could have its uses.

It gives the developer time to verify the package that depends on the old version works on the new one and then upgrade or if not fix the issues and then upgrade in a new release.

You would need a custom ES6 module loader though that understood node_modules but any complex web app would probably end up having it's own module loader anyway.

@domenic
Copy link
Contributor

domenic commented Jan 13, 2014

I mean, any complex web app would probably end up concatenating all its modules into a single file anyway, at which point you're best off using browserify :)

@rlidwka
Copy link
Contributor

rlidwka commented Jan 13, 2014

I mean, any complex web app would probably end up concatenating all its modules into a single file anyway

I wonder if this practice will go away with SPDY adoption...

@briandipalma
Copy link

@domenic For development too? You'd want F5 reload capability so I guess you use beefy?

@rlidwka I would love to move away from these tools and rely on the platform instead, I've had some problems with my SPDY tests but I haven't spend much time on them. I had them working up to 100 files pushed but then the browsers hit their max push resources limit and so I tried to push 100 files per request but that seemed to totally fail as the browsers requested files that I had already pushed...

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests