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

Islands that use useRequestContext are unresponsive #96

Closed
BasWilson opened this issue Feb 26, 2024 · 12 comments
Closed

Islands that use useRequestContext are unresponsive #96

BasWilson opened this issue Feb 26, 2024 · 12 comments
Labels
bug Something isn't working

Comments

@BasWilson
Copy link

BasWilson commented Feb 26, 2024

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

@BasWilson BasWilson added the bug Something isn't working label Feb 26, 2024
@yusukebe
Copy link
Member

Hi @BasWilson

This might be a bug!

@usualoma Could you investigate this matter?

@usualoma
Copy link
Member

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
Copy link
Author

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

@usualoma
Copy link
Member

@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
Copy link
Member

@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
Copy link
Member

usualoma commented Feb 28, 2024

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
Copy link
Member

@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
Copy link
Member

@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
Copy link
Member

usualoma commented Mar 1, 2024

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.

  • Output to the server-side request log
  • Throw an exception at bundle time if islands component contains useRequestContext()

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.

#96 (comment)

@yusukebe
Copy link
Member

yusukebe commented Mar 1, 2024

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
Copy link
Member

usualoma commented Mar 1, 2024

@yusukebe Thank you!

@yusukebe
Copy link
Member

yusukebe commented Mar 2, 2024

I think this can be closed. Thanks!

@yusukebe yusukebe closed this as completed Mar 2, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

3 participants