hono icon indicating copy to clipboard operation
hono copied to clipboard

Extending Context

Open yusukebe opened this issue 1 year ago β€’ 32 comments

There is a request to extend Context. Discussed at #410 and https://github.com/honojs/honojs.dev/pull/1 The possibility of adding c.afunction() by the middleware is a good feature, I think.

But I don't have any idea to implement it. One idea is using declare module:

declare module 'hono' {
   interface Context {
     afunction()
   }
 }

It seems to be not smart. Is there someone have other ideas?

yusukebe avatar Jul 20 '22 00:07 yusukebe

It must be something good thinking. it implies in all design philosophy of project

mgrithm avatar Jul 23 '22 19:07 mgrithm

Hi, @yusukebe πŸ‘‹ I saw this #410 and I came in here.

I want to set any values to the context in the middleware. and use it on main handler.

In my middleware (firebase-auth) is example here.

  async (c: Context, next: Next) => {
    const authorization = c.req.headers.get(config.AuthorizationHeaderKey);
    if (authorization === null) {
      return new Response(null, {
        status: 400,
      });
    }
    const jwt = authorization.replace(/Bearer\s+/i, "");
    const auth = Auth.getOrInitialize(
      c.env.PROJECT_ID,
      config.KeyStore ??
        WorkersKVStoreSingle.getOrInitialize(
          c.env.PUBLIC_JWK_CACHE_KEY,
          c.env.PUBLIC_JWK_CACHE_KV
        )
    );

    try {
       // I want to set "firebaseToken" value to the context. (but I can't do it now?)
      const firebaseToken = await auth.verifyIdToken(jwt, c.env);
      await next();
    } catch (err) {
      console.error({
        message: "failed to verify the requested firebase token",
        err,
      });
      return new Response(null, {
        status: 401,
      });
    }
  }

Go has this way.

Code-Hex avatar Jul 27 '22 12:07 Code-Hex

Hi @Code-Hex !

In fact, there is such the feature that is not documented: https://github.com/honojs/hono/pull/226

const v = c.get('key')
c.set('key', 'value')

How about using these methods? If they are used often, I would like to document them.

However

The value will be any. I want to add the types. Can we do that by using Generics?

And, also we want to implement adding "function" to Context, such as c.afunction().

Do you have a good idea? cc: @usualoma @metrue

yusukebe avatar Jul 27 '22 14:07 yusukebe

@yusukebe Nice API!! It's enough for me.

Personally, I can't agree with extending context after adding some functions because if we use some middleware and their middleware extends the context, it has the potential to cause unexpected behavior by overriding same name methods.

I think it would be easier for users to understand if the context was as simple as possible to implement.

I wonder though maybe get method should return any or unknown and the user should use typeguard if want a specific type.

Code-Hex avatar Jul 27 '22 14:07 Code-Hex

@Code-Hex Thank you for telling me your thought.

Exactly, extending the context will make that user confused. It should be easy to understand and should be a simple implementation.

Hmm. We may not have to implement "extending context"...

We can make the most of "get/set" method instead.

yusukebe avatar Jul 27 '22 14:07 yusukebe

And, also we want to implement adding "function" to Context, such as c.afunction().

I would like this ability, but mostly because I would like to add something like a ctx.auth() or ctx.trace().

That said, I'm fine creating a helper like getAuth(ctx) or getTrace(ctx) that uses get/set under the covers.

RichiCoder1 avatar Jul 27 '22 15:07 RichiCoder1

Hi @yusukebe I think the ctx should keep as simple as possible, get and set almost satisfy most of the requirements, and when we are using set/get we totally have to be aware of what we are doing.

And, also we want to implement adding "function" to Context, such as c.afunction().

Could you provide some use cases for this, in my thinking, if we are put something heavy into ctx, something would be wrong, it could be implemented in another way (maybe a individual middleware?)

@RichiCoder1

I would like this ability, but mostly because I would like to add something like a ctx.auth() or ctx.trace().

Could you share more about the ctx.auth() and ctx.trace() ? what are they for and how are you going to design it? are you going to make some special keys ___auth___ to read and write the authorization information inside ctx.auth() and getAuth(ctx) ? if so, I think it's something the user should do in their codes, instead of hono.

metrue avatar Jul 28 '22 01:07 metrue

Could you share more about the ctx.auth() and ctx.trace() ? what are they for and how are you going to design it? are you going to make some special keys auth to read and write the authorization information inside ctx.auth() and getAuth(ctx) ? if so, I think it's something the user should do in their codes, instead of hono.

