.gitignore for "eg2" should include package-lock.json

[Chris Sharp]

After installing the new Angular client bits, my git repo sees that Open-ILS/src/eg2/package-lock.json is changed, which it shouldn't care about. Adding this to the .gitignore file in that directory should solve the problem.

Branch on the way.

[Chris Sharp (chrissharp123)]

Here ya go:

http://git.evergreen-ils.org/?p=working/Evergreen.git;a=shortlog;h=refs/heads/user/csharp/lp1792394_ignore_json_package_lock

[Bill Erickson (berick)]

Chris, I committed the package-lock.json file to the repo intentionally, because the internet told me to. Admittedly, it's not clear to me yet how it benefits us in practice. And seeing a pending commit with every npm install is irksome. Before we ignore it, though, I'd like to allow for some time to confirm we really don't want it there.

https://github.com/npm/npm/blob/v5.0.0/doc/files/package-lock.json.md

[Chris Sharp (chrissharp123)]

According removed pullrequest tag. We'll revisit if needed. Thanks for the explanation, Bill.

[Ben Shum (bshum)]

Removing active milestone, since this is still being discussed only.

[Bill Erickson (berick)]

We've been developing with package-lock.json tracked in the repo for several months now and I finally have some clarity on this. I've come around to the idea of (mostly) not using it.

The main question we have to ask: in what contexts do we encourage updating npm dependencies and when should we lock it down?

In contexts where we encourage it, we should remove package-lock.json and prevent the file from being created via configuration -- not just ignore it. I think this should be the default behavior.

In contexts where it's useful to have a manifest of the exact dependencies used, we should track package-lock.json and restrict installation procedures to using "npm install" (which reads package-lock.json) and not "npm update", which ignores package-lock.json for depencency resolution.

I suggest we do this for release tag branches (e.g. tags/rel_3_2_3). That way every build of tags/rel_3_2_3 uses the same dependencies.

My proposal is this:

1. Disable package-lock.json in Ang and AngJS via .npmrc files within each directory
2. Do not add it to .gitignore since it won't be created anyway (unless we want it).
2. Remove the existing package-lock.json files from the repository.
3. Modify the build scripts to allow for the generation of package-lock.json files during release building so that it's added to tag-release branches only.

Here's a branch to implement #1 and #2. Will wait for feedback on #3.

https://git.evergreen-ils.org/?p=working/Evergreen.git;a=shortlog;h=refs/heads/user/berick/lp1792394-kill-package-lock

[Jason Stephenson (jstephenson)]

I agree completely with 1, 2, 2, and 3. :)

My issues with this rebase (https://bugs.launchpad.net/evergreen/+bug/1749475/comments/10) were caused by package-lock.json and package.json, so I'm all for eliminating package-lock.json if we can.

[Dan Wells (dbw2)]

After manually ignoring and working around package-lock.json changes all day, I will also give a hearty +1 to all of this. Thanks, Bill!

[James Fournie (jfournie)]

Most advice on the internet favours commiting the lockfiles [1][2][3]

Perhaps this is what is causing the problem?

https://stackoverflow.com/questions/45022048/why-does-npm-install-rewrite-package-lock-json

Maybe either ensuring all the packages are properly pinned in package.json, or using the "npm ci" command may be a better solution. It's also possible some other command in the install process is updating the package json files inadvertently.

NPM packages can be unstable, and are prone to security issues [4]. It's better to be deliberate about package updates and lock things rather than let the "npm install" gods decide which packages to use.

I can also say that in my experience, Yarn is a good option. It's quite simple to switch to, and is a little faster than NPM and maybe the yarn.lock is more reliable?

[1] https://docs.npmjs.com/files/package-lock.json
[2] https://medium.com/coinmonks/everything-you-wanted-to-know-about-package-lock-json-b81911aa8ab8
[3] https://stackoverflow.com/questions/44206782/do-i-commit-the-package-lock-json-file-created-by-npm-5
[4] https://www.theregister.co.uk/2018/11/26/npm_repo_bitcoin_stealer/

.

[Bill Erickson (berick)]

On Wed, Feb 20, 2019 at 7:30 PM James Fournie <email address hidden>
wrote:

> Most advice on the internet favours commiting the lockfiles [1][2][3]
>

Sure, in most cases referencing the npm documentation. If you expand the
comments on the stackoverflow page, or read other sites [1], support for
using the lock file is not so clear cut.

>
> Perhaps this is what is causing the problem?
>
> https://stackoverflow.com/questions/45022048/why-does-npm-install-
> rewrite-package-lock-json
> <https://stackoverflow.com/questions/45022048/why-does-npm-install-rewrite-package-lock-json>
>
>
That's likely causing confusion, but the bigger issue is that the lock file
is simply not helpful to us in its current form.

Maybe either ensuring all the packages are properly pinned in
> package.json, or using the "npm ci" command may be a better solution.
> It's also possible some other command in the install process is updating
> the package json files inadvertently.
>

Being more deliberate in our versioning in package.json should be part of
this as well.

> NPM packages can be unstable, and are prone to security issues [4].
> It's better to be deliberate about package updates and lock things
> rather than let the "npm install" gods decide which packages to use.
>

I appreciate the problem the lock file is trying to solve, but simply
having a lock file is no guarantee of anything. It's just a snapshot of
what happened to be "blessed" on a given day.

The only way it provides value is if it's aggressively maintained and,
given the state of flux in the universe of npm packages, it would have be
updated practically every day. I fail to see how this is better (or really
any different) than regularly running npm-update.

Maybe down the road, when the packages we rely on the most (and the
packages they rely on, etc.) are more stable, the lock file will make more
sense.

In any event, if we decide to use them, I suggest we elect one or more npm
dependency csars to regularly review/update the lock files and test the
updated dependencies. At the same time, we require everyone else leave the
file alone and avoid using npm-update.

>
> I can also say that in my experience, Yarn is a good option. It's quite
> simple to switch to, and is a little faster than NPM and maybe the
> yarn.lock is more reliable?
>

Thanks for the comments and suggestion, James. As it stands, I don't
really have a problem with npm in general, but I will read up.

[1]
https://www.codementor.io/johnkennedy/get-rid-of-that-npm-package-lock-json-e0bj7ai42