david-www icon indicating copy to clipboard operation
david-www copied to clipboard

added npm-shrinkwrap.json support

Open luscus opened this issue 8 years ago • 6 comments

Hello,

I'm making heavy use of npm shrinkwrap and I wanted to enable the tool to take it into account - some of my packages have dependencies defined with */latest to always be up to date on npm update. The used version is then pinned down automatcally on release by gulp-shrinkwrap.

Problem was that David wasn't showing the correct information because only interpreting the *version, over time it would not have shown deprecated dependencies...

The change merges the information contained in the npm-shrinkwrap.json with the package.json before the normal processing occurs. It overwrites the Manifest versions with the pinned versions: */latest becomes x.y.z

Due to the new depth level in the code, the diff is not showing useful data: I have commented the real new lines => one block is relevant

luscus avatar Oct 09 '15 09:10 luscus

Could I get a feedback on this pull?

luscus avatar Nov 05 '15 21:11 luscus

Hi @luscus,

Thanks for your contribution. It's an interesting issue. May I ask why you're using "*" or "latest" in your package.json and not a more usual "^" or "~" version range?

My concern with the change is that whilst this works for your workflow, I have no idea if this is something that all shrinkwrap users want, or expect from david.dm.org.

I'd be more open to shrinkwrap support like this through a querystring parameter.

Let me know what you think.

alanshaw avatar Nov 12 '15 08:11 alanshaw

Hello @alanshaw,

we had bad experience in the past with stable services, the longer they're running the more probability they would break because of some deep dependency (dependency from dependency). Our package.json always use full version numbers, but our dependencies mostly use wildcards (~, ^,...) and some of those projects do not use SEMVER correctly: over time the break chance is growing.

In order to prevent this and to be able to check out and run services without problems, we have been using npm-shrinkwrap extensively. It freeze all version numbers in deep for all used packages. npm shrinkwrap is run per git-hook on pre-push

npm install uses the npm-shrinkwrap.json file to request the right versions: no more breaks

As to keep our dependencies up to date, we use git-hooks to update them on post-checkout or post-merge. npm update ignores the shrinkwrap file and uses the package.json versions: "latest" always return the up to date version. For stable packages that rarely need maintenance this enforce the dependency updating.

I know there is an argument about shrinkwrap freezing also the source of the package (npmjs.org or some corporate repo) with the "from" property. We beleave shrinkwrap is part of the dev workflow to ensure all time running versions, as such it should only freeze the version - not the source - and we workaround the "from" property using gulp-shrinkwrap (which delete this property after the file was created)

For myself my expectations about david.dm.org is to provide "true" dependency information: what I get with an npm install It should reflect the gap between the used version and the latest available - anything else would be no information.

I'm open to query string parameters and would implement it: shrimkwrap=true ?

luscus avatar Nov 12 '15 14:11 luscus

Thanks for the info, I understand the reasons for using shrinkwrap but what I was asking was why use "latest" or "*" in your package.json. When you npm update you're going to get the latest versions...true, but you'll potentially bring in breaking changes which could be a nightmare to solve if you're not careful to update just a single dependency at a time. Personally I have my version ranges in package.json set using ^ or ~ even if using shrinkwrap, so when I npm update I (optimistically) don't get breaking changes. Major version changes I examine (after consulting david-dm.org) and update carefully.

Anyway, yes, I still stand behind my original concern that the proposed behaviour is not something all shinrkwrap users want. However, I appreciate inspecting the shrinkwrap dependencies is something developers might want to do. I've worked in a few places where the shrinkwrap deps have drifted from the versions defined in package.json so can see why you might want to do that! I think the querystring shrinkwrap=true is a good way to expose the functionality your proposing in a backwards compatible way.

alanshaw avatar Nov 20 '15 08:11 alanshaw

Great! I will implement it in the coming days and submit it for review

I never had that much problems with "latest", I use a minimalistic approach to dependencies. The behaviour is intended in order to use the patches or changes - the exception is setting a number if somehow the new version can't be integrated (never append to me yet)

luscus avatar Nov 20 '15 08:11 luscus

@luscus would you be able to rebase this?

gabrielcsapo avatar Jul 04 '17 22:07 gabrielcsapo