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 hooks for life cycle events: using .hook() #1514

Merged
merged 26 commits into from May 18, 2021

Conversation

shadowspawn
Copy link
Collaborator

@shadowspawn shadowspawn commented May 7, 2021

Pull Request

Problem

People ask for a way to run code before the action handler with access to the parsed options. And also some requests for a hook after the action.

Main issue: #1197
Related issues: #76 #229 #936 #996 #1158 #1457

Solution

Add .hook(event, callback) method

  • supports async callbacks
  • supports multiple hooks for same event on single command

Supported hooks: preAction, postAction. Triggered by action handler for hooked command and its subcommands.

Callback is passed the hooked command and the command running the action handler.

ChangeLog

  • add .hook() with support for "preAction" and "postAction" callbacks

@shadowspawn shadowspawn changed the title Feature/hook Add hooks for life-cycle events May 7, 2021
@shadowspawn shadowspawn changed the title Add hooks for life-cycle events Add hooks for life cycle events May 8, 2021
@shadowspawn

This comment has been minimized.

@shadowspawn
Copy link
Collaborator Author

shadowspawn commented May 11, 2021

Why not preHandler or afterHandler to also cover executable? The stand-alone executable seems quite different, and I have been avoiding tackling! Initial concerns without deep digging:

  • the command which is about to be "executed" is a dummy command
  • the command object which is about to be executed does not currently have .args set
  • the program gets terminated when the subprocess finishes (?), but could perhaps run afterHandler before exit? But what about async?

@shadowspawn shadowspawn changed the title Add hooks for life cycle events Add hooks for life cycle events: using .hook() May 13, 2021
@shadowspawn shadowspawn reopened this May 16, 2021
@shadowspawn shadowspawn marked this pull request as ready for review May 16, 2021
@shadowspawn shadowspawn added this to the v8.0.0 milestone May 16, 2021
@shadowspawn
Copy link
Collaborator Author

shadowspawn commented May 17, 2021

As noted in #1519 (comment), the .hook() api signature is similar to that used by Fastify:

https://www.fastify.io/docs/latest/Hooks/

Copy link
Collaborator

@abetomo abetomo left a comment

LGTM

@shadowspawn
Copy link
Collaborator Author

shadowspawn commented May 18, 2021

Thanks @abetomo

I'll take another look at the code refactor next, and then focus on a prerelease of Commander v8.

@shadowspawn shadowspawn added the pending release label May 18, 2021
@shadowspawn shadowspawn merged commit c20284d into tj:release/8.x May 18, 2021
12 checks passed
@shadowspawn shadowspawn deleted the feature/hook branch May 18, 2021
@shadowspawn shadowspawn removed the pending release label Jun 25, 2021
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

Successfully merging this pull request may close these issues.

None yet

2 participants