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 hooks befor/after execute run func #98

Closed
1 task done
tmg0 opened this issue Oct 31, 2023 · 6 comments
Closed
1 task done

Support hooks befor/after execute run func #98

tmg0 opened this issue Oct 31, 2023 · 6 comments

Comments

@tmg0
Copy link

tmg0 commented Oct 31, 2023

Describe the feature

Add defs in defineCommand function like interceptors or middlewares

example:

const customInterceptor1 = (ctx: CommandContext) => false
const customInterceptor2 = (ctx: CommandContext) => console.log('before run.')

defineCommand({
  meta: {
    name: "hello",
    version: "1.0.0",
    description: "My Awesome CLI App",
  },
  interceptors: [customInterceptor1, customInterceptor2]
  run({ args }) {
    // output: before run.
    // and this code will not be execute
    console.log(`${args.friendly ? "Hi" : "Greetings"} ${args.name}!`);
  },
})

Additional information

  • Would you be willing to help implement this feature?
@tmg0 tmg0 mentioned this issue Oct 31, 2023
8 tasks
@NozomuIkuta
Copy link
Member

It sounds reasonable by itself to support a way to run some logic before/after main logic.

Actually, other UnJS packages also have "hooks" options (e.g. Nitro) or onXYZ interceptors options (e.g. h3, ofetch) to do something before/after some logic.

The difference between these and your code is that your interceptors can abort the command execution, rather than making a side effect and/or modifying options passed.

So, to clarify, do you want to have "validators" to determine if the command should be run, and want to separate them as another option instead of writing them inside of run method body for better code organization?

@tmg0
Copy link
Author

tmg0 commented Oct 31, 2023

Actually what I need is a hook, such as checking the version of the CLI before/after run the main logic with a better code organization.

I'm not sure if determining whether to abort through the return value is a good idea, but it seems quite convenient.

Maybe add a abort callback function in CommandContext is a better way?

e.g.

const before= ({ abort }: CommandContext) => {
  const isLatest = version === "1.0.0"
  if (!isLatest) { abort() }
}

const after= () => { console.log('after run.') }

defineCommand({
  meta: {
    name: "hello",
    version: "1.0.0",
    description: "My Awesome CLI App",
  },
  before,
  run({ args }) {
    console.log(`${args.friendly ? "Hi" : "Greetings"} ${args.name}!`);
    // output: after run
  },
  after,
})

@tmg0
Copy link
Author

tmg0 commented Oct 31, 2023

Emmmmm, It seems unnecessary to add a abort callback, in before hook can just throw an error to terminate the logic.

@tmg0 tmg0 changed the title Support interceptors befor execute run func Support hooks befor/after execute run func Oct 31, 2023
@peterroe
Copy link
Contributor

It seems like your needs can be completely solved by setup/cleanup, maybe we need an idea with a clearer function orientation 👀

@tmg0 tmg0 closed this as completed Oct 31, 2023
@jgoux
Copy link

jgoux commented Mar 21, 2024

A middleware system would be super useful for stuff like telemetry:

const telemetryMiddleware = async (ctx, next) => {
  await telemetry.captureEvent(`$command:${ctx.cmd.meta.name}:start`); 
  await next(ctx);
  await telemetry.captureEvent(`$command:${ctx.cmd.meta.name}:end`);
}

defineCommand({
  meta: {
    name: "hello",
    version: "1.0.0",
    description: "My Awesome CLI App",
  },
  middlewares: [telemetryMiddleware],
  run({ args }) {
    console.log(`${args.friendly ? "Hi" : "Greetings"} ${args.name}!`);
  },
})

The idea would be that the run handler would be passed to the middleware chain defined in middlewares.

@pi0 pi0 mentioned this issue Mar 21, 2024
@pi0
Copy link
Member

pi0 commented Mar 21, 2024

@jgoux having a context-full api makes sense. I think i have a better idea inspired from your usecase to introduce with plugins ~> #130

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 a pull request may close this issue.

5 participants