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

Do we need exclude yarn.lock and package-lock.json from all repository ? #3370

Closed
yoshinorin opened this issue Nov 22, 2018 · 16 comments
Closed
Labels
dependencies Pull requests that update a dependency file discussion

Comments

@yoshinorin
Copy link
Member

yoshinorin commented Nov 22, 2018

@hexojs/core

Currently some repositories not exclude yarn.lock & package-lock.json.
Dependabot pull requests include yarn.lock like this.

https://github.com/hexojs/site/pull/815/files

Does we need add them to .gitignore before merge dependabot's pull request ?

@yoshinorin yoshinorin added discussion dependencies Pull requests that update a dependency file labels Nov 22, 2018
@yoshinorin yoshinorin changed the title Need exclude yarn.lock vs package-lock.json from all repository ? Does we need exclude yarn.lock vs package-lock.json from all repository ? Nov 22, 2018
@demurgos
Copy link

My opinion is that lockfiles should be commited for reproducibility (for all packages). And it's good that dependabot updates the lockfiles.

@JLHwung
Copy link
Collaborator

JLHwung commented Nov 22, 2018

I also agree that lockfile should be commit to the repository as recommended from https://docs.npmjs.com/files/package-locks#using-locked-packages.

And we should stick to one package manager. After years on yarn vs npm I have no specific preference, both are good to go.

@yoshinorin
Copy link
Member Author

Thanks!

I agree with your opnion & I prefer commit lockfiles.
But, actually this repository setting exclude them. I wanna know hexo organization guideline.

@tomap
Copy link
Contributor

tomap commented Nov 22, 2018

I believe that yarn now relies on package-lock.json

@JLHwung
Copy link
Collaborator

JLHwung commented Nov 23, 2018

I believe that yarn now relies on package-lock.json

@tomap What do you mean by yarn now relies on package-lock.json?

I wanna know hexo organization guideline.

@yoshinorin Here is a historical talks around package lock files. At this time I think I can further convince @NoahDragon that with the help of renovate we can handle dependencies bumps more smoothly.

Package lock files are not published to npm. Even if it is published by improper configuration, the package manager would not respect package lock file in node_modules.

@yoshinorin
Copy link
Member Author

yoshinorin commented Nov 23, 2018

@JLHwung Thanks.

Here is a historical talks around package lock files

I Got it.

I think I can further convince @NoahDragon

So, my understanding is that we need waiting @NoahDragon reply and discuss about this issue with him again. Is it correct?

@yoshinorin yoshinorin changed the title Does we need exclude yarn.lock vs package-lock.json from all repository ? Does we need exclude yarn.lock and package-lock.json from all repository ? Nov 23, 2018
@yoshinorin
Copy link
Member Author

yoshinorin commented Nov 23, 2018

@demurgos @JLHwung @tomap
Oops, I made mistake. Title were incorrect.
Not yarn.lock vs package-lock.json but yarn.lock and package-lock.json

PS. Also, It's better if we can decide which use.

@tomap
Copy link
Contributor

tomap commented Nov 23, 2018

https://yarnpkg.com/blog/2018/06/04/yarn-import-package-lock/

Yarn can read package-lock
But npm cannot read yarn.lock

So if we keep one, we should keep package-lock

Now, we have to decide if we start using package-lock or not.

For the sake of stability, we might want to use it, especially now that we have dependabot helping us for the updates.

Maybe, we should wait that we reduce the number of updates before we start using it

@tomap
Copy link
Contributor

tomap commented Dec 13, 2018

Hey, hexo build are failing. I don't want to jump to conclusions, but it might be caused by an npm update, which we cannot control without .lock file.
Dependabot seem capable of handling that. Maybe we should start using those to avoid situation where we are unsure why the build broke

@tcrowe
Copy link
Contributor

tcrowe commented Dec 13, 2018

Can you give an example of where it's failing? I will go look through the logs.

@tomap
Copy link
Contributor

tomap commented Dec 13, 2018

@yoshinorin
Copy link
Member Author

@tomap @tcrowe
The CI failed reason may not caused by lock file. I open #3392 about that.

@curbengh
Copy link
Contributor

curbengh commented Jul 1, 2019

Though personally I'm not so keen on package-lock, I can live with it.

I suggest to include it as part of hexo@4, after all deps have been updated. Currently many deps are blocked by (depends on?) #3598.

As part of this milestone?

@tomap
Copy link
Contributor

tomap commented Jul 1, 2019

1/ in general, I would not use yarn.lock, but package-lock json. yarn.lock is "deprecated in favor of the other

2/ In general, I would not use package-lock.json in any plugin. Each hexo user can decide or not to use a lock file on it's own repo, to have reproductible builds over time

3/ In some rare cases, temporarily, we could commit lock file to be able to fix known vulnerabilities using npm audit --fix, which I did, in some repos

4/ hexo-cli is a special case where it is not possible for the user to decide or not to use the lock file. In that case, it is possible that a dependency update breaks the package, and thus the ci of all hexo users. It occurred few months ago

5/ in any cases, the different cases should be advertized to inform user of the expected behavior

@ertrzyiks
Copy link
Contributor

ertrzyiks commented Jul 2, 2019

A lock file in library repo is more for developers than end users. Lock files of dependencies are ignored on installation anyway, the only relevant lock file is the one created in the root folder on the users' side. We can't even publish package-lock json file to npm registery.

According to https://docs.npmjs.com/files/package-lock.json

One key detail about package-lock.json is that it cannot be published, and it will be ignored if found in any place other than the toplevel package

To protect maintainers from breaking change installed in their unprotected projects, I would keep package-lock.json in repositories. When I contribute to my repos once a month, I'm not going to spend the first hours with the old project hunting issues with broken devDependencies. I would rather spend that time productively on features/bugfixes.

For my personal projects, I moved to yarn because of npm's problems with version resolution I encountered twice, so I would even vote for `yarn.lock. But to have any lock file is better than nothing.

hexo-cli can be installed locally and pinned with lock file too. Then it can be used like yarn hexo [command] or some npm fallback, like "scripts".

I prefer the flow with lock file committed and dependency bot updating the versions so I can switch to new version deliberately with care, rather than risking broken development experience every time I run yarn or npm install in the newly cloned repo.

@stevenjoezhang stevenjoezhang changed the title Does we need exclude yarn.lock and package-lock.json from all repository ? Do we need exclude yarn.lock and package-lock.json from all repository ? Apr 26, 2024
@stevenjoezhang
Copy link
Member

I will close this issue because I believe the problem discussed has been resolved. In practice, many Hexo plugin repositories do not retain the package-lock.json, but it has been shown that we have hardly encountered issues due to inconsistent dependency package versions.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
dependencies Pull requests that update a dependency file discussion
Projects
None yet
Development

No branches or pull requests

8 participants