koajs / koa

Expressive middleware for node.js using ES2017 async functions
https://koajs.com
MIT License
35.07k stars 3.22k forks source link

koa 3.0 roadmap #1114

Open dead-horse opened 6 years ago

dead-horse commented 6 years ago

There are lots of PRs are blocked due to breaking change, and maybe it's time to make a plan for koa 3.0 now, if anyone has other suggestion, please post in this issue. :)

fl0w commented 6 years ago

Is it to early to create a v3 branch? I have some spare time coming up and wouldn’t mind doing some work.

dead-horse commented 6 years ago

All these pending PRs are not so emergency to me, so if anyone has any thoughts or some feature / refactor required, post in the issue to help us decide when to start create v3 branch. :)

fl0w commented 6 years ago

How about officially dropping support for < node.js 8?

iyuq commented 6 years ago

koa-compose could rewrite using async/await

iyuq commented 6 years ago

totally rewrite using TypeScript

fl0w commented 6 years ago

I'm personally against introducing TypeScript, adding external typings should be easy enough for those in need.

marswong commented 6 years ago

How about officially dropping support for < node.js 8?

koa-compose could rewrite using async/await

@fl0w @iyuq according to community practice, engines support should always follow the Node.js release schedule :)

marswong commented 6 years ago

totally rewrite using TypeScript

like what @fl0w said above, just one more thing: koa is quite vary from eggexpress and any other framework. It's just a pure http request middleware close to Node.js as you can see. So we can just feel free to write in pure JavaScript, follow the traditional convention and trust Node.js :)

jonathanong commented 6 years ago

if you have any breaking changes you'd like to introduce, please state them now :)

fl0w commented 6 years ago

Let's announce EOL for Koa v1.

zhangxiang958 commented 6 years ago

How about support HTTP/2 ? https://nodejs.org/api/http2.html

marswong commented 6 years ago

@zhangxiang958 koa already support it since 2.3.0, u can simply setup a koa app over HTTP/2 like this:

const fs = require('fs');
const http2 = require('http2');
const Koa = require('koa');

const app = new Koa();

app.use(ctx => {
  ctx.body = 'Hello Koa'; // or any other stream
});

const options = {
  key: fs.readFileSync('xxx.key'),
  cert: fs.readFileSync('xxx.crt'),
};
const server = http2.createSecureServer(options, app.callback());

server.listen(443);
hiroqn commented 6 years ago

How about publish koa and official middleware module under @koajs/ ?

urugator commented 6 years ago

Adjust naming to follow WHATWG URL Standard. Eg: request.path should be request.pathname etc

likegun commented 6 years ago

I think #1044 has already been solved by this commit

tiendq commented 6 years ago

I think context.res and context.req are not good names and they make people confused with context.response and context.request. Is it better if we can name them behind NodeJS names, so context.res becomes context.serverResponse and context.req becomes context.incomingRequest?

ng- commented 6 years ago

@tiendq I completely agree! I keep mixing up req/request and res/response. I vote for something like

{
req:<koa request>,
res:<koa response>:,
raw:{
   req:<node request>,
   res:<node response>
}}
damianobarbati commented 5 years ago

How to access the http2 stream on ctx object to push assets to client using koa? It this supported right now?

mike-tobia commented 5 years ago

I second @damianobarbati , would be great to get an implementation snippet that displays the compatibility and its usage. For example, would the stream be accessible through ctx.res.stream?

I've been seeing a bit of chatter on HTTP/2 support for Koa - I agree that we don't need to define config just to "enable" out of the box. What we do need however, is an interface to utilize HTTP/2, let's say if the request type is HTTP/2, maybe context could be formed at this stage.

I believe that the moment the Koa community takes a stance on HTTP/2, this would promote its usage for the next generation of servers built using Koa. To me, this is a great step forward.

What I envision, as a mode of compatibility and usage, is to add ctx.stream if the request is in an HTTP/2 context. As a start, we could yield the benefits of pushing from the stream and get a grip on how to manage push streams without ending the connection prematurely.

