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

Proposal: Downgrade to warn for "npm ls" of known extraneous packages #6141

Closed
thanpolas opened this issue Sep 8, 2014 · 9 comments
Closed

Comments

@thanpolas
Copy link

While trying to apply npm shrinkwrap I had issues with extraneous packages. Their origin was known, they were installed by the napa package which is a handy library that installs frontend libraries as npm modules for consumption by browserify.

The extraneous Error originates from npm ls and I wanted to ask, before taking the time to submit a PR, if you would be open to create an array of knownPackages like napa and honor their respective keys in package.json (in this case it would be the napa key) so that detected extraneous packages that can be resolved to known packages like napa would not generate an Error but a Warn, allowing shrinkwrap to move forward.

/cc shama/napa#20

@thanpolas
Copy link
Author

@shama @kud @substack care to weigh in?

@timoxley
Copy link
Contributor

Perhaps a less invasive option would be to have ls and consequently shrinkwrap take an --ignore-extraneous flag.

@thanpolas
Copy link
Author

@timoxley is this a final suggestion? Should I go forward and implement it?

@timoxley
Copy link
Contributor

Needs @othiym23 to weigh in

@othiym23
Copy link
Contributor

How about putting napa-installed packages into bundledDependencies? That's basically what they are, and that's much simpler than the solution you're proposing.

@shama
Copy link

shama commented Oct 14, 2014

@othiym23 I experimented with bundledDependencies and napa and it generates the same error with npm@2.1.3.

Do you think it would be a good idea if bundledDependencies were ignored as extraneous by npm ls? That would solve this issue for us. Thanks!

@othiym23
Copy link
Contributor

bundledDependencies exists to make life easier for developers installing packages, and node_modules exists specifically as a place to put Node modules. It seems user-hostile to complicate people's browserify flows by not allowing client-side JS in node_modules. At the same time, the more I think about this, the trickier it gets.

  • If the napa-created package is going to be used as Node code, it seems natural to add it to dependencies, track it in Git, and (perhaps) mark it as private, to make clear that it's a local dependency.
  • If it's going to be part of a browserify pipeline, then see the previous point.
  • If it's going to be used in some other way, it should probably be installed somewhere other than node_modules.

I'm pretty sure that weakening the missing, invalid or extraneous package warnings here is the wrong thing to do; either we make it a switch, and add another little bit of conditional logic we need to support, or we change the default, which makes life difficult for people using npm in large builds, where they want assurance that what's available after a successful build is a consistent, complete installation.

I think the approach of putting napa configuration in package.json makes perfect sense, and using lifecycle scripts to drive napa also makes sense, but I think it should probably integrate more directly with npm's worldview.

Am I making sense? This seems like a tough balance to strike, and it's important to get right.

@shama
Copy link

shama commented Oct 14, 2014

Yep that makes sense. napa is very much a hack. Its only purpose is to install packages that don't cooperate with npm and much less Node; where currently on the client side, we are stricken with many. I see napa as a tool that attempts to fulfill that need within npm's worldview so I wouldn't want to weaken npm to support it.

I'll try adjusting the tooling on our end to work with npm shrinkwrap. Thanks for giving it consideration though!

@othiym23
Copy link
Contributor

Things have changed a bit since the advent of npm@3. Now, if you have a dependency included in bundledDependencies that’s not in your dependencies, it will still be included in the published package, and copied to the local install at npm install time. It doesn’t necessarily have to be in the dependencies list, but if it is, it just needs to have a ”version” that matches what’s in dependencies. That makes it possible to send napa-installed dependencies along for the ride when publishing a package. Those dependencies will still be marked as extraneous at npm shrinkwrap or npm ls time, so maybe an --ignore-extraneous switch would still make sense.

As to the rest, the point of npm shrinkwrap is to create reproducible builds. So far, that concept has assumed that all of the reproducibility is coming from npm itself. It is, and should be, an error at shrinkwrap time when there’s something in an installed application that npm doesn’t know how to reproduce. Perhaps tools like napa could be modified to run npm prune before creating the shrinkwrap file, or perhaps the CLI could implement an --ignore-extraneous switch, although the latter feels like a footgun that shouldn’t just be left lying around without a stronger justification. The CLI team is mulling over potential changes to how shrinkwrap works, and a number of those changes revolve around what kinds of install metadata are captured in the shrinkwrap file, and it’s possible that some kind of external tool integration could be incorporated there at some point. For now, though, I think it’s prudent to err on the side of conservatism and just say that we don’t intend to land these particular changes within npm itself in the near term. Thanks for your time, and all the discussion!

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

4 participants