feat: support getLoadContext

[yusukebe]

This PR enables users can access the Cloudflare values - cf, ctx, and caches on Remix routes.

Closes #5

[yusukebe]

Hey @jfsiii !

I've tried to implement regarding the issue. Can you review it?

[jfsiii]

@yusukebe Adding those items to the context is a step in the right direction, but it still doesn't allow users to add their own values to the context

As I quote in #5 , according to the Remix docs:

If you'd like to add additional properties to the load context, you should export a getLoadContext function from a shared module so that load context in Vite, Wrangler, and Cloudflare Pages are all augmented in the same way:

The official handler from Remix also accepts the getLoadContext

export const onRequest = createPagesFunctionHandler({
  build,
  getLoadContext,
});

Are you opposed to allowing this? It's a critical feature for me. I have this patched/working locally and am happy to send a PR if that helps.

[yusukebe]

@jfsiii Thank you for the comment.

Are you opposed to allowing this? It's a critical feature for me. I have this patched/working locally and am happy to send a PR if that helps.

No! It will be a good feature, and I'd like you to send a PR. So, first, I'll merge this PR, and after that, you will create a PR, okay?

[yusukebe]

I'm working on supporting getLoadContext. Please review it later.

[yusukebe]

@jfsiii

I've added the code to support getLoadContext with user values.

For dev server, you can add getLoadContext in vite.config.ts: https://github.com/yusukebe/hono-remix-adapter/pull/6/files#diff-6671dd857b3a02ae74576b82e0e7088e90b9c2220259fe16cf78a5833a2c6e10R24

For Cloudflare Pages, add it here: https://github.com/yusukebe/hono-remix-adapter/pull/6/files#diff-26d9f92c12cdb12bbf15be7f110c94f8bd7557c13b4929cb240829e6d4d13f68R7

What do you think about this?

[jfsiii]

TL;DR It works! Ship it 🚀

I left lots of comments/questions, but nothing that I think should hold up the PR. They'd all be easy enough to follow up on later. Especially during this 0.x period

[jfsiii]

I can't leave a comment on line 14, but it should be

    remixCloudflareDevProxy({
      getLoadContext,
    }),

to match the example vite.config.ts from https://remix.run/docs/en/main/guides/vite#augmenting-load-context

I'm not sure if/how that impacts serverAdapter.

[jfsiii]

Also, I'm not 💯 in my testing, but when I added

    remixCloudflareDevProxy({
      getLoadContext,
    }),

and removed getLoadContext from serverAdapter, the example app still passed my bindings and augmented context through

[yusukebe]

About remixCloudflareDevProxy, perhaps it may not need it because this hono-remix-adapter does the same thing. I'll test it; if so, I will create the PR.

[jfsiii]

It seems like we could do more than any here.

It might be tricky/impractical to use the "real" Env and PlatformProxy types, but I think this works

Suggested change

	// Relaxing the type definition
	// eslint-disable-next-line @typescript-eslint/no-explicit-any
	cloudflare: any
	cloudflare: {
	env: Record<string, unknown>
	cf: IncomingRequestCfProperties
	ctx: ExecutionContext
	caches: typeof caches
	}

[jfsiii]

Names are hard, but if you gave that type a name like:

type SomeContext = {
  cloudflare: {
    env: Record<string, unknown>
    cf: IncomingRequestCfProperties
    ctx: ExecutionContext
    caches: typeof caches
  }
};

We could use it like

export type GetLoadContext = (args: {
  request: Request
  context: SomeContext
}) => AppLoadContext

export const defaultGetLoadContext = ({ context }: { context: SomeContext}) => {

[yusukebe]

If you override the definition of AppLoadContext in your app: https://github.com/yusukebe/hono-remix-adapter/pull/6/files#diff-fe3b5413a3ab56944fc97a3eb9b71e83c41e4d0b41a2303932ae92bd8c82cc8fR11-R13

The type mismatch will occur. To remove the typo error, I've added the any. This may be fixed in the future, but it remains as any.

[jfsiii]

@yusukebe Oh, right. Apologies for not testing that locally. I think 0.2.0 shipped with this type, because I'm getting this type error in my app after updating

Nothing a @ts-expect-error can't fix, though

[yusukebe]

@jfsiii

Oooops. Sorry. The building way was wrong. Fixed it and released the new version. Try v0.2.1.

[jfsiii]

No worries. It happens. And it was kind of my fault, anyway.

Thanks!

[jfsiii]

I wasn't sure about this one. Do we want c.env which will return

{
      "MY_VAR": "My Valuez",
      "ASSETS": {},
      "eventContext": {
        "request": {},
        "functionPath": "/",
        "params": {},
        "data": {},
        "env": {
          "MY_VAR": "My Valuez",
          "ASSETS": {}
        }
      }
}

or c.env.eventContext.env, which would only return

        {
          "MY_VAR": "My Valuez",
          "ASSETS": {}
        }

[yusukebe]

This function is also used in the dev server: https://github.com/yusukebe/hono-remix-adapter/pull/6/files#diff-34d76a96faa930c4b9909c055beb422e601e2267f72ce12d6f15a1c2cc6ea914R23

It should be c.env to get MY_VAR correctly since it does not have eventContext.

[yusukebe]

Hey @jfsiii

Thank you for the comments. Some remain, but I'll merge this now and release the new version (maybe v0.2.0) that includes this update!