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 <chris.br...@gmail.com> 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 npm@6.4.1 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 > <oliver.salzb...@gmail.com> 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, raphine...@gmail.com 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 `jasmine@3.2.0`. > > > - 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: dev-unsubscr...@cordova.apache.org > > For additional commands, e-mail: dev-h...@cordova.apache.org > > --------------------------------------------------------------------- To unsubscribe, e-mail: dev-unsubscr...@cordova.apache.org For additional commands, e-mail: dev-h...@cordova.apache.org
