Closed allan-stewart closed 5 years ago
I suggest to commit it, and switch to a newer version of node in the .nvmrc
file. In order to use package-lock.json which was introduced in npm version 5 we need node version 8 or up.
I would tend to agree with committing it, if all development was on the same platform, but I have had heartburn over the package-lock.json
in multiple projects when switching between linux and windows, even on node 8. Given that, I'd vote for ignoring it, unless I can validate that those past woes have been solved with 8.12 or something.
FWIW, I haven't had the same problems with yarn.lock
files.
I'm well aware that the npm docs say you should commit the package-lock.json
. I even get a message to do so when I run npm install
on a fresh clone. But given the problems I've heard about it, I'm not ready to just give in and believe npm on this one because of their inherent bias. They provide 4 reasons to commit it:
Describe a single representation of a dependency tree such that teammates, deployments, and continuous integration are guaranteed to install exactly the same dependencies.
Provide a facility for users to “time-travel” to previous states of node_modules without having to commit the directory itself.
To facilitate greater visibility of tree changes through readable source control diffs.
And optimize the installation process by allowing npm to skip repeated metadata resolutions for previously-installed packages.
I have not yet experienced a need for any of those benefits, but I have felt some pain of merge conflicts with the package-lock.json in the past. And I worry about Parker's comment too since this project is meant to be cross-platform.
So I get it... it's supposed to be helpful, but I'm worried that it isn't actually helpful. Is the cure worse than the disease? Would anyone on our project benefit from those things?
The medium post illustrates a more clear benefit:
We both have express, but we have two different versions. Theoretically, they should still be compatible, but maybe that bugfix affected functionality that we are using, and our application would produce different results when run with express version 4.15.4 compared to 4.15.5.
But again, is this problem real or just theoretical in our case? Does it impact the mob-timer negatively in practice? Will that pain be worse than the pain of keeping the file?
I guess I'm hoping for more data / anecdotes on the pros and cons before picking one over the other.
Also, know that I'm intentionally going "devil's advocate" in playing the anti-authority card here. It's just because my experience has been mostly hearing people complain about the lock file. But I've never heard anyone say how it benefitted them.
For me it is mostly about deterministic builds, that building the same commit at a later date yields the same result. The package-lock issues you are mentioning are foreign to me but then again I have mostly been using yarn and yarn-lock, cross platform and without issues.
Locking versions is also a way to avoid updating packages when that was not the intent when opening the project to do a change. It makes unintentional package version changes clear. Changing a package version could have big consequences https://github.com/dominictarr/event-stream/issues/116
I do like the idea of being intentional about your package updates. But at best that's only a partial mitigation against bad packages (like the event-stream debacle). We'd have to verify every package first, and then verify each update. Humans are lazy and don't wanna do that.
I sympathize with the deterministic builds thing too, but how often is that actually a problem? And still you aren't 100% there as far as cross-platform determinism, or even that things like an OS update changing the electron behavior. /shrug
Looking at these arguments though, I'm starting to wonder if we should go ahead and commit it, and then see how it goes?
I say commit it
Right. After these discussions (and some others outside of GitHub) I concede this point. So I'm closing this and asking you to check out #26 instead.
The
package-lock.json
file that npm creates kept getting in my way. It needs to either be committed or ignored, and I prefer ignoring it.I know that the file has been controversial in the past, so I'm open to hearing reasons why it should be committed instead.