Request / response aliases are bad idea

[ivan-kleshnin]

I suspect I'll met a disagreement but I have to say it. Request / Response aliases are very confusing. And I'm not a Koa newcomer.

Why this.body is this.response.body while this.headers are this.request.headers? this.request.body is used often as well as this.response.headers. Oh, wait, the latter is this.set(...)! Did I tell you it's confusing?

I know, it's optional and I may use full path (as I do). But, the thing is, the library authors are following the "convention" and create first shortcut-ful then even shortcut-only APIs. Like this.path without a proper copy in this.request.path.

I feel like way too many things are in the single namespace right now.

Koa is mature and established. Why I bother? Because async-await is in the Node 7.0 and I expect more people will start to adopt this framework soon. Maybe Koa 2 could take an opportunity to clean this up.

Memorizing these tables is not a friendly option

Request aliases

The following accessors and alias Request equivalents:

ctx.header
ctx.headers
ctx.method
ctx.method=
ctx.url
ctx.url=
ctx.originalUrl
ctx.origin
ctx.href
ctx.path
ctx.path=
ctx.query
ctx.query=
ctx.querystring
ctx.querystring=
ctx.host
ctx.hostname
ctx.fresh
ctx.stale
ctx.socket
ctx.protocol
ctx.secure
ctx.ip
ctx.ips
ctx.subdomains
ctx.is()
ctx.accepts()
ctx.acceptsEncodings()
ctx.acceptsCharsets()
ctx.acceptsLanguages()
ctx.get()

Response aliases

The following accessors and alias Response equivalents:

ctx.body
ctx.body=
ctx.status
ctx.status=
ctx.message
ctx.message=
ctx.length=
ctx.length
ctx.type=
ctx.type
ctx.headerSent
ctx.redirect()
ctx.attachment()
ctx.set()
ctx.append()
ctx.remove()
ctx.lastModified=
ctx.etag=

I guess this.request.foobar felt too long to write and you probably didn't want to get this.req confused with native Node Request which is commonly called req. But still this is not worth the encomplication IMO.

[dominhhai]

It makes me confused too. Sometimes, have to check the API to use them.

[nathanloisel]

totally agree, I stop to use ctx.body ect... Newcomers on my koa projects are always lost with that kind of aliases.

Maybe ctx.req.x/ctx.res.x or cyx.in.x/ctx.out.x

[tj]

Many of these can only be read or write, ctx.method, ctx.path, ctx.status for example don't make sense for a response. A few are definitely ambiguous (ctx.body, ctx.headers), the rest are fine IMO, you only need to memorize 3 or 4.

Even ctx.headers doesn't even really apply since you'd never do ctx.headers['Content-Type'] = foo in Node, they have to be normalized before setting.

I don't disagree that it's potentially confusing in those cases but if you don't like the Koa sugar you can access the request/response directly as you mentioned, just a compromise.

[chanlito]

Been working for awhile with Koa now, but It doesn't seem to confused me at all.

[jonathanong]

i personally do const { request, response } = this all the time and always try to use those. i try to do the same in open source projects so its easier to understand.

PRs so that the following would be easier to do would be welcomed:

app.use(async ({ request, response }) => {
  await Promise.resolve('something');

  response.body = 'hi';
})

[ShimShamSam]

@jonathanong Isn't this solved in Koa 2.0 since the context is now passed in as the first parameter instead of overriding this?

[fl0w]

@ShimShamSam The example provided is of Koa@2 signature. That's why he can deconstruct the argument (ctx).

[JeffRMoore]

As a newcomer, I find the shortcut confusing. I don't think it adds value.

Should I use ctx.type or should I use ctx.response.type?

Since there can be ambiguity on some properties, I decided to prefer the long form.

On a larger team, I would now have to educate the team on that decision and maintain it through code reviews and through team turnover. If I decide not to enforce consistency on the choice, I still have to educate the team that inconsistency is ok.

It is better to not have a choice.

es6 de-structuring offers a much better shortcut. There is less cognitive load for those coming to Koa from other frameworks who are used to separate request and response parameters.

In submitting documentation PRs, I would have liked to written this example:

