Repository: honojs/honox
Pull request #168 (2024-05-03 21:12:19)

feat: `Script` and `HasIsland` available at multiple runtimes with `AsyncLocalStorage`
#167

This appears to be a good and simple solution to implement.
However (I am not familiar with Cloudflare Workers, so maybe there is another solution) it appears that `compatibility_flags = [ "nodejs_compat"]` needs to be added to wrangler.toml.

yusukebe: `AsyncLocalStorage`! We will be able to use `AsyncLocalStorage` in other places, so it's not bad to use it.

> `compatibility_flags = [ "nodejs_compat"]` 

Yes, we have to set it. But there is a difference between Cloudflare **Workers** and Cloudflare **Pages**. We intent HonoX works on Pages, I'll check whether that setting works or not in Pages.
yusukebe: Hmm. It works well in local development. For production. When we use Cloudflare **Workers**, we write `compatibility_flags` on `wrangler. toml`. But currently, if we want to use **Pages**, we need to set it up on the Dashboard. This means if the user runs the `npm run deploy` command, it will be uploaded, but the deployment will fail the first time. This is not a good DX, though the Cloudflare team may work on it.
yusukebe: For this matter, I was thinking of providing the components by each adapter separate from `honox/server` like this:

```ts
import { Script } from 'honox/react/server'
```

Or creating `@honox/react` package:

```ts
import { Script } from '@honox/react/server'
```

With this method, we can introduce the method for `createClient`.

```ts
// app/client.ts
import { createClient } from 'honox/react/client'
// Or, import { createClient } from '@honox/react/client'

createClient()
```

Anyway, it would be best if we didn't have to make the adapter per UI library.
usualoma: @yusukebe 

Great!!

> Sorry!! I was mistaken. I asked a Cloudflare Pages team, and I tried it, and now AsyncLocalStorage works on production without setting it on the dashboard! This means a user can deploy it by only writing the flag on wrangler.toml.

#170 is part of a number of things we've tried, so don't worry about it.

### If we merge this PR.

There is one point of concern. If we merge this, `wrangler.toml` will be required in `wrangler dev` and production cloudflare environment, even if the app does not use `HasIsland`. If existing users update, they will need to create (or maybe update) `wrangler.toml` after the update. Would it be possible to do it that way?

I tried "importing AsyncLocalStorage dynamically and not raising an error if import fails", but it didn't work, and even if it did work, I don't think it would be good from a DX perspective.

### Benefits of being able to use AsyncLocalStorage