tajnymag commented 5 years ago

How about publish koa and official middleware module under @koajs/ ?

Why not simply @koa/?

Hiswe commented 5 years ago

Here is my little suggestions

[EDIT]

In fact everything's working perfectly. The application doesn't crash and we have a warn.
Up to us to fix that by sending the headers before next/nuxt rendering.

fl0w commented 5 years ago

Note, I am not privileged to make this decision. This is my personal opinion.

I'd like to chime in that I agree that namespace:ing npm-releases under npm:@koa is a good idea.

One or two caveats thought.

  1. Only for packages that reside in github:@koajs
  2. Don't namespace Koa core, i.e. npm i koa still resolves as expected.

Bug reports for 3rd party packages often end up in @koajs/koa#issues, which is burdensome for both maintainers and the user. This might clear things up a bit.

I also haven't considered possible implications of moving the packages (e.g. does npm have tooling for moving origins?).

maticrivo commented 5 years ago

+1 on #1044

mytecor commented 5 years ago

Please do normal error handling. The ability to set content type

catsalive commented 5 years ago

+1 on #998

dolsup commented 5 years ago

I suggest #1326

Hanggi commented 5 years ago

Is there any timeline or schedule for 3.0?

libook commented 4 years ago

Need some help?

tinovyatkin commented 4 years ago

I think removing next parameter from middleware and just awaiting all middleware can be most useful breaking change in 3.0. Debugging something like bellow is a nightmare:

app.use(async (ctx, next) => {
 // ... do something
 await next();
// ... do some more staff
})
app.use(async (ctx, next) => {
 // ... do something
 await next();
// ... do some more staff
})

while removing next will make composition simple and koa-compose will not be needed:

// koa-compose will not be needed, just do this:
  async handleRequest(ctx) {
    const res = ctx.res;
    res.statusCode = 404;
   try {
     for (const handler of this.middleware) {
        await handler(ctx);
     }
    respond(ctx);
   } catch (err) {
     ctx.onerror(err);
  } finally {
    // any finalisation logic here
  }
}

if something need to be guaranteed last handler then it can use events or better composition and .state

octet-stream commented 4 years ago

@tinovyatkin What if you need to do something after the next() call? For example, I'm using this scenario in multipart middleware:

import {parse} from "then-busboy"
import {unlink} from "promise-fs"

const unlinkFile = ({path}) => unlink(path)

const methods = ["post", "put"]

async function multipart(ctx, next) {
  if (!methods.includes(ctx.method.toLowerCase())) {
    return next()
  }

  if (!ctx.is("multipart/form-data")) {
    return next()
  }

  const body = await parse(ctx.req)

  ctx.request.body = body

  await next()

  // Cleanup files
  return Promise.all(Array.from(body.files.values()).map(unlinkFile))
    .catch(err => err.code !== "ENOENT" && Promise.reject(err))
}

export default multipart

How can I do so without next() as simple as this?

tinovyatkin commented 4 years ago

@octet-stream you mean if you ship your middleware as a module? With proposed #1395 you may do it easily:

