hono icon indicating copy to clipboard operation
hono copied to clipboard

context `c.set/get` not working in cloudflare pages functions

Open zoubingwu opened this issue 1 year ago • 5 comments

What version of Hono are you using?

4.5.8

What runtime/platform is your app running on?

cloudflare pages

What steps can reproduce the bug?

made a minimal repro here: https://github.com/zoubingwu/hono-ctx-set-cf-pages

git clone https://github.com/zoubingwu/hono-ctx-set-cf-pages.git
pnpm install
pnpm run dev

then open http://localhost:8888/, you will see alert with hello undefined

What is the expected behavior?

should be hello joe since it was set in middleware

What do you see instead?

hello undefined

Additional information

// functions/_middleware.ts
import { handleMiddleware } from "hono/cloudflare-pages";

export const onRequest = [
  handleMiddleware(async (c, next) => {
    console.log("middleware setting user");
    c.set('user', 'joe')
    await next();
  })
]

// functions/api/[[route]].ts
import { Hono } from "hono";
import { handle } from "hono/cloudflare-pages";

interface HonoContext {
  Variables: {
    user: string
  };
}

const app = new Hono<HonoContext>().basePath('/api');

const routes = app.get('/hello', (c) => {
  return c.json({
    message: `Hello, ${c.get('user')}!`
  });
});

export const onRequest = handle(routes);


// index.html
fetch('/api/hello').then(res => res.json()).then(data => {
    alert(data.message)
})

zoubingwu avatar Aug 24 '24 18:08 zoubingwu

Hi @zoubingwu

Thank you for creating the issue. This looks like a bug, but it's a known issue that can be resolved because a Context in Pages Middleware is not shared for the handler. This is a limitation for using Cloudfare Pagea Middleware.

If you want to do more complex things for Cloudflare Pages, you can try the Cloudflare Pages starter template:

npm create hono@latest -- --template cloudflare-pages

yusukebe avatar Aug 25 '24 10:08 yusukebe

@yusukebe cf context prodvides a data object to attach some data and will persist during the request, their docs didn't mention that clearly tho but it works

see https://developers.cloudflare.com/pages/functions/api-reference/#eventcontext and https://community.cloudflare.com/t/what-is-context-data-in-pages-functions/476559

can we use that instead? I found that this object is not accessible in hono's context.

I updated the code in the reproduce repo, you can check it out.

// _middleware.ts
export const onRequest = [
  // handleMiddleware(async (c, next) => {
  //   console.log("middleware setting user");
  //   c.set('user', 'joe');
  //   console.log(c);
  //   c.env.eventContext.data.user = 'joe2' // not working
  //   return await next();
  // })
  async (context) => {
    context.data.user = "joe"; // working
    return await context.next();
  },
];

// [[route]].ts
const routes = app.get("/hello", (c) => {
  return c.json({
    //@ts-ignore
    message: `Hello, ${c.env.eventContext.data.user}!`,
  });
});

zoubingwu avatar Aug 25 '24 16:08 zoubingwu

Hi @zoubingwu

Exactly. Cloudflare Pages provides data. Nice suggestion. I'll consider it.

Hi @BarryThePenguin If you have any thoughts, please share them.

yusukebe avatar Aug 26 '24 11:08 yusukebe

I'd expect this to work..

handleMiddleware(async (c, next) => {
  console.log("middleware setting user");
  c.set('user', 'joe');
  console.log(c);
- c.env.eventContext.data.user = 'joe2' // not working
+ c.executionCtx.data.user = 'joe2' // should work
  return await next();
})

The Cloudflare EventContext is passed to new Context() and can be accessed through the executionCtx property. However the types don't allow for this..

https://github.com/honojs/hono/blob/f9349ecca263c16b340b979f1628ed0f238aaecd/src/adapter/cloudflare-pages/handler.ts#L50-L53

https://github.com/honojs/hono/blob/f9349ecca263c16b340b979f1628ed0f238aaecd/src/context.ts#L407

BarryThePenguin avatar Aug 26 '24 21:08 BarryThePenguin

I can see a solution where using c.get() and c.set() update the underlying c.executionCtx.data property, but making the types work might be tricky..

BarryThePenguin avatar Aug 26 '24 21:08 BarryThePenguin

according to https://github.com/honojs/hono/issues/1219, if we use AsyncLocalStorage for context, will this simplifies the typings?

zoubingwu avatar Aug 29 '24 08:08 zoubingwu

This is closed by #3332!

yusukebe avatar Sep 12 '24 06:09 yusukebe