There would a jwt middleware for example that would expose the jwt's claims via ctx.auth().sub === "my-user" for example. Or for tracing, an Open T racing Middleware which would expose the current active trace information via something like a ctx.trace().spanId and expose a helper for tracing downstream requests via ctx.trace.fetch(....).

RichiCoder1 avatar Jul 28 '22 05:07 RichiCoder1

Hi @metrue !

Could you provide some use cases for this

For example, I'm planning to make the "Validator Middleware" https://github.com/honojs/hono/issues/425. In this middleware, the user must get the results in a handler. So, I thought that ctx.getResult() is a helpful method to do it:

app.use('/', validator(...))

app.get('/', (c) => {
  const result = c.getResult()
  return c.json(result)
})

BUT, I notice we don't have to extend the Context. We can write it without validatorResult() method:

app.use('/', validator(...))

app.get('/', (c) => {
  const res = validatorResult(c)
  return c.json(res)
})

Let's see implementation. In the "validator" method, set the result using ctx.set() method:

ctx.set('validatorResult', result)

And, the validatorResult() method will get the result using ctx.get() method:

const validatorResult = (ctx: Context): Result => {
  return ctx.get('validatorResult')
}

I think this way is better than extending Context. For instance, @Code-Hex 's "Firebase Auth Middleware" is using this method:

https://github.com/honojs/firebase-auth/blob/main/src/index.ts#L77-L84

yusukebe avatar Jul 28 '22 15:07 yusukebe

Hi @yusukebe

I also think it is not a good practice for middleware to add methods to Context object. I agree with the use of helper functions, as in "Firebase Auth Middleware."

However, I think that in some cases it is not a bad idea for middleware to extend type definitions.

For example, making changes like the following PR... https://github.com/honojs/hono/pull/429

Middleware will be able to add types as follows...

declare module 'hono' {
  interface ContextVariableMap {
    calculationResults: number
  }
}

This allows users to benefit from type definitions.

image

It can also detect before execution if the same key is already in use by other middleware.

image

I think helper functions are the best practice for this issue, so #429 will be an adjunct, but I think it will help solve some of the problems.

usualoma avatar Jul 28 '22 22:07 usualoma

@usualoma Ultra Great!

I've tried it for "Validator Middleware" which is WIP, it worked, and I've felt fine DX.

γ‚Ήγ‚―γƒͺγƒΌγƒ³γ‚·γƒ§γƒƒγƒˆ 2022-07-29 8 34 38

It seems that writing declare module... is trouble, but this is just for the developer of the middleware. @Code-Hex will be able to use this feature for "Firebase Auth Middleware" soon.

Let's get this feature in!

And, we need to consider... The middleware can provide two syntaxes for getting "results", "variables pattern" and "function pattern".

const result = c.get('validatorResults') // function pattern
const result = validatorResult(c) // method pattern

We/Middleware developers can provide both patterns, but should we choose one? or both?

yusukebe avatar Jul 28 '22 23:07 yusukebe

We/Middleware developers can provide both patterns, but should we choose one? or both?

I think that is what bothers me too.

While being able to declare type to values of set/get is an interesting feature, I believe the following is often the good pattern for users (who are not middleware developers).

const result = validatorResult(c)

usualoma avatar Jul 29 '22 01:07 usualoma

Hi @usualoma !

I believe the following is often the good pattern for users

Agree. I think it would be easier for users to understand if there is an explicit method. So, the author of the middleware should provide a method like "validatoResult" if wants to get some values from Context.

Though we have this guideline, feature #429 is good for us. I'll merge it.

yusukebe avatar Jul 30 '22 08:07 yusukebe

@yusukebe Thinking about this stuff I had a thought. What if there was a way to provide the initializer for a context key:

app.var("validatorResults", (c) => {
  return ...
})

With some internal handling of get calls:

class Context {
  ...
  var(key: string) {
    let value = this.get(key)
    if (!value && this._initializers[key]) {
      this.set(key, value = this._initializers[key](this))
    }
    return value
  }
}

matthewrobb avatar Sep 09 '22 18:09 matthewrobb

Not yet big experience with Hono, but from my superficial usage I have 2 thoughts: Extending Context is not too crazy Mainly because, I am using cloudflare and I manually overwrite the environemnt anyway, to access those globals / services. Doing something like this is maybe not perfect, but seems fine. Considering that for "some" global services (e.g. KV) I am doing this anyway.

