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

Allow to chain event handlers #192

Closed
tobiasdiez opened this issue Oct 14, 2022 · 16 comments
Closed

Allow to chain event handlers #192

tobiasdiez opened this issue Oct 14, 2022 · 16 comments
Labels
enhancement New feature or request

Comments

@tobiasdiez
Copy link
Contributor

It would be nice if event handlers could be chained to create a new event handler that invokes each handler after each other. Something like:

chain(defineCorsHandler(...), eventHandler(event => ...)) // this is an event handler again

This is especially relevant for usage in nuxt, where one usually declares one event handler for a certain route and then in the declaration would like to specify cors options (using h3-cors) in parallel to invoking the actual handler.

Related: #177

@pi0
Copy link
Member

pi0 commented Oct 14, 2022

Hi @tobiasdiez. As explained in the relevant issue, h3 utilities like cors, are meant to be called functions on demand and in the body of the event handler as best practices.

Middleware style chaining is also possible using an app stack, createApp().use(corsMiddleware).use(actualHandler) which itself is an event handler if you prefer chaining. Have you tried this pattern?

For the purpose of pre/post handlers, we might improve defineHandler BTW probably by supporting something like this:

defineHandler((event) => {}, { before: [cors], after: [validate] })

Wdyt?

@tobiasdiez
Copy link
Contributor Author

Thanks for your answer!

As explained in the relevant issue, h3 utilities like cors, are meant to be called functions on demand and in the body of the event handler as best practices.

That works, except if you want to chain two event handlers that you don't really have control (in my case the cors handler, and a apollo-h3-server handler that I'm currently developing). It is also not so convenient if the cors handler sometimes should handle the complete request (preflight) but in other cases only modify the response.

Middleware style chaining is also possible using an app stack

Nice suggestion. Is this production ready or is the overhead of createApp too big? (creating an app sounds expensive, but since h3 is so lightweight...)

defineHandler((event) => {}, { before: [cors], after: [validate] })

Sounds like a good idea. I guess if you need multiple handlers you just add them to the array?

defineHandler((event) => {}, { before: [cors, auth], after: [validate] })

@notshekhar
Copy link

createApp is not working. ERROR: Invalid lazy handler result. It should be a function

@notshekhar
Copy link

@pi0

@notshekhar
Copy link

notshekhar commented Nov 23, 2022

import { createApp } from "h3"

export default function eventHandlers(...handlers: Array<any>) {
    const app = createApp()
    app.use(handlers.map((func) => defineEventHandler(func)))
    return app
}

ERROR: [nuxt] [request error] [unhandled] [500] Invalid lazy handler result. It should be a function:
at ./node_modules/h3/dist/index.mjs:618:17
at async Object.handler (./node_modules/h3/dist/index.mjs:681:19)
at async Server.toNodeHandle (./node_modules/h3/dist/index.mjs:745:7)

@notshekhar
Copy link

@pi0

@notshekhar
Copy link

@pi0 ?

@nopeless
Copy link

@notshekhar Currently the code for supporting lazy handlers is plaguing the interpretation of functions

The correct way would be something like

handlers.reduce((a, h) => a.use(eventHandler(h)), app);

@notshekhar
Copy link

notshekhar commented Nov 30, 2022

@nopeless I tried this and it worked

 handlers.reduce((a, h) => a.use(eventHandler(h)), app).handler

@nopeless
Copy link

@nopeless I tried this and it worked

 handlers.reduce((a, h) => a.use(eventHandler(h)), app).handler

Without the .handler it doesn't work?

@notshekhar

@notshekhar
Copy link

notshekhar commented Nov 30, 2022

@nopeless No it wasn't working without .handler

@nopeless
Copy link

@nopeless No it wasn't working without .handler

Oh, right, you were using nuxt. Yeah that should be the behaviour. I really think Nuxt should accept Router and App as well

Thanks for responding quickly

@ThomasBerne
Copy link

Found a solution that works here: #127

@nozomuikuta nozomuikuta added the enhancement New feature or request label Dec 12, 2022
@notshekhar
Copy link

notshekhar commented Jan 8, 2023

You can also try this I am using in my project, This function creates an event handler that executes all of the event handlers passed to it in the order they are received. The function returns the result of the first event handler that returns a value that is not undefined. If all of the event handlers return undefined, the function returns undefined as well. The event handler is asynchronous and awaits the result of each event handler before moving on to the next one. The event handler is also wrapped in another event handler called eventHandler.

function eventHandlers(...handlers: Array<Function>) {
  return eventHandler(async function chained(event: any) {
    let res = undefined;
    for (const func of handlers) {
      res = await func(event);
      if (res !== undefined) return res;
    }
    return res;
  });
}

@pi0
Copy link
Member

pi0 commented Jul 27, 2023

Hi. Sorry for not adding comments on this for long time. I belive this is something worth to support to allow hooks such as authentication, cors and validation to run before and after event handler but chaining them is probably not best solution. Instead we are gonna support an object syntax for event handlers with before and after hooks for fine gained control and also composibility. Please track via #424 and ping us here if you belive chaining handler might have it's own advantages comparing to object syntax to reopen discussion 👍🏼

@pi0 pi0 closed this as not planned Won't fix, can't repro, duplicate, stale Jul 27, 2023
@tobiasdiez
Copy link
Contributor Author

Is the before/after syntax only a different syntax or will it behave differently? That is, are the following equivalent?

defineEventHandler({ handler: d, before: [a,b,c], after: [e,f] })

chainEventHandlers([a,b,c,d,e,f])

If they are functionality-wise equivalent, then I'm indeed a fan of the before/after syntax as the ofther handlers are most likely defined globally somewhere else and only imported, and in this case this syntax is shorter and puts the focus on the actual event handler.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging a pull request may close this issue.

6 participants