farrow-js / farrow

A Type-Friendly Web Framework for Node.js
https://www.farrowjs.com
MIT License
768 stars 37 forks source link

Question(Pipeline): Should run `onLast` with same context with other middlewares? #111

Closed tqma113 closed 3 years ago

tqma113 commented 3 years ago
const createCurrentCounter = (hooks: Hooks, onLast?: (input: I) => O) => {
    return createCounter<I, O>((index, input, next) => {
      if (index >= middlewares.length) {
        if (onLast) return onLast(input)
        throw new Error(`Expect returning a value, but all middlewares just calling next()`)
      }

      const middleware = middlewares[index]
      const result = runHooks(() => middleware(input, next), hooks)

      return result
    })
  }

All middlewares run with runHooks but the onLast is not, is it a feature or could improve?

Lucifier129 commented 3 years ago

It could be improved as an opt-in feature when calling pipeline.run(context, options)

tqma113 commented 3 years ago

Emm, why is it a opt-in feature? I think it could a default feautre? It also not affects the code written before.

tqma113 commented 3 years ago

Just like this.

runHooks(() => onLast(input), hooks)
Lucifier129 commented 3 years ago

Yes, it can be a default setting, I mean we should offer an option to enable or disable.