app.use(async ({request, response}, next) => {
  if (!request.accepts('xml')) ctx.throw(406);
  await next();
  response.type = 'xml';
  response.body = fs.createReadStream('really_large.xml');
});

This does not work because now I've lost throw in de-structuring.

What drew me to Koa was its minimalistic sensibility. Context aliases seem directly opposed to that.

I'd recommend:

• go "full delegation" on context in v2.x to support de-structuring
• update all documentation to use de-structuring instead of aliases (I volunteer to update docs)
• indicate the aliases are deprecated in v2.x with scheduled removal in 3.x.
• Move the alias setup to a separate module, required in 2.x, call it yourself in 3.x

[tj]

👍 I think that's fair, though if we're destructuring we might as well just pass (req, res) in the signature, I'm not a fan of ctx.request.stuff or destructuring always personally.

Ideally node core was more usable out of the box in terms of what Koa introduces so we wouldn't have to touch any of that. It's pretty ugly having to replace them since it doesn't help node form a thriving middleware ecosystem around primitives that everyone agrees on.

Especially now with async/await allowing for more generic middleware that don't use callbacks. Go's net/http is very Koa-like but I think it's a great model of this if anyone wants inspiration.

[fengmk2]

I do think ctx is a great abstraction on koa, especially in enterprise development. It's very helpful to make tracer impl in a easy way, most like ThreadLocal in Java.

It can store many information about the current request context and use it in the following process flows. (e.g.: request a down stream PRC with the tracer data)

More detail can find on "Context on egg"

[tj]

I can definitely agree with the ambiguity aspect. I also think getters/setters (mostly setters) are kind of bad practice, like doing response.type = "text" still warrants double-checking documentation since you may be wrong about its existence, vs a method call.

Koa definitely straddles that line between work-horse and magic, perhaps a bit too much magic.

[fl0w]

@tj considering what you wrote, would a getter/setter function on context which throws/validates on use be a good idea?

app.use(async (ctx, next) => {
  // pseudo fn-name
  ctx.safeGet('typ') // throws
  ctx.safeGet('type') // get from KoaRequest by default
  ctx.safeSet('type', 'application/json') // set on KoaResponse by default
  ctx.safeGet('response', 'typ') // -> throws
  ctx.safeSet('request', 'type', 'hack')
})

[tj]

Or just req.type and res.type() etc, getters are maybe reasonable if they always have a value, but if they're sometimes null then that's ambiguous as well, they probably shouldn't even be in JS to be honest haha.

[JeffRMoore]

Radical.

At first I thought Context didn't have much purpose once it moved from this to a parameter and that (req, res) would represent "least surprise." However, Request and Response don't extend http anyway, so there is always going to be some surprise.

Would (req, res) be worth the turmoil of another signature change? Is that really on the table?

I can imagine down the line, someone opening an issue "Why so much de-structuring?"

I have a hard time imaging someone opening an issue "merge request and response" after a split.

Recap of options

1. Prefer de-structuring in the docs, use aliases (included or separate) or long form if you don't like de-structuring.
2. Prefer long form in the docs, use aliases (included or separate) or de-structuring if you don't like long form.
3. Change signature to (req, res, next).
4. Prefer aliases in the docs, de-structure or use long form if you don't like them.

The setters take some getting used to, but I don't mind them.

I haven't tried this, but could one do something like this to catch mis-assignment to properties?

if (process.env.NODE_ENV !== 'production') {
  app.use(async (ctx, next) => {
    ctx.freeze();
    ctx.request.freeze();
    ctx.response.freeze();
    await next();
  });
}

[tj]

Yea I think splitting is nicer than basically forcing the destructuring. Originally I (obviously) wanted this to be the focus so context made a bit more sense there, I still prefer this over ctx since I would never have closures in a route, always await'd calls. having callbacks in the routes is definitely an anti-pattern but hey.

Clarity-wise ctx.body(val) and friends makes the most sense, not as "cute" but whatever. One potential solution is to have a proxy which yells at you if you use the wrong setter/getter haha.... but yeaaaahhh that's just getting weird, or maybe freezing like you mention, I've never actually used freezing, all these features seem weird in JS.

[magicdawn]

safeGet/safeSet to mustGet/mustSet like MustXxx in golang :joy:

[ivan-kleshnin]

