compose icon indicating copy to clipboard operation
compose copied to clipboard

Add support for a wrapper array in context

Open PlasmaPower opened this issue 8 years ago • 11 comments

This is a new way to implement wrappers (another one was #51).

PlasmaPower avatar Mar 24 '16 16:03 PlasmaPower

Tests should be passing now, had some linting problems.

PlasmaPower avatar Mar 24 '16 17:03 PlasmaPower

It sure is less invasive compared to #51, even though it too could be iterable. There's some risk of overuse in user land I guess.

fl0w avatar Mar 24 '16 19:03 fl0w

If this merges (with the koa PR) then I'll make a PR for koa-mount, and make a PR to revert the deprecated generators in koa and add wrapper support for koa-convert.

PlasmaPower avatar Mar 24 '16 19:03 PlasmaPower

It's a little weird to have dependency from within context in compose though, isn't it?

fl0w avatar Mar 24 '16 19:03 fl0w

I guess, but I'm not sure that has much of a practical implication. I've already checked that context.wrappers exists, so it shouldn't matter if the context is not standard.

PlasmaPower avatar Mar 24 '16 19:03 PlasmaPower

The code now throws an error if it doesn't end up with a function (or it ends up with a generator function). I think now that we have wrappers, it makes sense for that to be here. However, this makes the code a bit long. Should we abstract lines 43-52 into it's own module (e.g. koa-call-middleware)? This would also solve a problem I had earlier where mount didn't warn me that I had passed it generator middleware by accident, which broke my code.

PlasmaPower avatar Mar 24 '16 19:03 PlasmaPower

We could also create context.callMiddleware(middleware, context, next). Edit: That further cements the context dependency though.

PlasmaPower avatar Mar 24 '16 20:03 PlasmaPower

Okay, I've consolidated the code into another module: https://github.com/PlasmaPower/call-middleware I haven't published the module to NPM yet, and the module references itself as if it was part of koajs. That's why the build is failing. My plan is someone can clone that repo, push it to a koajs repo (forking it removes the issues tab), make any needed changes, publish the module, then the build will pass and this can be merged. I've already checked that with the module there the build passes.

PlasmaPower avatar Mar 25 '16 03:03 PlasmaPower

is this taking .wrappers from the context? if so, -1

jonathanong avatar Mar 28 '16 22:03 jonathanong

Yep. It helps preserve the wrappers and ensure the compatibility, but it also makes compose dependent on context to some extent. The alternative is another paramater, like @fl0w implemented. I prefer the former because it's better for the user, but the latter is less opinionated.

PlasmaPower avatar Mar 28 '16 22:03 PlasmaPower

Rebased. Remember tests are failing because they rely on koa-call-middleware which isn't published yet.

PlasmaPower avatar Mar 08 '17 20:03 PlasmaPower