outmoded / hapi-contrib

Discussion forum for project contributors
Other
78 stars 25 forks source link

How to handle package-lock.json? #114

Closed johnbrett closed 5 years ago

johnbrett commented 7 years ago

In npm5 the package-lock.json has been added.

When you npm install this file is created automatically, and when you npm version patch etc, this file is will be committed unless removed.

Have we any stance on whether hapi ecosystem modules should add/ignore/remove these, considering maintainers will see this message:

image

Will these affect how shrinkwrap will work?

geek commented 7 years ago

hapi is ignoring the new lock file https://github.com/hapijs/hapi/blob/master/.gitignore#L17

I think this is a good move, but there is a bug in npm5 around using version when the package-lock.json is in .gitignore, so you are forced to manually delete the file if you want to run that command.

Until most of the major issues in npm5 are addressed around the package-lock.json we should avoid committing it.

johnbrett commented 7 years ago

Thanks @geek! Is the plan to start committing it when the issues around npm5 are resolved, or we'll discuss that when the time comes?

geek commented 7 years ago

I am not sure what other people's plans are, but if having the lock file improves performance with cloning/installing the repo then I'd like to start committing it.

johnbrett commented 7 years ago

Makes sense, thanks again đź‘Ť

ljharb commented 7 years ago

You can also add package-lock=false to your .npmrc

Marsup commented 7 years ago

I'm personally against so I'll keep ignoring it.

nlf commented 7 years ago

i don't see it as any different than npm-shrinkwrap.json. we don't include it for libraries now (except for hapi itself) so why would we start?

to prevent npm from creating the package-lock.json automatically, we can add a .npmrc file to each repo with package-lock=false as its contents. i think that's a better approach than adding it to .gitignore since it helps make sure that the developer isn't running tests with locked dependencies that won't be locked when the library is published.

ljharb commented 7 years ago

I'd recommend both - I believe only apps should have lock files, and you both want to prevent people from accidentally creating one and also from accidentally committing one.

devinivy commented 7 years ago

package-lock.json is different than a shrinkwrap because it does not ship with the npm package, and it is superseded by any shrinkwraps. Because hapi has a shrinkwrap, it will simply be ignored by npm anyway.

AdriVanHoudt commented 7 years ago

It does indeed not make too much sense to have it for non-apps. It does not get bundled on publish. CI will always test the latest of everything which mirrors the install behaviour of a user. It would only lock for the developers which might cause a mismatch with what gets installed by a user or CI.

csrl commented 6 years ago

So what's the conclusion on this? add it to .gitignore? add a repo specific .npmrc? I'm going to reopen the issue so we can get the best practice agreed u[pon and captured.

Related, I scanned the contrib docs, and I don't see anything the captures our use of .npmignore .gitignore best practices. There's been various issues raised and resolved that we've all generally aligned on, but it doesn't seem those decisions made it into the docs?

ljharb commented 6 years ago

@csrl i'd suggest something like https://github.com/ljharb/qs/commit/f0c14802ca05fdd06a44316c159327d363b80ab5 or https://github.com/substack/tape/commit/df48bfae19d8ba4b48055dacac8b81912b8887f2

hueniverse commented 6 years ago

.gitignore + local config based on your preferences.

ljharb commented 6 years ago

(if I'm understanding your comment) I don't think "preferences" should figure into it for individual developers; every user of the repo should be forced to use, or to not use, a given lockfile, for consistency. (Obv repo collab's preferences should decide what repo users do)

hueniverse commented 6 years ago

Point is, never commit your lock file to git. If it is in the ignore file, it will never get committed. I don't care if you generate one locally or not.

eliasmelgaco commented 6 years ago

Create a file named .npmrc and put in your body

package-lock=false

hope this helps

dominykas commented 6 years ago

Create a file named .npmrc and put in your body package-lock=false

Do note that this will not only disable the creation of package-lock, but it will also ignore the npm-shrinkwrap if present, so adding it into a global (~/.npmrc) rather than a per-repo file may be undesirable.

wambete commented 5 years ago

package-lock.json is supposed to facilitate flawless operation in webpage development and Mobile Apps especially where minor changes are made, like bug fixes, and point to major changes where there is a version upgrade....I think. Check this link on how to react a package.json file for subsequent version / iterations https://docs.npmjs.com/creating-a-package-json-file

Marsup commented 5 years ago

Lock file is useful for apps, never for libraries.

Nargonath commented 5 years ago

Doesn't it have value for libs as well to speed up installs on CI using npm ci? The command requires a package-lock to work.

ljharb commented 5 years ago

Why do installs need to be sped up on libs? What libs have a large enough dep tree that that makes a difference? and isn’t that really a problem with npm ci, not with not having a lockfile?

Nargonath commented 5 years ago

@ljharb Good point.