app.use(async ctx => {
  if (methods.includes(ctx.method.toLowerCase() && ctx.is("multipart/form-data")) {
    ctx.request.body = await parse(ctx.req)

   ctx.app.once('responded', async ({ request }) => {
     // Cleanup files
   if(request?.body?.files) {
    try {
      await Promise.all(Array.from(request.body.files.values()).map(unlinkFile))
    } 
   catch (err) {
       if(err.code !== "ENOENT" ) 
      // you probably don't want to throw neither here, nor in your version
     // as files written by your middleware to the disk have nothing to do
    // with request handling
            throw err;
   }
  finally {
   delete requests.body.files;
  }
   }
 })
}
})

In your current implementation you actually deleting files from disk before response is sent to the network and response stream is destroyed, so, if these files have opened read stream you can get in trouble:

app.use(octetStreamMiddleware());
app.use(async (ctx, next) => {
  if (ctx.request.body.files && 'greatFile' in ctx.request.body.files)
    ctx.body = createReadStream(ctx.request.body.files.greatFile);
 next();
})

How would you clean up that @octet-stream?

fl0w commented 4 years ago

@tinovyatkin I don't see it.

We'd have to (again) ostracise the entire ecosystem. You're essentially removing an abstraction that is well understood for no real benefit. Personally I haven't had issues debugging current middleware signature and usage, and there's also no evidence that this is a general issue either (just from experience by idling in Koa issues).

It feels like a change for the sake of change? But maybe I'm missing something?

tinovyatkin commented 4 years ago

@fl0w see my response and example for @octet-stream - developers tends to think that after awaiting next it's safe to cleanup resource while, in fact, the response is not even sent to the network at that point. Removing next will force to think the whole serving lifecycle right.

While I've started with Koa about 4 years ago one of the hardest things to understand was what happens if I don't call or await for next? and I still doesn't sure in it :-)

octet-stream commented 4 years ago

In your current implementation you actually deleting files from disk before response is sent to the network and response stream is destroyed, so, if these files have opened read stream you can get in trouble

Sure, in actual code I would decide to cleanup files out of a request processing, because of something here will crash – the request will be also crash. It was just an example of a case where you need to do something after the next middleware is called, or maybe before respond. And the question was how to do so if your proposal will be added? Guess I need to clarify my thoughts better :)

tinovyatkin commented 4 years ago

@fl0w what will happen if in above example by @octet-stream he will forget to await the next and just call next()? It will very hard to find race condition.

The benefit here is clear message to developers - middleware composition are sequential and you can't jump in the middle of another middleware runtime

tinovyatkin commented 4 years ago

@fl0w if the next is well understood, following are some easy questions:

  1. If I will not call next in my middleware is it guaranteed than no middleware will be run after my processing?

  2. If I call, but not await next - will I start parallel request processing with this and whole middleware stack will wait for my middleware to finish anyway?

iyobo commented 4 years ago

I would rather have next stay as is. I would agree that taking it out would be a pointless change.

There is absolutely no issue with debugging middleware in koa. And Whatever perceived micro-issues might exist can be worked around and is certainly not worth upheaving the whole ecosystem.

I don't even like looking at the alternative you posted @tinovyatkin for octet's question of what "what happens if I want to run something else after?"... Which is a far more common occurrence with middleware than what you seem to be getting at.

tinovyatkin commented 4 years ago

@iyobo the example @octet-stream posted most probably will result in race condition if a next middleware will open a read stream on files. Making developers understand that is exactly the point of removing next

fl0w commented 4 years ago

@tinovyatkin there was a discussion about missing await next() but I think it ended in agreement that it is impossible to protect users from common JS mistakes.

You're mixing streams, promises and events to make a case, and I can understand it gets confusing, but that's not the fault of Koa tbh. Personally, I find it quite simple. When the topmost promise resolves Koa tries to respond.

developers tends to think that after awaiting next it's safe to cleanup resource while, in fact, the response is not even sent to the network at that point

That might or might not be true, I'll argue it's not as big of a problem as you're asserting. Koa 2 has been out for quite some time. I'd say I've been pretty active answering general usage questions, both in issues here and on slack and I don't see the evidence to support your case (others might disagree). Had this been a major issue it would have popped up.

If I will not call next in my middleware is it guaranteed than no middleware will be run after my processing?

Unless you do something hacky, sure. Why would it? It's how promises and regular functions work. If you don't call it, it will not be called (maybe I misunderstood your question here?)

If I call, but not await next - will I start parallel request processing with this and whole middleware stack will wait for my middleware to finish anyway?

If you call a promise but don't return, chain or await it'll run out of context (which isn't equivalent to parallel btw). Notice that the explanation has nothing to with Koa but rather the atomics of how a promise works.

~In your response to @octet-stream you propose using events (I haven't tried your PR nor your example). To me this reads like you just introduced a bug because you're trying to respond in the event handler which is out of context. Koa doesn't "care" if these events exists or not.~. Alright you're not mutating ctx while awaiting within the handler, nvm.

I don't see the benefits. The current design is simple and not a unique concept (e.g. Go http in standard library provides the same pattern).

I am strongly -1 on removing next.

nbcarey commented 4 years ago

@tinovyatkin said (and I quote):

I think removing next parameter from middleware and just awaiting all middleware can be most useful breaking change in 3.0. . . . while removing next will make composition simple and koa-compose will not be needed:

// koa-compose will not be needed, just do this:
  async handleRequest(ctx) {
    const res = ctx.res;
    res.statusCode = 404;
   try {
     for (const handler of this.middleware) {
        await handler(ctx);
     }
    respond(ctx);
   } catch (err) {
     ctx.onerror(err);
  } finally {
    // any finalisation logic here
  }
}

The problem is that by removing next(), you eliminate the highly useful nesting aspects of Koa's middleware:

Without this, common tasks, like, say, measuring elapsed time and logging a begin and end event would require (1) two separate pieces of middleware, (2) adding data to the request context, and (3) possibly never logging the end event because an exception was thrown.

In Koa, each piece of middleware decorates and wraps the next rather than simple concatenation like you see with most middleware implementations. It would be a shame to lose that.

pke commented 4 years ago

I agree with @nbcarey after my recent journey to polka. I never really understood how the "middleware" there is really only an "upstream-ware". It does not sit in the middle of the execution process. At least not in a useful way (for me) like redux and koa use it.

In koa I can easily have one place were I finally format the response according to the request Accept header or add a Location header based on certain context information.

This middleware concept of up- and downstream handling I find too useful to be broken.

I have to say though the performance difference between polka and koa is visible. Anyone knows where the most time is spent in koa? Maybe this could also be tackled in a next major release.

sonicoder86 commented 4 years ago

Is there a plan to release v3?

hufei365 commented 4 years ago

How about publish koa and official middleware module under @koajs/ ?

need more resource/people to maintain

Ajaxy commented 4 years ago

Why is this repo so calm? What do people use now instead of it? Is Express still the "default option" with all its callback hell?

pke commented 4 years ago

I'd guess they are using more and more serverless or old express servers that work.

iyobo commented 4 years ago

The pros of serverless hardly ever outweigh the cons. There is still that annoying warmup and warmth maintenance. My thing is, If you have to do that, just have a server running... unless you are working around very particular/un-common edge cases. Actual servers are easy enough to create and run in node land.

But I digress...

The thing about koa is that it is so modular that the lack of repo activity is actually one of its features, not a sign of its decay. There really isn't anything major that can be added to Koa that cannot be created as a separate plugin so as not to overload the default middleware pipeline with what is not needed per implementation.

What I would like to see though, is something that a plugin can't do, like the codebase converted to typescript.

pke commented 4 years ago

I agree with your thoughts about serverless, although I use it sometimes to quickly try out some ideas.

What I would like to see here for koa is more speed improvement but a TS seems also a nice goal.

kubejm commented 4 years ago

I've been chipping away at converting this project to TypeScript to get more hands on experience with TypeScript and to learn more about the inner workings of this project. I have it nearly converted and ported all the tests to Jest as well.

Along the way I made some slight changes to the implementation to accommodate typing and simplify some logic. I've annotated some of my questions/thoughts along the way. However, I have a little bit of work and clean up left.

https://github.com/kubejm/koa-ts

imaksp commented 4 years ago

@Ajaxy

Why is this repo so calm? What do people use now instead of it? Is Express still the "default option" with all its callback hell?

There are other options for express like https://www.fastify.io/ OR https://hapi.dev/

crazybber commented 4 years ago

I suggested this #1475 we need typescript