This would provide a "renderer-independent useRequestContext".  Even if (and I don't know if this is actually the case) the user is using their own `preact` renderer, the renderer does not have to implement own `useRequestContext`.

Thus, if we decide to make honox itself (not only HasIsland) dependent on `AsyncLocalStorage`, we can proceed with the content of this PR. If the `requirement to create (or update) wrangler.toml` is a problem, we may want to consider a different approach.
yusukebe: @usualoma 

> There is one point of concern. If we merge this, `wrangler.toml` will be required in `wrangler dev` and production cloudflare environment, even if the app does not use `HasIsland`. If existing users update, they will need to create (or maybe update) `wrangler.toml` after the update. Would it be possible to do it that way?

I think it's not a blocker. We need just update [the starter template](https://github.com/honojs/starter/tree/main/templates/x-basic) and write about the `nodejs_compat` flag on `README`. Of course, if an existing user updates `honox`, their application may not work on the production. But we don't have to be so serious because HonoX is still in "alpha" status, and using `AsyncLocalStorage` is worth more than the pain.

> Benefits of being able to use AsyncLocalStorage

Yeah. I was thinking `AsyncLocalStorage` would be a super powerful API for us. 

> Thus, if we decide to make honox itself (not only HasIsland) dependent on `AsyncLocalStorage`, we can proceed with the content of this PR. 

Let's go with `AsyncLocalStorage`!




usualoma: @yusukebe 
Understood!
I think we can almost go with current PR, but give me a minute to review it once.
usualoma: @yusukebe 
Added a little test in 4d52781.
I think we can now merge this PR!
yusukebe: Hi @usualoma 

Thanks! Let's go with this.

Patch:
diff --git a/src/server/components/has-islands.tsx b/src/server/components/has-islands.tsx
index b100b3b..dbfb21a 100644
--- a/src/server/components/has-islands.tsx
+++ b/src/server/components/has-islands.tsx
@@ -1,8 +1,11 @@
-import type { FC } from 'hono/jsx'
-import { useRequestContext } from 'hono/jsx-renderer'
 import { IMPORTING_ISLANDS_ID } from '../../constants.js'
+import { contextStorage } from '../context-storage.js'
 
-export const HasIslands: FC = ({ children }) => {
-  const c = useRequestContext()
-  return <>{c.get(IMPORTING_ISLANDS_ID) ? children : <></>}</>
+// eslint-disable-next-line @typescript-eslint/no-explicit-any
+export const HasIslands = ({ children }: { children: any }): any => {
+  const c = contextStorage.getStore()
+  if (!c) {
+    throw new Error('No context found')
+  }
+  return <>{c.get(IMPORTING_ISLANDS_ID) && children}</>
 }
diff --git a/src/server/components/script.tsx b/src/server/components/script.tsx
index 3552d4f..062071c 100644
--- a/src/server/components/script.tsx
+++ b/src/server/components/script.tsx
@@ -1,4 +1,3 @@
-import type { FC } from 'hono/jsx'
 import type { Manifest } from 'vite'
 import { HasIslands } from './has-islands.js'
 
@@ -10,7 +9,8 @@ type Options = {
   nonce?: string
 }
 
-export const Script: FC<Options> = async (options) => {
+// eslint-disable-next-line @typescript-eslint/no-explicit-any
+export const Script = (options: Options): any => {
   const src = options.src
   if (options.prod ?? import.meta.env.PROD) {
     let manifest: Manifest | undefined = options.manifest
diff --git a/src/server/context-storage.ts b/src/server/context-storage.ts
new file mode 100644
index 0000000..4e958cf
--- /dev/null
+++ b/src/server/context-storage.ts
@@ -0,0 +1,3 @@
+import { AsyncLocalStorage } from 'node:async_hooks'
+import type { Context } from 'hono'
+export const contextStorage = new AsyncLocalStorage<Context>()
diff --git a/src/server/server.ts b/src/server/server.ts
index ec79a31..480ba57 100644
--- a/src/server/server.ts
+++ b/src/server/server.ts
@@ -10,6 +10,7 @@ import {
   listByDirectory,
   sortDirectoriesByDepth,
 } from '../utils/file.js'
+import { contextStorage } from './context-storage.js'
 
 const NOTFOUND_FILENAME = '_404.tsx'
 const ERROR_FILENAME = '_error.tsx'
@@ -62,6 +63,11 @@ export const createApp = <E extends Env>(options: BaseServerOptions<E>): Hono<E>
   const app = options.app ?? new Hono()
   const trailingSlash = options.trailingSlash ?? false
 
+  // Share context by AsyncLocalStorage
+  app.use(async function ShareContext(c, next) {
+    await contextStorage.run(c, () => next())
+  })
+
   if (options.init) {
     options.init(app)
   }
diff --git a/src/vite/island-components.ts b/src/vite/island-components.ts
index c2aa466..995af2b 100644
--- a/src/vite/island-components.ts
+++ b/src/vite/island-components.ts
@@ -244,7 +244,7 @@ export function islandComponents(options?: IslandComponentsOptions): Plugin {
     },
 
     async load(id) {
-      if (/\/honox\/.*?\/vite\/components\//.test(id)) {
+      if (/\/honox\/.*?\/(?:server|vite)\/components\//.test(id)) {
         if (!reactApiImportSource) {
           return
         }


Test patch:
diff --git a/src/vite/island-components.test.ts b/src/vite/island-components.test.ts
index d9b8edc..3523cbf 100644
--- a/src/vite/island-components.test.ts
+++ b/src/vite/island-components.test.ts
@@ -1,3 +1,5 @@
+import fs from 'fs'
+import os from 'os'
 import path from 'path'
 import { transformJsxTags, islandComponents } from './island-components'
 
@@ -218,30 +220,67 @@ export { utilityFn, WrappedExportViaVariable as default };`
 
 describe('options', () => {
   describe('reactApiImportSource', () => {
-    // get full path of honox-island.tsx
-    const component = path
-      .resolve(__dirname, '../vite/components/honox-island.tsx')
-      // replace backslashes for Windows
-      .replace(/\\/g, '/')
-
-    // prettier-ignore
-    it('use \'hono/jsx\' by default', async () => {
-      const plugin = islandComponents()
-      await (plugin.configResolved as Function)({ root: 'root' })
-      const res = await (plugin.load as Function)(component)
-      expect(res.code).toMatch(/'hono\/jsx'/)
-      expect(res.code).not.toMatch(/'react'/)
+    describe('vite/components', () => {
+      // get full path of honox-island.tsx
+      const component = path
+        .resolve(__dirname, '../vite/components/honox-island.tsx')
+        // replace backslashes for Windows
+        .replace(/\\/g, '/')
+
+      // prettier-ignore
+      it('use \'hono/jsx\' by default', async () => {
+        const plugin = islandComponents()
+        await (plugin.configResolved as Function)({ root: 'root' })
+        const res = await (plugin.load as Function)(component)
+        expect(res.code).toMatch(/'hono\/jsx'/)
+        expect(res.code).not.toMatch(/'react'/)
+      })
+
+      // prettier-ignore
+      it('enable to specify \'react\'', async () => {
+        const plugin = islandComponents({
+          reactApiImportSource: 'react',
+        })
+        await (plugin.configResolved as Function)({ root: 'root' })
+        const res = await (plugin.load as Function)(component)
+        expect(res.code).not.toMatch(/'hono\/jsx'/)
+        expect(res.code).toMatch(/'react'/)
+      })
     })
 
-    // prettier-ignore
-    it('enable to specify \'react\'', async () => {
-      const plugin = islandComponents({
-        reactApiImportSource: 'react',
+    describe('server/components', () => {
+      const tmpdir = os.tmpdir()
+
+      // has-islands.tsx under src/server/components does not contain 'hono/jsx'
+      // 'hono/jsx' is injected by `npm run build`
+      // so we need to create a file with 'hono/jsx' manually for testing
+      const component = path
+        .resolve(tmpdir, 'honox/dist/server/components/has-islands.js')
+        // replace backslashes for Windows
+        .replace(/\\/g, '/')
+      fs.mkdirSync(path.dirname(component), { recursive: true })
+      // prettier-ignore
+      fs.writeFileSync(component, 'import { jsx } from \'hono/jsx/jsx-runtime\'')
+
+      // prettier-ignore
+      it('use \'hono/jsx\' by default', async () => {
+        const plugin = islandComponents()
+        await (plugin.configResolved as Function)({ root: 'root' })
+        const res = await (plugin.load as Function)(component)
+        expect(res.code).toMatch(/'hono\/jsx\/jsx-runtime'/)
+        expect(res.code).not.toMatch(/'react\/jsx-runtime'/)
+      })
+
+      // prettier-ignore
+      it('enable to specify \'react\'', async () => {
+        const plugin = islandComponents({
+          reactApiImportSource: 'react',
+        })
+        await (plugin.configResolved as Function)({ root: 'root' })
+        const res = await (plugin.load as Function)(component)
+        expect(res.code).not.toMatch(/'hono\/jsx\/jsx-runtime'/)
+        expect(res.code).toMatch(/'react\/jsx-runtime'/)
       })
-      await (plugin.configResolved as Function)({ root: 'root' })
-      const res = await (plugin.load as Function)(component)
-      expect(res.code).not.toMatch(/'hono\/jsx'/)
-      expect(res.code).toMatch(/'react'/)
     })
   })
 })
diff --git a/test-integration/api.test.ts b/test-integration/api.test.ts
index 4b95232..1487762 100644
--- a/test-integration/api.test.ts
+++ b/test-integration/api.test.ts
@@ -17,6 +17,7 @@ describe('Basic', () => {
 
   it('Should have correct routes', () => {
     const routes = [
+      { path: '/*', method: 'ALL', handler: expect.anything() }, // ShareContext
       { path: '/*', method: 'ALL', handler: expect.anything() },
       {
         path: '/about/*',
diff --git a/test-integration/apps.test.ts b/test-integration/apps.test.ts
index a47a023..2394712 100644
--- a/test-integration/apps.test.ts
+++ b/test-integration/apps.test.ts
@@ -18,6 +18,11 @@ describe('Basic', () => {
 
   it('Should have correct routes', () => {
     const routes = [
+      {
+        path: '/*',
+        method: 'ALL',
+        handler: expect.any(Function), // ShareContext
+      },
       {
         path: '/*',
         method: 'ALL',