I do not agree with your understanding here. We should use shrinkwrap for all releases. Please see my _very extensive_ argument and explanation on that topic in the thread you referenced [1].
It has all kinds of references too, some of which you then found again yourself in a later message of the same thread [2]. I have to ask: did you read my message [1] at all? [1]: https://lists.apache.org/thread.html/c06d35b6944fb44a04449a7baf10797f8ae11cbcfddb3e90851dc0b9@%3Cdev.cordova.apache.org%3E [2]: https://lists.apache.org/thread.html/bce77e5892543003c3b3671cb6a2e9e02c33485073e691c6cf2c5fcf@%3Cdev.cordova.apache.org%3E Am Mo., 17. Sep. 2018 um 13:14 Uhr schrieb Chris Brody < [email protected]>: > Originated in another thread ([1]), moving into a new thread. > > My understanding is that we should only do npm shrinkwrap if needed to > deal with a dependency problem in an end-user package such as > cordova-cli. > > I would like to ask people to please use references whenever possible. > > [1] > https://lists.apache.org/thread.html/7f92561d382f143aaf49e083bbe215dcf95a3f4d8b6e3cbb6089a5f3@%3Cdev.cordova.apache.org%3E > > On Mon, Sep 17, 2018 at 5:38 AM Oliver Salzburg > <[email protected]> wrote: > > > > +1 to shrinkwrap before publish > > > > On 2018-09-15 00:27, [email protected] wrote: > > > Please note that I'm not suggesting to commit shrinkwrap to the master > of > > > every repository. That is what had been discussed in the mailing list > > > thread from 2014 that you shared. That option was dismissed then, and > > > rightly so. It is also strongly advised against in the npm docs. > > > > > > I suggest using it only in release branches of CLI or other non-library > > > releases, so they are immutable and don't mutate over time. _This is > > > exactly the intended use_. And it comes at no cost other than having > to run > > > `npm shrinkwrap` right before the release. > > > > > > Plus, it allows us to override transitive dependency versions for a > release > > > if absolutely necessary. > > > > > > Am Sa., 15. Sep. 2018 um 00:16 Uhr schrieb Jesse < > [email protected]>: > > > > > >> I personally see shrinkwrap as a step in the wrong direction, I feel > like > > >> it has been explored in the past, and the general consensus is that > it is > > >> something to avoid. > > >> If committing package-lock is contentious, I am okay with skipping > it, but > > >> we need to be careful to lock down our own dependencies and not use > > >> wildcards. > > >> > > >> We have flexibility in our current GitHub repos, which allow us to > remix > > >> and freely link other dependent packages so we can debug right > through our > > >> deps. I agree we need more stability around the release/publish > steps so > > >> we can move more quickly through the collection of packages that make > up a > > >> 'tools' release. > > >> > > >> @purplecabbage > > >> risingj.com > > >> > > >> > > >> On Fri, Sep 14, 2018 at 2:56 PM <[email protected]> wrote: > > >> > > >>> No. This won't fix anything. Plus it goes directly against npm's > > >>> recommendation. Please double check the docs for the use cases of > > >>> package-lock.json vs npm-shrinkwrap.json. > > >>> > > >>> Am Fr., 14. Sep. 2018 um 23:48 Uhr schrieb Chris Brody < > > >>> [email protected]>: > > >>> > > >>>> A really nice alternative may be to turn the generated > > >>>> package-lock.json into npm-shrinkwrap.json (using npm shrinkwrap > > >>>> command) then commit npm-shrinkwrap.json. Then I think any other npm > > >>>> install updates would update npm-shrinkwrap.json instead of > > >>>> package-lock.json. Could be more predictable and easier to > understand. > > >>>> > > >>>> This was already discussed in 2014 [1], thanks to Jesse for the > link in > > >>>> [2]. > > >>>> > > >>>> Thanks for the suggestion to use npm shrinkwrap as a solution for > > >>>> cordova-cli 8.1.0 minor release in [2]. > > >>>> > > >>>> [1] > > >>>> > > >> > https://lists.apache.org/thread.html/99184622129935eb473e843e583bf6648faff279a014e8508cc2c660@1411013202@%3Cdev.cordova.apache.org%3E > > >>>> [2] > > >>>> > > >> > https://lists.apache.org/thread.html/f89a074add24f2ace7006b0211cf43a47cc5c1a0a65932fc22515828@%3Cdev.cordova.apache.org%3E > > >>>> On Fri, Sep 14, 2018 at 3:53 PM Chris Brody <[email protected]> > > >>> wrote: > > >>>>> To be honest I have pretty limited experience with package lock > file > > >>>>> and it is now starting to show. From Oliver's very unfortunate > > >>>>> experience I would conclude that this is something we should do > very > > >>>>> carefully and not just on a whim. Some things I can think of: > > >>>>> > > >>>>> * always use recent version of npm such as [email protected] to generate > or > > >>>>> update package-lock.json > > >>>>> * do not use. npm cache when generating or updating > > >> package-lock.json, > > >>>>> or use the npm cache with extreme care (also limited experience for > > >>>>> me) > > >>>>> * be extremely careful with assumptions; I think we should both > > >>>>> double-check the documentation and do our own experimentation, > like I > > >>>>> did in <https://github.com/apache/cordova-cli/pull/325> to > validate > > >> as > > >>>>> best we can > > >>>>> * semver package seems to be a major library package used by npm; > we > > >>>>> should both read the documentation and experiment, ideally with its > > >>>>> own test cases > > >>>>> > > >>>>> On Fri, Sep 14, 2018 at 9:47 AM Oliver Salzburg > > >>>>> <[email protected]> wrote: > > >>>>>> The problems that appear when you have linked dependencies is that > > >>> npm > > >>>>>> will pick them up as being bundled and mark them as such in the > > >>>>>> lockfile. *However* this behavior has changed in the past. At one > > >>> point > > >>>>>> this affected any direct dependency, at another point it "only" > > >>>> affected > > >>>>>> dependencies of dependencies. > > >>>>>> > > >>>>>> Either way, the result is: > > >>>>>> > > >>>>>> a) a corrupted lockfile, which has dependencies marked as bundled > > >>>>>> b) a lockfile that lists incorrect versions, resulting from > linking > > >>>>>> temporary development snapshots together > > >>>>>> > > >>>>>> When you use a local npm cache and you neglected to correctly > > >>>>>> parameterize your npm calls, you now have your custom registry URL > > >> in > > >>>>>> the lockfile for every package it installed from there. This makes > > >> it > > >>>>>> unusable for others. > > >>>>>> > > >>>>>> The issue that ultimately drove us away from this concept was that > > >>>>>> locally cached packages were installed over linked modules, > because > > >>> the > > >>>>>> package manager did not understand that they are linked. > > >>>>>> But because they were linked, the cached package contents were > > >> placed > > >>>> in > > >>>>>> my local development checkout of that linked module. That > obviously > > >>>>>> caused all uncommitted changes to be deleted. > > >>>>>> > > >>>>>> Additionally, if you already have linked modules set up, but the > > >>>>>> lockfile says that a certain dependency is to be replaced, it will > > >>> just > > >>>>>> break your link or replace your linked code as soon as you `npm > > >>>> install`. > > >>>>>> We had so many issues with this, I'm sure I'm only remembering the > > >>> tip > > >>>>>> of the ice berg. Maybe all of this was fixed somehow, but I doubt > > >> it. > > >>>> At > > >>>>>> the time when I reported these issues, there was little interest > in > > >>>>>> resolving them. > > >>>>>> > > >>>>>> However, I'm not unfamiliar with the lockfile-driven workflow as > > >> many > > >>>>>> OSS projects I contributed to use it. It is not uncommon to > > >>> completely > > >>>>>> wipe your node_modules and/or package-lock.json to rebuild it, > > >>> because > > >>>>>> of corruptions in either entity. And that is something that has > > >> been > > >>>>>> confirmed to me many times by other developers. As in "That's just > > >>> how > > >>>>>> it is." > > >>>>>> > > >>>>>> This entire area of issues was not exclusive to npm either. We > > >>>>>> extensively evaluated yarn regarding these aspects and it > performed > > >>>> just > > >>>>>> as poorly. > > >>>>>> > > >>>>>> I consider these aspects unacceptable for a development workflow > as > > >>>> they > > >>>>>> introduce an unreliability where I can't have one. > > >>>>>> > > >>>>>> If someone came out and told me "Hey, you've been doing it wrong > > >> all > > >>>>>> along. These are your mistakes and this is how you resolve them." > > >>> then > > >>>>>> I'd be very happy to hear that :) > > >>>>>> > > >>>>>> On 2018-09-14 15:13, [email protected] wrote: > > >>>>>>> Thanks for picking this up again Chris. I think now is a better > > >>> time > > >>>> for > > >>>>>>> second thoughts than later. > > >>>>>>> > > >>>>>>> I've had a look at your experiment and the behavior you observed > > >> is > > >>>> to be > > >>>>>>> expected and desirable, as I explained in more detail in a > > >> comment > > >>>> [1] > > >>>>>>> there. As I also mentioned there, deleting and regenerating > > >>>> package-lock.json > > >>>>>>> is a valid approach _if and when_ you want to do a full > > >> dependency > > >>>> update, > > >>>>>>> as we regularly do. > > >>>>>>> > > >>>>>>> Also, thanks for posting Oliver's message here for better > > >>> visibility > > >>>> in > > >>>>>>> this discussion. What I _do_ find a bit disturbing is the > > >> problems > > >>> he > > >>>>>>> mentioned regarding linking (as in `npm link`) of different > > >> modules > > >>>> which > > >>>>>>> are all using lock files. He expressed his concern regarding that > > >>>>>>> particular use-case again in a comment [2] of a PR where we > > >> touched > > >>>> that > > >>>>>>> topic. I think it is important we test this, since the ability to > > >>>> link > > >>>>>>> modules is vital for our development workflow and I have no > > >>>> experience with > > >>>>>>> package-lock.json in projects where a lot of linking is > > >> necessary. > > >>>>>>> Finally, I think we might need to re-evaluate our presumed > > >>> knowledge > > >>>> about > > >>>>>>> the topic at hand. I encourage all those interested to read > > >>>> [3][4][5] so we > > >>>>>>> all know what we are talking about. I had my facts wrong too and > > >>>> nobody > > >>>>>>> corrected me, when I uttered them here in this thread. So here's > > >> a > > >>>> quick > > >>>>>>> (probably incomplete) round up of what package-lock.json does and > > >>>> does not > > >>>>>>> do: > > >>>>>>> > > >>>>>>> - It does provide a snapshot of the dependency tree that can > > >> be > > >>>>>>> committed into source control to avoid automatic updates of > > >>>> (transitive) > > >>>>>>> dependencies break the build _during development and CI_ > > >>>>>>> - It _does not_ ensure that a user installing the published > > >>>> package gets > > >>>>>>> exactly the dependency versions that are specified in > > >>>> package-lock.json. > > >>>>>>> That is what npm-shrinkwrap.json [5] is for. > > >>>>>>> - It does speed up installation of dependencies in > > >> conjunction > > >>>> with `npm > > >>>>>>> ci` by skipping the entire dependency resolution and using > > >> the > > >>>> versions > > >>>>>>> from the lock file. > > >>>>>>> - It is required to be present for `npm audit` [6], although > > >> it > > >>>> could be > > >>>>>>> generated ad-hoc. > > >>>>>>> - It is possible to manually tinker with the lock file to > fix > > >>>> audit > > >>>>>>> issues with transitive dependencies that have no update > > >>>> available. This > > >>>>>>> requires some special care to prevent npm from resetting > > >> these > > >>>> manual > > >>>>>>> changes, but it's a valuable last-resort option. However, > > >> this > > >>>> is far more > > >>>>>>> useful with npm-shrinkwrap.json to create releases without > > >>>> security > > >>>>>>> issues. > > >>>>>>> > > >>>>>>> With that cleared up, my stance on committing package-lock.json > > >> is > > >>> as > > >>>>>>> follows: > > >>>>>>> > > >>>>>>> - Faster CI installations and faster/easier usage of `npm > > >>> audit` > > >>>> are > > >>>>>>> purely convenience features for me. > > >>>>>>> - Consistent developer builds and updates only on-demand are > > >>> real > > >>>>>>> advantages for me. I just recently spent hours finding out > > >> why > > >>>> some tests > > >>>>>>> of cordova-lib were suddenly failing. It turned out it was > > >>>> caused by an > > >>>>>>> update to `[email protected]`. > > >>>>>>> - If the package-lock.json really should interfere with our > > >>>> ability to > > >>>>>>> link repositories for development, then that would be a deal > > >>>> breaker for me. > > >>>>>>> However, the primary goal that I wanted to achieve was _immutable > > >>>>>>> releases_. That is, installing e.g. `cordova@9` should _always > > >>>> install the > > >>>>>>> exact same set of packages_. What we need for that is > > >>>> npm-shrinkwrap.json. > > >>>>>>> So IMO whether we decide for or against using package-lock.json, > > >> we > > >>>> should > > >>>>>>> lock down the dependencies for releases of our CLIs, platforms > > >> and > > >>>> possibly > > >>>>>>> plugins by generating and committing a npm-shrinkwrap.json to the > > >>>> _release > > >>>>>>> branch_ before packaging the release. > > >>>>>>> > > >>>>>>> Cheers, > > >>>>>>> Raphael > > >>>>>>> > > >>>>>>> [1]: > > >>>> > https://github.com/apache/cordova-cli/pull/325#issuecomment-421336025 > > >>>>>>> [2]: > > >>>>>>> > > >> > https://github.com/raphinesse/cordova-common/pull/1#issuecomment-420950433 > > >>>>>>> [3]: https://docs.npmjs.com/files/package-locks > > >>>>>>> [4]: https://docs.npmjs.com/files/package-lock.json > > >>>>>>> [5]: https://docs.npmjs.com/files/shrinkwrap.json > > >>>>>>> [6]: > > >>> https://docs.npmjs.com/getting-started/running-a-security-audit > > >>>>>> > > >>>>>> > > >> --------------------------------------------------------------------- > > >>>>>> To unsubscribe, e-mail: [email protected] > > >>>>>> For additional commands, e-mail: [email protected] > > >>>>>> > > >>>> > --------------------------------------------------------------------- > > >>>> To unsubscribe, e-mail: [email protected] > > >>>> For additional commands, e-mail: [email protected] > > >>>> > > >>>> > > > > > > --------------------------------------------------------------------- > > To unsubscribe, e-mail: [email protected] > > For additional commands, e-mail: [email protected] > > > > --------------------------------------------------------------------- > To unsubscribe, e-mail: [email protected] > For additional commands, e-mail: [email protected] > >