Alternative: Infer additional types from middleware This is just a general thought, not sure about the details, but think would be possible. That "using" the middleware extends the request. This would look something like this (very pseudo code)


type ContextExtend = {
  foo: string
}
const myMiddleware: Middleware<ContextExtend> = ...
app.get('/my-endpoint-with-middleware', myMiddleware, c => {
  // context was extend by middleware, via generic inference on the myMiddleware argument
  c.context.foo
})

And in the case when the middleware is defined on the app level, we could try to use a fluent interface or inject middlewares during construction, to give the extend context in the resulting type


const app = createHono(...middlewares)
// alternative
const app = createHono()
  .use(middleware)
  .use(middelware2)

I hope I could communicate the base ideas. This is just high level idea and they probably require some typescript trickery, but theoretically possible.

KeKs0r avatar Nov 11 '22 17:11 KeKs0r

I just spend a short while on doing a demo of how something like this could be achieved. I do not think it you should be scared of letting the middleware changing the Context type, since adding something to the context is a common thing to do in the middleware.

type Context<Type> = {
  [Property in keyof Type]: Type[Property]
}

type App<T> = {
  use: <U>(a: (ctx: Context<T>) => Context<T> & Context<U>) => App<U> & App<T>
  get: (path: string, callback: (ctx: Context<T>) => void) => void
}

const app: App<unknown> = {} as any

const x = (ctx: Context<any>) => {
  ctx.hello = 'world'
  const newContext: typeof ctx & Context<{ hello: 'world' }> = ctx
  return newContext
}

app.use(x).get('', (ctx) => {
  ctx.hello // Exists
  return ctx
})

app.get('', (ctx) => {
  ctx.hello // Doesn't exist
  return ctx
})

As you can see from the example, when using get on a app that has uses the middleware, the variable hello is now exposed. This demo is solely to show what's possible and a brain storm idea of how to do it.

eikooc avatar Nov 28 '22 15:11 eikooc

Hi, I'm interested in this too. I'm using Prisma with Hono for Cloudflare Workers target. It would be cool to have a property c.prisma that is PrismaClient and is also typed properly for good DevX. Or, is there already a method to do that, without having to do c.set() and c.get()?

eliezedeck avatar May 14 '23 12:05 eliezedeck

@eliezedeck

While it's true that writing c.prisma is shorter, I avoid extending Context directly. Hence, I recommend using c.set() and c.get() or ContextVariableMap:

declare module 'hono' {
  interface ContextVariableMap {
    prisma: PrismaClient
  }
}

const prismaMiddleware: MiddlewareHandler = async (c, next) => {
  // ...
  c.set('prisma', prismaClinet)
  await next()
}

app.get('/list', prismaMiddleware, (c) => {
  const prisma = c.get('prisma')
  // ....
})

yusukebe avatar May 14 '23 22:05 yusukebe

Thanks @yusukebe.

Is there a specific reason why extending the Context is not recommended?

I'm using Cloudflare Workers and facing issues with the Prisma client library. Instantiating the client outside of a request is not possible due to limitations with Cloudflare Workers or the library itself. The values required for client instantiation are only accessible through c.env.PRISMA_URL. Considering this, I thought about extending the Context to avoid the verbosity of additional functions like getPrismaClient(c: Context).

eliezedeck avatar May 16 '23 08:05 eliezedeck

@eliezedeck

You can do that with this code:

type Environment = {
  Bindings: {
    PRISMA_URL: string
  }
}

const app = new Hono<Environment>()

const prismaMiddleware: MiddlewareHandler<Environment> = async (c, next) => {
  const prismaUrl = c.env.PRISMA_URL

  // pseudo code
  const client = new PrismaClient({
    url: prismaUrl,
  })

  c.set('prisma', client)
  await next()
}

Is there a specific reason why extending the Context is not recommended?

As discussed above. We don't use what we don't need and we don't want to complicate Types. At least we won't be getting into this any time soon.

yusukebe avatar May 16 '23 13:05 yusukebe

Thank you @yusukebe

eliezedeck avatar May 17 '23 02:05 eliezedeck

I have been trying to write a similar dependency injection middleware, and while it does work... I still get type errors/warnings in my IDE (latest WebStorm 2023.2.1).

I have tried various methods, including the one described on the docs but still get type errors on c.get() access.

type Variables = {
  container: Record<string, any>
}

const app = new Hono<{Variables: Variables}>()

const containerMiddleware = async (c, next) => {
  const container: Record<string, any> = {
    // ... register various services here
  }
  c.set('container', container)
  await next()
}

