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

middleware doesn't work as expected when used in isolation #1351

Closed
bcoe opened this issue May 20, 2019 · 5 comments · Fixed by #1852
Closed

middleware doesn't work as expected when used in isolation #1351

bcoe opened this issue May 20, 2019 · 5 comments · Fixed by #1852

Comments

@bcoe
Copy link
Member

bcoe commented May 20, 2019

middleware doesn't work as I'd expect for yargs.argv; I would expect the middlware runs, it does not.

@bcoe bcoe added the bug label May 20, 2019
@wesleytodd
Copy link

wesleytodd commented Jun 15, 2019

I just ran into this, and here is my feedback:

  • Should run even if no command is matched
  • Should run for default command (it does not right now)

Right now I am doing the following:

yargs()
  .middleware(common)
  .command('$0', 'default', {}, (argv) => {
     common(argv)
    // ...
  })

function common (argv) {
  // common mw
}

This works, but it took me a bit to figure out why the middleware was not being run.

I looked into submitting a PR, but I dont have time right now to fully learn this codebase :(. If I am pointed in the right direction I might be able to work it out, but at first read I was unsure where to make the change.

@bcoe
Copy link
Member Author

bcoe commented Jun 17, 2019

@wesleytodd would very much appreciate help, I can at least point you at the right files when I have some cycles.

@mleguen
Copy link
Member

mleguen commented Dec 18, 2019

@wesleytodd I tried to reproduce this issue with your code and master branch:

  • your middleware is called as expected with default command (and it is called twice with your workaround)
  • middleware is not run when no command is matched... but the doc for middleware() states that the middlewares are called for commands only

So I do not think there is something to correct here.

@bcoe
Copy link
Member Author

bcoe commented Nov 22, 2020

When a default command is used, middleware works as expected on the latest release (Refs: #1813).

I would like to make it so middleware is applied even if you don't specify a default command, perhaps we could refactor the codebase to always have a default command registered.

@bcoe
Copy link
Member Author

bcoe commented Jan 11, 2021

Refs: #1823

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants