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

Stuck build, many plugins affected. Any reason to gitignore package-lock.json? #13

Closed
gavvvr opened this issue Sep 8, 2021 · 4 comments

Comments

@gavvvr
Copy link

gavvvr commented Sep 8, 2021

The package-lock.json is not included to git repo by intention.
What is the reason? If you want stable build, you usually want to check in such lock-files to VCS.

Today I faced the problem with my Imgur plugin based on this template. My build has stuck. That's exactly because package-lock.json is not committed. The package.json is not enough, because it does not pin exact version of dependencies. With the ^-notation (example: "typescript": "^4.2.4") it allows actual minor and patch versions to be higher on npm install if newer version of the dependency is available on npm registry (see npm docs).

The build of this plugin's template is currently broken too. It will stuck forever in the very end after:

...
main.ts → ....
created . in 1.4s
# stuck here

Other plugins generated from this template and not having lock-file in git are affected too, i.e.:

I do not know the root cause of the problem. But from my experiments, the culprit of the stuck build with this Rollup setup is TypeScript 4.4.1+ which gets installed now on npm install without package-lock.json.

I think that the lock-file should not be ignored and must be committed to git.
Also another common mistake I see people make is: using npm install instead of npm ci on their automated builds. npm install leaves a possibility for lock-file to be updated on clean install if the situation on npm registry has changed. I think the right choice for CI should be the npm ci command with lock-file tracked by vcs to get stable reproducible builds.

@lishid
Copy link
Collaborator

lishid commented Sep 8, 2021

Perhaps we should pin the versions? I've specifically excluded package-lock.json because it's very finnicky and tends to get updated all the time. Internally, we pin all of our dependencies with package.json, but not the entire sub-dependencies tree.

@joethei
Copy link
Contributor

joethei commented Sep 9, 2021

From my research this seems to be the main culprit: rollup/rollup#4213

@gavvvr
Copy link
Author

gavvvr commented Sep 9, 2021

Hi @lishid

I've specifically excluded package-lock.json because it's very finnicky and tends to get updated all the time

This does not sound like a serious reason to ignore it. It is designed that way and the lock-file usually changes together with package.json (For me it is not: all the time, it changes as expected) or whenever you are ready to upgrade your dependencies. All the other time npm ci is the best choice, which won't edit your lock-file and will guarantee a consistent build and dependencies resolution.

Hard-pinning dependencies manually in package.json is an option, but looks more like a workaround. I see 2 disadvantages:

  • Everyone should remember that by default npm i --save will add dependency not hard-pinned, with a ^ prefix. And should always fix this manually.
  • By hard-pinning dependencies manually you miss the advantage of npm update command

My vote goes for committing lock-file to git


@joethei, thanks for sharing. I think you are right, this is the correct issue

@lishid
Copy link
Collaborator

lishid commented Nov 27, 2021

Should have been fixed by 3afc9d7

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

No branches or pull requests

3 participants