Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Discourage usage of ContextVariableMap and provide helper ExtractEnv #2249

Open
KilianB opened this issue Feb 21, 2024 · 4 comments
Open

Discourage usage of ContextVariableMap and provide helper ExtractEnv #2249

KilianB opened this issue Feb 21, 2024 · 4 comments
Labels
enhancement New feature or request.

Comments

@KilianB
Copy link

KilianB commented Feb 21, 2024

What is the feature you are proposing?

We currently build a large application with multiple nested routes and the need for different middleware

const app = new Hono().use(ipAddress).use(logger);

app.route("/private",privateRoutes);
app.route("/frontend", frontendRoutes);
app.route("/management", managmentRoutes);

// managementRoutes.tsx
export const managmentRoutes = new Hono().use(jwt);
... do something

// ipAddress.tsx
export const ipAddress: MiddlewareHandler<{
  Variables: {
    remoteIp: string;
  };
}> = async (c, next) => {
 ....
}

Where we now have 2 issues at the same time.

  1. The type remoteIp which is a global middleware is not available in any of the mounted mounted Hono instance
  2. The type jwtPayload is available in all hono instances even though it is not set as the middleware is only bound to managementRoutes

I believe the ContextVariableMap is really convinient, but it also provides incorrect typings and now we can not trust the IDE anymore and have to either just assume we are right or use some additional checks everywhere.

A better solution would be to provide a utility type to extract the ENV generic and let the user manually pass it down to the mounted Hono instances.

const app = new Hono().use(ipAddress).use(logger);

type ExtractEnv<T> = T extends Hono<infer Env, any,any> ? Env : never;
export type BaseAppEnv = ExtractEnv<typeof app>;

// managementRoutes.tsx
export const managmentRoutes = new Hono<BaseAppEnv>().use(jwt);

If usability is favored over correctness we should at least have a section in the docs hinting that such a type helper could be used.

@KilianB KilianB added the enhancement New feature or request. label Feb 21, 2024
@yusukebe
Copy link
Member

Hi @KilianB

Sorry for the late response! I totally agree with you.

We will try to avoid using ContextVariableMap as much as possible and provide ExtractEnv. For example, in the case of JWT Middleware, we should stop defining types in the ContextVariableMap and define it like the following.

MiddlewareHandler<{
  Variables: {
    jwtPayload: any
  }
}>

We have to think about whether the change would be a breaking change, but we will go ahead.

@Angelelz
Copy link

Hey @KilianB, I'm encountering this issues with the jwt middleware. Have you used any workarounds or do you have any suggestion on how to avoid the jwtPayload type issue?

@marceloverdijk
Copy link
Contributor

Inspired on this X post I'm doing it like:

export type Env = {
  Variables: {
    prisma: PrismaClient;
  },
  Bindings: {
    DB: D1Database;
  },
};

export const createApp = () => {
  const app = new OpenAPIHono<Env>();
  app.use(async (c, next) => {
    const adapter = new PrismaD1(c.env.DB);
    const prisma = new PrismaClient({ adapter });
    c.set('prisma', prisma);
    await next();
  });
  return app;
}

const app = createApp();

const pets = new OpenAPIHono<Env>();
pets.openapi(
  createRoute({
  ..

app.route('/api/pets', pets);

@marceloverdijk
Copy link
Contributor

I was trying to go a bit further, to make some var available on the root and its child apps, but also some vars only on a child app only. And this in a type-safe manner.

export type Env = {
  Variables: {
    prisma: PrismaClient;
  },
  Bindings: {
    DB: D1Database;
  },
};

export const createApp = () => {
  const app = new OpenAPIHono<Env>({
    defaultHook: (result, c) => {
      ..
  });
  return app;
};

export const createRootApp = () => {
  const app = createApp();
  app.use(logger());
  app.use(prettyJSON());
  app.use(async (c, next) => {
    console.log('setting prisma');
    const adapter = new PrismaD1(c.env.DB);
    const prisma = new PrismaClient({ adapter });
    c.set('prisma', prisma);
    await next();
  });
  return app;
};

creating the root app like

const app = createRootApp();
app.route('/pets', petsApp);

The root app and children will contain the prisma var.

Creating a child app:


const petsApp = createApp();

const petsMiddlewareHandler: MiddlewareHandler<{
  Variables: {
    petResourceModelAssembler: PetResourceModelAssembler;
  };
}> = async (c, next) => {
  console.log('setting pet resource model assembler');
  c.set('petResourceModelAssembler', new PetResourceModelAssembler());
  await next();
}

app.use(petsMiddlewareHandler);

app.openapi(
  createRoute({
    method: 'get',
    path: '/',
    responses: {
      200: {
        description: 'OK',
        content: {
          'application/json': {
            schema: z.array(PetSchema),
          },
        },
      },
    },
  }),
  async (c) => {
    const pets = await c.var.prisma.pet.findMany({
      orderBy: { name: 'asc' },
    });
    const petResourceModels: PetResourceModel[] = c.var.petResourceModelAssembler.toCollectionModel(pets);
    return c.json(petResourceModels);
  },
);

but the c.var.petResourceModelAssembler does exists now unfortuantely:

Property 'petResourceModelAssembler' does not exist on type 'Readonly<ContextVariableMap & { prisma: PrismaClient<PrismaClientOptions, never, DefaultArgs>; }>'.

Am I doing something wrong?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request.
Projects
None yet
Development

No branches or pull requests

4 participants