honojs / honox

HonoX - Hono based meta framework
https://hono.dev
MIT License
1.64k stars 43 forks source link

Islands that use useRequestContext are unresponsive #96

Closed BasWilson closed 8 months ago

BasWilson commented 8 months ago

What version of HonoX are you using?

0.1.5

What steps can reproduce the bug?

Create a basic island with the useRequestContext hook.

import { useState } from 'hono/jsx';
import { useRequestContext } from 'hono/jsx-renderer';

export default function Counter() {
    const c = useRequestContext()
    const [count, setCount] = useState(0)
    return (
        <div>
            <p>Count: {count}</p>
            <button onClick={() => {
                setCount(count + 1)
            }}>Increment</button>
        </div>
    )
}

What is the expected behavior?

The count state should increment.

What do you see instead?

No increments.

Additional information

No response

yusukebe commented 8 months ago

Hi @BasWilson

This might be a bug!

@usualoma Could you investigate this matter?

usualoma commented 8 months ago

Hi @BasWilson Thank you for your comment.

Unfortunately, useRequestContext() throws an exception when used in the island component, so this is the behavior of the specification.

@yusukebe

In island, there is nothing that can be returned from useRequestContext(). It is possible to return an alternative value or return undefined, but it would be incomplete and not what the user expects, so I think it is better to throw an exception as we do now. What do you think?

BasWilson commented 8 months ago

Ah I see, thought as much. But where would the exception be visible? See none thrown in server on client logs.

usualoma commented 8 months ago

@BasWilson

Thanks for the confirmation. Currently, an error appears in the JavaScript console of the web browser that displayed the website

CleanShot 2024-02-28 at 09 16 03@2x

It would be better if we could do something like "if useRequestContext() is called on a component placed in island, a warning will be displayed on the server side as well", but that is not done in the current hono.

yusukebe commented 8 months ago

@usualoma

In island, there is nothing that can be returned from useRequestContext(). It is possible to return an alternative value or return undefined, but it would be incomplete and not what the user expects, so I think it is better to throw an exception as we do now. What do you think?

I'd like to confirm, does this mean that we can't use useRequestContext() only on the client?

I understand that we cannot refer to the Context value on the client. However, we can get the value from the Context on the server side.

I think there are use cases where we want to use the value of the Context in Island components. Is it possible to render the value on the server only and pass the value to the client and not get an error?

usualoma commented 8 months ago

Let's see, if an application wants to use the following component,

import { useState } from 'hono/jsx';
import { useRequestContext } from 'hono/jsx-renderer';

export default function Counter() {
    const c = useRequestContext()
    const [count, setCount] = useState(parseInt(c.req.query("c") ?? "0", 10))
    return (
        <div>
            <p>Count: {count}</p>
            <button onClick={() => {
                setCount(count + 1)
            }}>Increment</button>
        </div>
    )
}

In island, I would like to see the following used

import { useState } from 'hono/jsx';

export default function Counter({init}) {
    const [count, setCount] = useState(init)
    return (
        <div>
            <p>Count: {count}</p>
            <button onClick={() => {
                setCount(count + 1)
            }}>Increment</button>
        </div>
    )
}
// server component
import { useRequestContext } from 'hono/jsx-renderer';
import Counter from '../islands/counter.tsx';

export default () => {
    const c = useRequestContext()
    return <Counter init={parseInt(c.req.query("c") ?? "0", 10) />
}
usualoma commented 8 months ago

@yusukebe

Is it possible to render the value on the server only and pass the value to the client and not get an error?

I believe this is impssible because it is not possible to identify "which value was retrieved, processed, and output" on the server component side. Do you have code for the island component that says "this component should work"?

yusukebe commented 8 months ago

@usualoma

Sorry to take the time to explain. You are right, that is impossible. Plus, if we want to use a context value in the Island component, we should pass the value that way from the server side.

For this issue, I think it is a good idea to throw an error if useRequestContext() is called in the Island component.

usualoma commented 8 months ago

Hi @yusukebe

I think it would be a little more convenient if the following were done, but I think it would be difficult to accomplish this without compromising performance and without using more black-magic methods.

As noted in the comments below, the client is currently throwing errors and can identify the error location, so I, for my part, would prefer no additional action at this time.

https://github.com/honojs/honox/issues/96#issuecomment-1967957518

yusukebe commented 8 months ago

Good morning @usualoma

Yes, it would be nice to do that, but we don't want to do anything magical. You are right. I like it as it is now. But, I've written to the document about this: #108

Thanks!

usualoma commented 8 months ago

@yusukebe Thank you!

yusukebe commented 8 months ago

I think this can be closed. Thanks!