RFC: Problems with the current router

[cyco130]

⚠️ Only listing the problems for now. Actual proposals will follow later.

Hattip's router, like Express, is a flat array of routes. We just run them one by one until one of them returns a response. There's no difference between middlewares and endpoints. So, a route like app.post("/api/users", createUser) ends up being more or less equivalent to the following:

app.use((ctx) => {
  if (ctx.url.pathname !== "/api/users" || ctx.method !== "POST") {
    return;
  }

  return createUser(ctx);
});

And a middleware that protects routes that require authentication looks like this:

function authGuard(ctx) {
  const isAuthenticated = await checkIfAuthenticated(ctx);
  if (!isAuthenticated) {
    return new Response("Forbidden", { status: 403 });
  }
}

And a middleware that extends the context object with new APIs might look like this:

function cookies(ctx) {
  ctx.incomingCookies = parseIncomingCookies(ctx);
}

This design inherits several problems from Express:

Selective application problem

A flat array doesn't represent very well a typical web app's routes which tend to form a tree structure. This causes a correctness problem in some cases and a performance problem in most.

In terms of correctness, for example, you want an authentication guard middleware to apply only to some routes because you usually have public routes as well as private ones. In terms of performance, you only want to parse the cookies for routes that actually need them. The latter can be mitigated by using a lazy approach that only parses the cookies when they're actually accessed, but this approach is only convenient when the job to be done is synchronous. Asynchronous APIs end up being very awkward to use.

In real life Express apps, I've seen people using one of these three strategies:

• Manually order routes: Register public routes first, then the auth guard middleware, and finally the protected routes:

  app.get("/public-1", publicHandler1);
  app.get("/public-2", publicHandler2);
  app.use(authGuard);
  app.get("/protected-1", protectedHandler1);
  app.get("/protected-2", protectedHandler2);

  This is tedious and error-prone and it doesn't help when you have a more complex tree structure that can't be represented as a simple linear chain.

• Use path prefixes or other conditions: Register the auth guard middleware with a path prefix that applies to all protected routes:

  app.use("/admin/*", authGuard);
  app.get("/public-1", publicHandler1);
  app.get("/public-2", publicHandler2);
  app.get("/admin/protected-1", protectedHandler1);
  app.get("/admin/protected-2", protectedHandler2);

  CSRF protection middleware that only applies to POST requests also belongs to this category.

  This strategy avoids tedious manual ordering but it forces you to structure your URLs in a specific way which is not always possible or convenient.

• Use route-specific middleware: Register the auth guard middleware directly on the protected routes (not directly supported by Hattip's router yet but easy to implement):

  app.get("/public-1", publicHandler1);
  app.get("/public-2", publicHandler2);
  app.get("/protected-1", authGuard, protectedHandler1);
  app.get("/protected-2", authGuard, protectedHandler2);

  This is the best solution in terms of expressiveness but requires a lot of manual repetition, especially if your route tree is more complex.

Typing problem

For middlewares that extend the context object with new APIs, we have to use module augmentation to properly type context extensions. But then, the types apply globally, whereas they should only apply to routes that come after the extension middleware:

declare module "hattip" {
  interface RequestContext {
    incomingCookies: Record<string, string>;
  }
}

app.get("/foo", (ctx) => {
  // Here it's undefined but TS won't warn
  console.log(ctx.incomingCookies);
});

app.use((ctx) => {
  ctx.inclomingCookies = parseCookies(ctx);
});

app.get("/foo", (ctx) => {
  // Only OK here
  console.log(ctx.incomingCookies);
});

Another problem with module augmentation is that libraries can't provide a nice way of using the same middleware with different parameters under different names: E.g. two sessions: ctx.session for non-sensitive, signed cookie based session data, and ctx.authSession for sensitive, database-backed authentication session data.

Deoptimization problem

In most JavaScript engines, adding a new property to an object after construction prevents the engine from applying many optimizations (hidden classes, inline caches etc.). It means that context extending middlewares greatly harm the performance:

app.use((ctx) => {
  // Bad! The engine has to deoptimize access to ctx
  // because we're creating a new propery after construction
  ctx.inclomingCookies = parseCookies(ctx);
});

Unfortunately, this rarely shows up in naïve benchmarks which excercise the same route again and again (almost all of them 😞). Such benchmarks are mostly useless since the engine can predict the access patterns of a single code path and apply specific optimizations. It would not be possible under realistic loads where the engine would have to deal with different code paths for each incoming request.

Async problem

Already discussed here. Basically, await ctx.next() forces the router/composer to wait for the next microtask even if the next handler in the chain could return synchronously, which is a very common case. This effect compounds with each await ctx.next() in the chain, harming the performance.