async (ctx, next) => await next()

[jonathanong]

closes #530

please test this. if you guys wanna benchmark this, that would be helpful too!

• update docs
• add end-to-end testing w/ babel
• publish and set koa-compose version
• add a warning to koa@1 about the middleware signature change

[jonathanong]

oops

[niieani]

❤️

[gyson]

fixed a few issues in #531

// ok
app.use(function (ctx, next) {
  // ...
})

// ok
app.use(co.wrap(function* (ctx, next) {
  yield next()
})

// ok (legacy generator middleware)
app.use(function* (next) {
  yield next
})

// not ok
app.use(function* (ctx, next) {
  // ...
})

[jonathanong]

are you guys okay with above? supporting legacy middleware but not the following?

app.use(function* (ctx, next) {
  // ...
})

i'd personally prefer to only support (ctx, next).

[gyson]

How about this ?

// ok
app.use(function (ctx, next) {
  return next()
})

// ok
app.use(async function (ctx, next) {
  await next()
})

// ok, implicitly wrap generator function using `co.wrap`
app.use(function* (ctx, next) {
  yield next()
})

// ok, legacy support, may have a better name ? `app.useGenerator` ?
app.useLegacy(function* (next) {
  yield next
})

[travisjeffery]

i'd personally prefer to only support (ctx, next).

👍 to this since we're breaking the api anyway.

[gyson]

Actually, I am 👍 to only support (ctx, next).

It's also very easy to support legacy middleware like this:

app.use(convert(function* (next) {
  yield next
}))

[ide]

The key APIs to support are async (ctx, next) => {} (modern) and function*(next) { this === ctx } (legacy). We should get (non-async) (ctx, next) => {} and function(next) => { this === ctx } for free too.

I suspect anything else like async (next) => {} or function*(ctx, next) {} is going to add more fragmentation in the long run. The smaller API makes the migration guide pretty straightforward.

[fengmk2]

@jonathanong Should keep old style middleware function*(next){ this } continue work.

There're so many koa-xx modules, we should think about that. It's hard to change all those modules.

[dead-horse]

Should keep old style middleware function*(next){ this } continue work.

we should keep all the koa's middlewares working both in koa@1 and koa@2.

[ide]

If Koa 1 generators end up being slow or expensive you could add a console warning (that could be disabled via a flag) when people use the legacy generator API. But definitely good to have a deprecation phase before removing a core API.

[felixfbecker]

+1 for supporting both but showing deprecation warnings. We could also look at function.length to check if the function takes only one parameter and if yes, call it with this/next. I know this is dirty but it is just for the transition phase. Or if we don't "support" the old style, still check function.length to directly throw so people know why their middleware doesn't work anymore.

[atian25]

+1 for support both.
dirty inside koa is better than dirty at every koa-xx modules to support koa@1 and koa@2

[felixfbecker]

@atian25 exactly.

[smcmurray]

@felixfbecker a middleware function with a single argument could very well be the new style:

(ctx) => {}

So checking function.length doesn't help.

[felixfbecker]

@smcmurray You're right of course. Is backwards compatibility even possible then? We would have to assume that all generator functions work with this and normal (asnyc) functions work with (ctx, next). Which means current middleware that uses normal (async) functions will break.

[gyson]

@felixfbecker We can convert any function* (next) { ... } to (ctx, next) => { ... }. You can check koa-convert about how it works.

[gyson]

I suspect anything else like async (next) => {} or function*(ctx, next) {} is going to add more fragmentation in the long run.

I am kind of worried about fragmentation.

@jonathanong I think we would deprecate app.use(function* (ctx, next) { ... }) API when ES7 async/await function is available natively. It may not be a good idea to support a feature which would be deprecated in long run at the beginning.

Just checked the status of ES7 async/await in V8 at here. Really hope it lands ASAP (may we should vote / star for it?)

[ide]

Re: V8 and Node. Babel is really good now and can convert async functions to generators (or to plain functions - your choice), or you can simulate an async function with co.wrap + generators. Async functions are production-ready today.

[niieani]

I'm curious, why did we switch to having the context passed in as a ctx param, rather than having the middleware method bind the context to this?
I thought this was more elegant and made more sense.

[catherinekassim]

The reason for not binding this was presented here: #415 (comment)
I think it'll simplify use-cases, like mine. I've used a wrapper method to be able to put my middleware in a consistent class. E.g. you would be able to call from one method to the other within one class: https://github.com/catherinekassim/koa-router-metadata#example

[smcmurray]

First, thank you @jonathanong for this branch. It is very helpful.

Second, I get a stack overflow because co gets caught in a spiral between toPromise() and objectToPromise().

[jonathanong]

@smcmurray do you have a test case?

[smcmurray]

import koa from 'koa';
import path from 'path';
import logger from 'koa-logger';
import stylus from 'koa-stylus';
import nib from 'nib';
import serve from 'koa-static';

var app = new koa();

app.use(logger('dev'));
if ('development' == app.env) {
  app.use(stylus(path.join(__dirname, 'public')));
}
app.use(serve(path.join(__dirname, 'public')));

/// catch 404 and forwarding to error handler
app.use((ctx) => {
    var err = new Error('Not Found');
    ctx.throw(err, 404);
});

export default app;

[jonathanong]

like this? 77f1d2a

oh you know what. those middleware have the old signature, so they aren't going to work right now.

[jonathanong]

anyone have any ideas how to support the old middleware style? i think promoting koa-convert or having .useLegacy() could be helpful.

i'm against passing different arguments based on whether a function or a generator function is passed though.

[jonathanong]

otherwise, i can push an alpha as soon as i have time

[ide]

A separate module like koa-convert seems preferable because in Koa 3 (for example) when useLegacy is removed, people won't have to go around updating all of their useLegacy call sites if koa-convert is already letting their middleware work with koa.use().

i'm against passing different arguments based on whether a function or a generator function is passed though.

Fair point. What do you think of this set up?

Modern API:

koa.use(async (ctx, next) => { ... })
// or when you don't need to await anything for example:
koa.use((ctx, next) => { ... })

Legacy API:

import convert from 'koa-convert';
koa.use(convert(function*(next) { /* this === ctx */ }));
// or for synchronous functions that don't need to yield 
koa.use(convert(function(next) { /* this === ctx */ }));

This is a breaking change, but with a clear migration path (I think? maybe there are edge cases we'll find).

Publishing 2.0.0-yolo.1 would be great. If you believe koa-convert is ready I can upgrade our exponentjs servers to Koa 2 and if it goes well the commits can show how you might migrate from Koa 1 to 2.

[gyson]

Maybe only support modern API in Koa 2.0 and people can override .use if they need for migration:

const use = app.use
app.use = (mw) => use.call(app, convert(mw))

[tj]

It's probably sane to have some kind of deprecation warning for 2.x personally, then people at leasta have a chance to migrate the deps, apps themselves would be a quick scripted refactor but publishing all the modules back to npm is a big pain

[ide]

Re: deprecation warning -- I'm not sure how to programmatically distinguish between old and new middleware, especially if some people are using Babel to convert generator functions to regular functions.

Migration:

Based on my understanding of koa-convert (I haven't read the code) it converts today's middleware into new middleware with the async (ctx, next) signature. On the caller's side (the app) the converted middleware is indistinguishable from other new middleware. And on the callee's side (the middleware and anything it invokes) it still runs in a generator and has access to the Koa context via this.

So I'm assuming that if Koa 2 ships without support for Koa 1 middleware (and potentially without support for generators at all, for that matter), there's a straightforward migration path:

1. Use something like jscodeshift or find & replace to convert app.use(<anything>) to app.use(convert(<anything>)).
2. Middleware authors update their middleware from function*(next) to async (ctx, next) and publish to npm with a major semver bump.
3. Koa users upgrade their npm deps and convert app.use(convert(<old middleware>)) to app.use(<new middleware>).

[fengmk2]

If most of current apps using koa 1.x, all the koa middlewares authors need to maintain two branchs to support koa 1.x and 2.x two.

But we need to move on, koa-convert seem the better than useLegacy.

[gyson]

@ide in Koa v0.x and v1.x (without experimental flag), app.use has an assertion that all (legacy) middleware must be generator function and it's tested with fn.constructor.name == 'GeneratorFunction' at here.

Therefore, we can distinguish old and new middleware with fn.constructor.name == 'GeneratorFunction'.

[ide]

@gyson Cool, good to know! Then maybe we can support old functions too (include koa-convert with Koa for Koa 2, and remove it in Koa 3?).

[tejasmanohar]

If most of current apps using koa 1.x, all the koa middlewares authors need to maintain two branchs to support koa 1.x and 2.x two.

But we need to move on, koa-convert seem the better than useLegacy.

👍 I agree w/ @fengmk2. Most of the Koa users before were on an experimental version of Node anyways. I honestly think we need to take this and move fast without holding back for koa 1.x.x backwards-compatibility but following semver so it's all clear what goes back / forward to what.

[felixfbecker]

+1 for koa-convert. We could also name it koa-legacy.
For deprecation warnings, consider this: The new koa works with promise-returning functions. Therefore, we can show a deprecation warning everytime it is called with a generator function, which most of the middleware will be. Also, we can show a deprecation warning if a function does not return a promise (an async function always returns a promise, even if it doesn't await next). This will work with babel-transpiled generators. The warning could look like "Generator functions are deprecated in favor of promises/async functions. Use koa-convert to convert legacy middleware."

[jonathanong]

cool i think the consensus is to not include koa-convert

[jonathanong]

merging and releasing an alpha, then putting the pending issues into separate issues. i'd really like help on docs!

[ide]

I'll help proofread if someone wants feedback on the content and how the docs read.