I haven't tried this, but could one do something like this to catch mis-assignment to properties?

@JeffRMoore once you start to enforce things like that – you hit the opposite case (when you want to unfreeze stuff) 😄 Why? For example, you're in the middle of a huge code refactoring and you need to do some weird things with variable / header names. Including request mangling...

I remember working in Python:Flask where Request was freezed by default and it was a PITA for corner cases like that.

I've never actually used freezing, all these features seem weird in JS.

👍

[JeffRMoore]

Hehe, freezing is weird, but generators and accessors are not? :)

I see the point of .mustGet(field), but .field or .getField() is more friendly to static tools like flowtype, TypeScript, autocomplete, etc.

[tj]

Accessors are weird, generators are fine, I don't think they add to the list of error-prone JS features personally.

[zh99998]

if we have any chance to change that (e.g. koa v3), strongly hope to change signature be (request, response, next) .

[nervgh]

Request / response aliases are bad idea

I'm agree. Because we need keep in mind the mapping. Furthermore, for example, if we parse request params

ctx.params = ctx.request.params = {...}

we should care about two references.

https://en.wikipedia.org/wiki/Single_source_of_truth

For me, ctx by itself is good because I can pass it to another function/middleware very simple.

[tj]

I still think if Node's core had this useful stuff it would be the best for everyone, then all middleware could easily share the (req, res) signature. Granted you can already do that, you just lose a lot of the convenience.

[JeffRMoore]

There is another signature possibility along the lines of eliminating context that offers some additional error detection possibilities.

The request and context objects could be combined with the delegation removed. response is left alone except for getting a duplicate throw and assert and cookies. Then This PR could be used to enable response to be the value of the promise returned by next.

So you'd end up with either

app.use(async (request, next) => {
  const response = await next()
  response.body = 'Hello World'
  return response
});

or

app.use(async (request, next) => {
  const response = await next()
  response.body = 'Hello World'
});

Requiring the return is more verbose, but allows the framework to detect this case where await is left off:

app.use(async (request, next) => { next() })

It doesn't have the familiarity of (req, res) but maybe makes sense given that you have to pay attention to keeping the Promise chain intact anyway.

[DesignByOnyx]

I can almost guarantee that if you move to (req, res, next) you will get bogged down with "why doesn't this [meant-for-express] middleware work". Having just started with koa, I like the(ctx, next) signature. At the very least, the signature says "hey, this is koa middleware" and disambiguates from connect-style middleware. The ctx is also a great place to expand information that might be shared between middleware (eg. ctx.user). After only several days of testing, the destructuring does not bother me as I don't use request and response "everywhere" as implied earlier in this thread. I have even started doing things like this:

app.use(async (ctx, next) => ctx.user = { name: "Ryan" });
app.use(async ({user, response}, next) => ...)

Going back to the point of this thread, I definitely think the shortcut aliases are confusing. I'd even pull it into a koa-shortcuts module or put it behind a flag so people can turn it on if they want. This way the "shortcut" stuff can stay separate and documented all in one place - allowing users to be in control of their own confusion.

[ianstormtaylor]

I agree that ctx is actually a nice pattern to have. In the Express world since there's no obvious place to put things people augment the req/res objects in non-standard ways, and then you have to dig through all of the existing properties on them to figure out where things are stored.

ctx.request.params isn't bad to type at all, it's explicit. And if it feels too long, you can always destructure. (I honestly destructure in most of my middleware already like @jonathanong.)

So yeah, +1 to this idea. It would be awesome if the shortcuts were deprecated (maybe with logging in NODE_ENV == 'development') and removed in the next major version.

[ghost]

I found this confusing too. The other one that confuses me is ctx.req v.s. ctx.request (same for response). And the shorter one, which is easier to write, is not the one that is used more often.

[jon-cooke]

I have never used any of the context shortcut methods as they make the code harder to understand, I'd advise others to avoid also.

[ianstormtaylor]

FWIW, I've changed my thinking now, and I think ctx is no longer necessary either. Instead you can always just use a getter like:

const body = await getBody(req)

…and no longer need a place to store things, since they can be retrieved on demand. And anything that is expensive or a singleton can be cached in a WeakMap with the req itself as the cache key.

micro does this and it works really well to make things explicit.