Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Disable package-lock.json #433

Merged
merged 1 commit into from
Sep 27, 2017
Merged

Disable package-lock.json #433

merged 1 commit into from
Sep 27, 2017

Conversation

joelanman
Copy link
Contributor

Currently after npm install, npm tells users that a package-lock file has been created and they should commit it. This functionality has caused problems for many people, including our own projects at GDS.

This PR disables package lock, so npm continues to function in the way it has in the past.

See more here: https://codeburst.io/disabling-package-lock-json-6be662f5b97d

@fofr
Copy link
Contributor

fofr commented Sep 27, 2017

Can you be more specific about what problems it's caused?

@frankieroberto
Copy link
Contributor

I'm intrigued by the issues it's caused too. I've experienced the opposite problem before, with issues caused by developers and production having different sub-dependencies installed (as package.json only specifies top-level package requirements). package-lock.json solves some of these issues, in the same way that Yarn does.

@gemmaleigh
Copy link
Contributor

We've run into the following issues on govuk-frontend:

npm/npm#17722
npm/npm#18135

where npm installs optional dependencies, this results in build failure in Travis CI.

The lockfile can also easily get out-of-date. We added a check to ensure if someone using < npm@5 updated package.json, CI would remind you to update the package-lock file: https://github.com/alphagov/govuk-frontend/blob/master/scripts/check-package-lock.sh

@joelanman
Copy link
Contributor Author

There are issues in the blog post linked above, plus a monster thread here: npm/npm#16866

@gemmaleigh mentioned she's had these kinds of issues, and referred me to this:
alphagov/govuk-frontend#173

@joelanman joelanman mentioned this pull request Sep 27, 2017
@quis
Copy link
Member

quis commented Sep 27, 2017

You need to git rm package-lock.json as well. Adding it to .gitignore doesn’t remove the file from source control if it’s already being tracked.

@joelanman
Copy link
Contributor Author

@quis I don't think it's in source control?

@quis
Copy link
Member

quis commented Sep 27, 2017

@joelanman Ah, didn’t check sorry. The documentation says you should commit it so I assumed…

@gemmaleigh
Copy link
Contributor

I'm going to merge this, if there are objections - feel free to raise an issue.

@gemmaleigh gemmaleigh merged commit 74cf613 into master Sep 27, 2017
@36degrees 36degrees deleted the disable-package-lock branch September 27, 2017 14:31
36degrees added a commit that referenced this pull request Jul 20, 2018
Package lock files are a representation of the dependency tree which means that anyone running `npm install` is guaranteed to install exactly the same dependencies as specified in the lock file – including sub-dependencies, which are not mentioned in the package.json file at all.

We disabled this feature in #433 because it was causing issues for users. At the time, the package-lock.json file was not being committed, which means everyone who cloned the repo and ran npm install was generating the package-lock file for the first time. Because of this, we were confusing users by npm giving them  “created a lockfile as package-lock.json. You should commit this file” notices, whilst not actually getting any of the benefits.

Additionally, Node 5 which introduced the package-lock functionality had some issues which meant that the generated package-lock files were not deterministic – so depending on who ran `npm install` the file would change. Most of these issues appear to have been resolved in newer versions of Node. We have been using the package-lock functionality on our other repos (such as Frontend) without issue.

This therefore re-enables the generation of package-lock files and commits the generate package-lock.json file to source control.
36degrees added a commit that referenced this pull request Jul 20, 2018
Package lock files are a representation of the dependency tree which means that anyone running `npm install` is guaranteed to install exactly the same dependencies as specified in the lock file – including sub-dependencies, which are not mentioned in the package.json file at all.

We disabled this feature in #433 because it was causing issues for users. At the time, the package-lock.json file was not being committed, which means everyone who cloned the repo and ran npm install was generating the package-lock file for the first time. Because of this, we were confusing users by npm giving them  “created a lockfile as package-lock.json. You should commit this file” notices, whilst not actually getting any of the benefits.

Additionally, Node 5 which introduced the package-lock functionality had some issues which meant that the generated package-lock files were not deterministic – so depending on who ran `npm install` the file would change. Most of these issues appear to have been resolved in newer versions of Node. We have been using the package-lock functionality on our other repos (such as Frontend) without issue.

This therefore re-enables the generation of package-lock files and commits the generate package-lock.json file to source control.

It also overrides the default install command (`npm install`) to use `npm install --no-optional` to avoid an issue with Travis and the `fs-events` package – see npm/npm#14042
@36degrees 36degrees mentioned this pull request Jul 20, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants