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

Support appending additional arguments #30

Open
dizlexik opened this issue Jan 22, 2019 · 5 comments
Open

Support appending additional arguments #30

dizlexik opened this issue Jan 22, 2019 · 5 comments

Comments

@dizlexik
Copy link

Would it be possible to add the ability to accept additional arguments so I could do something like this:

"husky": {
  "hooks": {
    "post-checkout": "yarnhook --ignore-optional",
    "post-merge": "yarnhook --ignore-optional",
    "post-rewrite": "yarnhook --ignore-optional"
  }
}

I'd really like to prevent yarnhook from installing optional dependencies.

@frontsideair
Copy link
Owner

frontsideair commented Oct 16, 2020

I think adding --ignore-optional (or comparable) to the default set of flags makes sense since they're optional and it'll make git operations faster.

I'm on the fence about adding arguments though, since—despite the name—yarnhook is package manager independent. I'm open to changing my mind but I'll make the --ignore-optional change regardless.

@zkochan
Copy link
Contributor

zkochan commented Jan 11, 2021

I don't think this was a good idea. If someone wants to influence how the package manager is behaving itself, they can just create a config file for that package manager with settings. So if I want to skip optional dependencies, I would create a .npmrc with this content:

optional=false

Now because the CLI flags have the biggest priority, there is no way to include optional dependencies. I need optional dependencies to be installed. So now I have to always rerun pnpm install after switching branches. So yarnhook loses its benefits in my case.

@qnp
Copy link

qnp commented Feb 25, 2022

@zkochan 100% agreed. In case of nuxt3, esbuild needs to be installed in the correct platform and is done using optionalDependencies so everytime yarnhook runs, the nuxt3 project cannot be built until a new yarn install is run.

@frontsideair
Copy link
Owner

I came back to this and I believe it was not the right call to exclude optional dependencies by default. While it's important that branch switches to be fast, it's more important that yarnhook doesn't change the end result. It should be a core principle that yarnhook doesn't install different dependencies than when you install them manually with default flags.

I reviewed the current default flags and this seems like the only offending one. If we categorize them:

  • Preferring offline is fine, having internet connection shouldn't change behavior anyway
  • Pure/frozen lockfile is good actually, it should be the default (and it is in many cases)
  • No audit (npm only) is also good, it doesn't change the end result
  • No save (also npm only) is also good, switching branches shouldn't change files (not sure if npm install does it, need to check)

So the first thing I'll do is to remove --no-optional, adding a way to configure it may come later. Not sure if it'll be needed as @zkochan demonstrated. I had a major refactor in a branch, I can probably add providing the option to that scope.

@frontsideair
Copy link
Owner

I cut a new release, it's a major one due to the reasons mentioned in the release page. I hope this solves existing problems and please let me know if it breaks anything.

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

4 participants