-
-
Notifications
You must be signed in to change notification settings - Fork 124
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
feat(deploy): add cloudflare context to platform object #872
feat(deploy): add cloudflare context to platform object #872
Conversation
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
This pull request is automatically built and testable in CodeSandbox. To see build info of the built libraries, click here or the icon next to each commit SHA. |
81632d4
to
58d6cba
Compare
58d6cba
to
5e66745
Compare
5e66745
to
9d9d5a7
Compare
9d9d5a7
to
be9cedb
Compare
be9cedb
to
1992a35
Compare
1992a35
to
006eeca
Compare
I'm getting close. I just need to do testing on this latest implementation. Does https://github.com/dai-shi/waku/pull/872/files#diff-cd6a8e530f81183dc69573add03d2338dfb64976b74eec5eb387e66a713e6522R45-R53 look good? I'm trying to remove non-string vars that Cloudflare puts in env before passing to the runner. In addition to the bindings, I found that cloudflare allows JSON in env vars: https://developers.cloudflare.com/workers/configuration/environment-variables/#add-environment-variables-via-wrangler |
There are some typescript issues due to differences in workerd built-in types like Request/Response but I worked around them with casting. |
006eeca
to
bf95b66
Compare
Example usage from a server component: ```ts import { unstable_getPlatformObject } from 'waku/server'; const getData = async () => { const { env, executionCtx } = unstable_getPlatformObject() as { env: Env; executionCtx: ExecutionContext }; executionCtx?.waitUntil( new Promise<void>((resolve) => { console.log("Waiting for 5 seconds"); setTimeout(() => { console.log("OK, done waiting"); resolve(); }, 5000); }), ); const { results } = await env.DB.prepare( "SELECT * FROM users WHERE user_id = ?", ) .bind(userId) .all(); return results; }; ```
bf95b66
to
497b69f
Compare
@@ -1,41 +1,75 @@ | |||
import { Hono } from 'hono'; | |||
import { unstable_getPlatformObject } from 'waku/server'; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It might not change anything real, but for consistency. (Will be refactored anyway.)
import { unstable_getPlatformObject } from 'waku/server'; | |
import { unstable_getPlatformObject } from '../../server.js'; |
import { runner } from '../hono/runner.js'; | ||
import type { | ||
ExportedHandler, | ||
fetch, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is confusing. So:
fetch, | |
fetch as CloudflareFetch, |
throw new Error('serveWaku is not initialized'); | ||
} | ||
const platform = unstable_getPlatformObject(); | ||
platform.honoContext = c; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is not good.
The platform object is something stable all the time. It shouldn't change for each request.
unstable_getCustomContext
is for each request. And, the hono context is already exposed. #852
export function unstable_getPlatformObject< | ||
T = Record<string, unknown>, | ||
>(): PlatformObject<T> { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I believe asserting types is more appropriate than annotating types, because it never be type safe.
}, | ||
// TODO ability to add other handlers or Durable Objects? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Unless we do this, this PR doesn't provide any more capability than #852.
@dai-shi Thank you reviewing the PR. You mentioned the context API before and I saw #852 but I hadn't found Is it correct to access it like this? import type { ExecutionContext } from '@cloudflare/workers-types/experimental';
import { unstable_getCustomContext } from 'waku/server';
const getData = async ({ userId }: { userId: string }) => {
const { env, executionCtx } = unstable_getCustomContext().__hono_context as {
env: Env
executionCtx: ExecutionContext
};
executionCtx?.waitUntil(
new Promise<void>((resolve) => {
console.log("Waiting for 5 seconds");
setTimeout(() => {
console.log("OK, done waiting");
resolve();
}, 5000);
}),
);
const { results } = await env.DB.prepare("SELECT * FROM users WHERE user_id = ?")
.bind(userId)
.all();
return results;
} I think I will close this PR. Re: How could we allow a developer to inject handlers and export Durable Object classes from their worker index file? The way I have been doing it is to build waku and then copy in a custom index.js file to the dist folder. We could make that an option to the cloudflare deploy vite plugin - a path to an alternate worker entry file. Maybe a cloudflare deploy vite plugin option could point to an alternate worker entry file that would be used instead of the default. Or could there be another way to implement it using getPlatformObject()? |
Looks good.
Oh, I think I misunderstood something. |
@rmarscher see #884 |
Example usage from a server component:
Server console output:
The page was already sent to the client and loaded while the promise was still executing.
It is necessary to cast the Cloudflare types. See https://developers.cloudflare.com/workers/languages/typescript/#generate-types-that-match-your-workers-configuration-experimental for info on generating types for your project.