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

feat: Add hono enhancer to cloudflare #923

Draft
wants to merge 1 commit into
base: main
Choose a base branch
from
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
45 changes: 35 additions & 10 deletions packages/waku/src/cli.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,13 +3,13 @@ import { existsSync, readFileSync } from 'node:fs';
import { pathToFileURL } from 'node:url';
import { parseArgs } from 'node:util';
import { createRequire } from 'node:module';
import { Hono } from 'hono';
import { Hono, type MiddlewareHandler } from 'hono';
import { compress } from 'hono/compress';
import { serve } from '@hono/node-server';
import { serveStatic } from '@hono/node-server/serve-static';
import * as dotenv from 'dotenv';

import type { Config } from './config.js';
import type { Config, unstable_HonoEnhancer } from './config.js';
import { runner } from './lib/hono/runner.js';
import { build } from './lib/builder/build.js';
import { DIST_ENTRIES_JS, DIST_PUBLIC } from './lib/builder/constants.js';
Expand Down Expand Up @@ -98,13 +98,20 @@ if (values.version) {

async function runDev() {
const config = await loadConfig();
const honoEnhancer =
config.unstable_honoEnhancer || ((createApp) => createApp);
const honoEnhancer: unstable_HonoEnhancer<Hono, MiddlewareHandler> =
config.unstable_honoEnhancer
? (config.unstable_honoEnhancer as unknown as unstable_HonoEnhancer<
Hono,
MiddlewareHandler
>)
: async (createApp: (app: Hono, _serveWaku: MiddlewareHandler) => Hono) =>
createApp;
const serveWaku = runner({ cmd: 'dev', config, env: process.env as any });
const createApp = (app: Hono) => {
if (values['experimental-compress']) {
app.use(compress());
}
app.use(runner({ cmd: 'dev', config, env: process.env as any }));
app.use(serveWaku);
app.notFound((c) => {
// FIXME can we avoid hardcoding the public path?
const file = path.join('public', '404.html');
Expand All @@ -116,7 +123,10 @@ async function runDev() {
return app;
};
const port = parseInt(values.port || '3000', 10);
await startServer(honoEnhancer(createApp)(new Hono()), port);
await startServer(
(await honoEnhancer(createApp))(new Hono(), serveWaku),
port,
);
}

async function runBuild() {
Expand Down Expand Up @@ -147,16 +157,28 @@ async function runBuild() {
async function runStart() {
const config = await loadConfig();
const { distDir = 'dist' } = config;
const honoEnhancer =
config.unstable_honoEnhancer || ((createApp) => createApp);
const honoEnhancer: unstable_HonoEnhancer<Hono, MiddlewareHandler> =
config.unstable_honoEnhancer
? (config.unstable_honoEnhancer as unknown as unstable_HonoEnhancer<
Hono,
MiddlewareHandler
>)
: async (createApp: (app: Hono) => Hono) =>
(app: Hono, _serveWaku: MiddlewareHandler) =>
createApp(app);
const loadEntries = () =>
import(pathToFileURL(path.resolve(distDir, DIST_ENTRIES_JS)).toString());
const serveWaku = runner({
cmd: 'start',
loadEntries,
env: process.env as any,
});
const createApp = (app: Hono) => {
if (values['experimental-compress']) {
app.use(compress());
}
app.use(serveStatic({ root: path.join(distDir, DIST_PUBLIC) }));
app.use(runner({ cmd: 'start', loadEntries, env: process.env as any }));
app.use(serveWaku);
app.notFound((c) => {
// FIXME better implementation using node stream?
const file = path.join(distDir, DIST_PUBLIC, '404.html');
Expand All @@ -168,7 +190,10 @@ async function runStart() {
return app;
};
const port = parseInt(values.port || '8080', 10);
await startServer(honoEnhancer(createApp)(new Hono()), port);
await startServer(
(await honoEnhancer(createApp))(new Hono(), serveWaku),
port,
);
}

function startServer(app: Hono, port: number) {
Expand Down
10 changes: 6 additions & 4 deletions packages/waku/src/config.ts
Original file line number Diff line number Diff line change
Expand Up @@ -49,14 +49,16 @@ export interface Config {
*/
middleware?: () => Promise<{ default: Middleware }>[];
/**
* Enhander for Hono
* Enhancer for Hono
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks.

* Defaults to `undefined`
*/
unstable_honoEnhancer?:
| (<Hono>(createApp: (app: Hono) => Hono) => (app: Hono) => Hono)
| undefined;
unstable_honoEnhancer?: unstable_HonoEnhancer | undefined;
}

export type unstable_HonoEnhancer<Hono = never, MiddlewareHandler = never> = (
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would like to avoid exporting type from this file. If you need it in the plugin, simply define it there.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, that sounds good.

createApp: (app: Hono) => Hono,
) => Promise<(app: Hono, serveWaku: MiddlewareHandler) => Hono>;
Copy link
Owner

@dai-shi dai-shi Sep 30, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Promise<> is fine, but exposing serveWaku is too much. It doesn't look like a nice enhancer abstraction.

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Another idea would be:

(createApp: (app: Hono) => Promise<Hono>) => (app: Hono) => Promise<Hono>

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

accept an async function to support dynamic imports.

Hm, on second thought, I wonder if promise is really necessary. Do you really need dynamic imports?

Can you try top-level await?

At this point, it's not convincing to change unstable_honoEnhancer.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you try top-level await?

Yes, I can try. It needs to import for the waku dev command and not import during build. I'm not sure if there is a better way to do that than using import.meta.env.PROD.

exposing serveWaku is too much. It doesn't look like a nice enhancer abstraction.

Exposing the built Hono middleware (serveWaku) is useful if the user doesn't want Waku to be the default route. I agree that it doesn't look as clean. Maybe we should leave it out until someone has a more compelling use case? I am fine with transforming the build output for my personal use cases.

I will make adjustments to this PR to remove the API changes.

I will also try to fix the error it throws when deployed to Cloudflare. I tried using top-level await there but the same error occurred.

Copy link
Owner

@dai-shi dai-shi Sep 30, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

useful if the user doesn't want Waku to be the default route.

We expose runner for such use cases. I don't think they would use unstable_honoEnhancer in such cases. But, maybe I miss some use cases.

I agree that it doesn't look as clean.

I would like it to be clean. 😄

Yes, I can try.

Let me know if it doesn't work. There might be other workarounds.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We expose runner for such use cases. I don't think they would use unstable_honoEnhancer in such cases. But, maybe I miss some use cases.

OK great. I will try that for the use case I am thinking of.


export function defineConfig(config: Config) {
return config;
}
51 changes: 33 additions & 18 deletions packages/waku/src/lib/plugins/vite-plugin-deploy-cloudflare.ts
Original file line number Diff line number Diff line change
Expand Up @@ -17,34 +17,49 @@ import { DIST_ENTRIES_JS, DIST_PUBLIC } from '../builder/constants.js';
const SERVE_JS = 'serve-cloudflare.js';

const getServeJsContent = (srcEntriesFile: string) => `
import { runner, importHono } from 'waku/unstable_hono';
import { runner, importHono } from "waku/unstable_hono";

const { Hono } = await importHono();

const loadEntries = () => import('${srcEntriesFile}');
const loadEntries = () => import("${srcEntriesFile}");
let serveWaku;

const app = new Hono();
app.use((c, next) => serveWaku(c, next));
app.notFound(async (c) => {
const assetsFetcher = c.env.ASSETS;
const url = new URL(c.req.raw.url);
const errorHtmlUrl = url.origin + '/404.html';
const notFoundStaticAssetResponse = await assetsFetcher.fetch(
new URL(errorHtmlUrl),
);
if (notFoundStaticAssetResponse && notFoundStaticAssetResponse.status < 400) {
return c.body(notFoundStaticAssetResponse.body, 404);
}
return c.text('404 Not Found', 404);
});
const configPromise = loadEntries().then((entries) => entries.loadConfig());

const createApp = (app) => {
app.use((c, next) => serveWaku(c, next));
app.notFound(async (c) => {
const assetsFetcher = c.env.ASSETS;
const url = new URL(c.req.raw.url);
const errorHtmlUrl = url.origin + "/404.html";
const notFoundStaticAssetResponse = await assetsFetcher.fetch(
new URL(errorHtmlUrl)
);
if (
notFoundStaticAssetResponse &&
notFoundStaticAssetResponse.status < 400
) {
return c.body(notFoundStaticAssetResponse.body, 404);
}
return c.text("404 Not Found", 404);
});
return app;
};

let honoEnhanced;

export default {
async fetch(request, env, ctx) {
if (!serveWaku) {
serveWaku = runner({ cmd: 'start', loadEntries, env });
serveWaku = runner({ cmd: "start", loadEntries, env });
}
if (!honoEnhanced) {
const honoEnhancer =
(await configPromise).unstable_honoEnhancer ||
(async (createApp) => createApp);
honoEnhanced = (await honoEnhancer(createApp))(new Hono(), serveWaku);
}
return app.fetch(request, env, ctx);
return honoEnhanced.fetch(request, env, ctx);
},
};
`;
Expand Down
Loading