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

Add option --lockfile to include package-lock.json content changes #3

Closed
andidev opened this issue Dec 17, 2021 · 7 comments
Closed

Comments

@andidev
Copy link
Contributor

andidev commented Dec 17, 2021

Thanks for the improvements of install-changed package!
Would be nice to have a way to include package-lock.json file and package.json deps changes.

andidev added a commit to andidev/package-changed that referenced this issue Dec 17, 2021
andidev added a commit to andidev/package-changed that referenced this issue Dec 17, 2021
andidev added a commit to andidev/package-changed that referenced this issue Dec 17, 2021
@andidev andidev changed the title Add option --lockfile to make hash based on lockfile content Add option --lockfile to include package-lock.json content changes Dec 17, 2021
@andidev
Copy link
Contributor Author

andidev commented Dec 17, 2021

PR is submitted

@thdk
Copy link
Owner

thdk commented Dec 17, 2021

Hi, do you have a use case where this is needed?

@andidev
Copy link
Contributor Author

andidev commented Dec 17, 2021

@thdk
Well in case someone removes package-lock.json and reinstalls to get new patch updates from ^dependencies.

4 thumbs up in cloned repo for this feature.
ninesalt/install-changed#18

I think checking package.json and package-lock.json is the correct thing to check to be honest and not just package-lock file as mentioned in feature request.

@thdk
Copy link
Owner

thdk commented Dec 18, 2021

(I have reviewed your PR)

Just curious about how people are using this library.

Note that there should never be any good reason for deleting your package-lock.json. If I am not mistaken, running npm install will use the latest patch version of your dependencies and update your lock file accordingly.

All other version upgrades can be managed with running npm audit followed by another npm install.

In the first scenario, it might be possible that you end up with two different version (patches) of a dependencies being used by developers of your application. It shouldn't be a big problem and worst case scenario you need to remove .packagehash file and run package-changed again or simply run npm install instead of package-changed.

A force option would solve this nicely. So that the current .packagehash file is ignored and a fresh install happens which would update the .packagehash file.

We are no longer using package-changed in our team since we have implented workspaces. We migrated to yarn workspaces but npm v7+ also has workspaces support. package-changed has no support for workspaces and there is honestlty no big need for using package-changed with workspaces since running a second install takes just a few seconds now.

So feel free to share your use more in detail, there might be other options than using this library.

@andidev
Copy link
Contributor Author

andidev commented Dec 18, 2021

Well my main point for this change is this:
the plugin should respect if any package dependency has changed or not regardless if you use node the right way or not.
So if running npm install will lead to a change in installed dependencies the plugin should respect that (if possible).
So that said we should cover the following:

  1. package.json changes since that will lead to new dependency getting installed
  2. package-lock.json has changed since that will lead to new dependency to install

Note that there should never be any good reason for deleting your package-lock.json. If I am not mistaken, running npm install will use the latest patch version of your dependencies and update your lock file accordingly.

Maybe you are right, but it could still happen that a developer does that. And if it does should plugin not respect that because its a bad practice doing so? does not make sense to me.

All other version upgrades can be managed with running npm audit followed by another npm install.

Ok this is actually an argument to check package-lock.json. If you run npm audit fix in package-changed repo. You will notice that package.json is not updated but package-lock.json is updated. So why should package-changed not discover a change? Because when package-lock.json is committed and code is pulled by another developer, it would be required to run npm install to get correct dependencies installed.

In the first scenario, it might be possible that you end up with two different version (patches) of a dependencies being used by developers of your application. It shouldn't be a big problem and worst case scenario you need to remove .packagehash file and run package-changed again or simply run npm install instead of package-changed.

Why should plugin not handle this? Isn't this the reason of the plugin?

@andidev
Copy link
Contributor Author

andidev commented Dec 18, 2021

Now this is what we are trying to achieve:

  1. A developer checks out some branch and runs npm run start without running npm install since they are not aware that installed dependencies has changed. We want to make sure they run code with correct dependencies and install dependencies automatically.
    Solution: we add a script to run package install with options --lockfile, npm install and echo dependencies are updated
"start:localhost": "npm run package-changed:npm-install && npm run start",
"package-changed:npm-install": "npx --yes package-changed run \"echo \\\"\\\\033[0;32mDEPENDENCY CHANGES DETECTED running 'npm install' since package.json dependencies or package-lock.json has changed\\\\033[0m\\\" && npm install\" --lockfile",

npm run start 2021-12-17 12-24-17

  1. A developer checks out some code that requires npm install (package.json or package-lock.json is changed).
    We want them to get informed that new dependencies needs to be installed. We don't want to install them because that could be annoying if they are not gonna run code.
    Solution: we create a post-checkout githook where we run package-changed with options --lockfile --skip-write-hash and echo warning dependencies needs to be install. We don't want to update hash since we want npm install to be triggered by 1).
#!/bin/sh
. "$(dirname "$0")/_/husky.sh"

red='\033[0;31m'
no_color='\033[0m'
npx --yes package-changed run "echo \"${red}WARNING run 'npm install' since package.json dependencies or package-lock.json has changed${no_color}\"" --lockfile --skip-write-hash

anders@airsomtoranders~Workspaceswedishtechnologiesletseatmanager 2021-12-17 12-17-28

  1. A developer pulls or merges or pulls some code that requires npm install. We want to run npm install to make sure developer installs and runs correct dependencies automatically.
    Solution: we create a post-merge githook where we run package install with options --lockfile, npm install and echo dependencies are updated.
#!/bin/sh
. "$(dirname "$0")/_/husky.sh"

yellow="\033[0;32m"
no_color='\033[0m'
npx --yes package-changed run "echo \"${yellow}DEPENDENCY CHANGES DETECTED running 'npm install' since package.json dependencies or package-lock.json has changed${no_color}\" && npm install" --lockfile

anders@airsomtoranders~Workspaceswedishtechnologiesletseatmanager 2021-12-17 12-14-58

@thdk
Copy link
Owner

thdk commented Dec 18, 2021

This has been released in 1.8.0

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

2 participants