app.get('/test', containerMiddleware, (c) => {
  const container = c.get('container')
                        // ^-- "Argument types do not match parameters"
  
  return c.text(`${container.message}: ${container.name}`)
})

Considering the code works fine, I am okay ignoring it but it seems like I am not getting any type-safety since it still thinks container is any unless I explicitly state the type... which kind of defeats the purpose.

My expectation would to be have something like an optional state type that you would initialize via like this:

// Initialize some dependency injection container
const container: Record<string, any> = {}

type AppState = {
  container: Record<string, any>
  someOptional?: any
  // other properties...
}

// Create the app using a constant value
const app = new Hono<AppState>({ container })

// Or a factory in the case of dynamic initialization
const app = new Hono<AppState>(() => {
   // do some initialization...
   return { container }
})

Then this could be accessed via c.state as the type provided:

app.use('*', async (c, next) => {
  const { container } = c.state
  container.message = 'hello from the container'

  await next()
})

app.get('/', (c) => {
  const { message } = c.state.container
  return c.text(`The container says: ${message}`)
})

ewrogers avatar Sep 11 '23 03:09 ewrogers

@eliezedeck

While it's true that writing c.prisma is shorter, I avoid extending Context directly. Hence, I recommend using c.set() and c.get() or ContextVariableMap:

declare module 'hono' {
  interface ContextVariableMap {
    prisma: PrismaClient
  }
}

const prismaMiddleware: MiddlewareHandler = async (c, next) => {
  // ...
  c.set('prisma', prismaClinet)
  await next()
}

app.get('/list', prismaMiddleware, (c) => {
  const prisma = c.get('prisma')
  // ....
})

Is there a way to only declare the attribute on ContextVariableMap if the given middleware is used on the path? So only allow c.get("foo") when FooMiddleware is actually used in the chain before the request.

vassbence avatar Nov 07 '23 17:11 vassbence

Hi @vassbence ,

You can do it with this code:

import { createMiddleware } from 'hono/factory'

const echoMiddleware = createMiddleware<{
  Variables: {
    echo: (str: string) => string
  }
}>(async (c, next) => {
  c.set('echo', (str) => str)
  await next()
})

app.get('/echo', echoMiddleware, (c) => {
  return c.text(c.var.echo('Hello!'))
})

yusukebe avatar Nov 08 '23 01:11 yusukebe

Hi @vassbence ,

You can do it with this code:

import { createMiddleware } from 'hono/factory'

const echoMiddleware = createMiddleware<{
  Variables: {
    echo: (str: string) => string
  }
}>(async (c, next) => {
  c.set('echo', (str) => str)
  await next()
})

app.get('/echo', echoMiddleware, (c) => {
  return c.text(c.var.echo('Hello!'))
})

Lovely, thanks!

vassbence avatar Nov 08 '23 10:11 vassbence

The variable types do not seem to get propagated if the middleware is wired up via .use call.

const someMiddleware = createMiddleware<{ Variables: { foo: string } }>(...);

const app = new Hono()
  .use(someMiddleware())
  .get('/', c => {
    c.var.foo // <- fails here
  })
  .get('/other', someMiddleware(), c => {
    c.var.foo // <- works here
  })

I feel like .use should compose the Environment type to the existing environment.

vsviridov avatar Nov 08 '23 23:11 vsviridov

@vsviridov

That is a thing which is not implemented or a just bug. But we may be enable to fix it.

yusukebe avatar Nov 09 '23 00:11 yusukebe

Is there a reason not to do it similar to Express with .locals?

https://expressjs.com/en/5x/api.html#res.locals

Could you make the locals property of type object and allow any fields to be added and still satisfy its type?

I feel like this is easier to reason about and easier to debug. If I want to know what's been added by mine or any other middleware I just log c.locals.

jazoom avatar Feb 04 '24 02:02 jazoom

Is there a reason not to do it similar to Express with .locals?

Express is not a typescript native. Those types were added after.

Could you make the locals property of type object and allow any fields to be added and still satisfy its type?

I feel like this is easier to reason about and easier to debug. If I want to know what's been added by mine or any other middleware I just log c.locals.

In typescript we like to be precise.

vsviridov avatar Feb 04 '24 17:02 vsviridov

@vsviridov I'm not a Typescript expert, so please forgive my ignorance, but I don't see how the black box of set and get is better?

jazoom avatar Feb 04 '24 21:02 jazoom