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
