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

<honox-island> tag breaks direct child CSS #141

Open
ersinakinci opened this issue Apr 10, 2024 · 10 comments
Open

<honox-island> tag breaks direct child CSS #141

ersinakinci opened this issue Apr 10, 2024 · 10 comments
Labels
bug Something isn't working

Comments

@ersinakinci
Copy link

What version of HonoX are you using?

0.1.9

What steps can reproduce the bug?

  1. Create some global CSS that affects direct children. For example, label > input { ... }
  2. Write an island component with an outer tag that should be affected by your CSS. For example, const MyInput = () => <input />
  3. Write JSX with an island component that's nested within the CSS parent. For example, <label><MyInput /></label>

What is the expected behavior?

The CSS rule should be respected and affect the styling of the island component as if you were using the outer tag without an island.

What do you see instead?

HonoX generates a wrapper <honox-island> tag that interferes with the CSS parent/child relationship and the CSS rule isn't applied.

Additional information

No response

@ersinakinci ersinakinci added the bug Something isn't working label Apr 10, 2024
@yusukebe
Copy link
Member

@ersinakinci

Sorry for the delayed reply.

HonoX generates a wrapper <honox-island> tag that interferes with the CSS parent/child relationship and the CSS rule isn't applied.

Yeah, this causes some problems, including the CSS issue. We'll try to fix it.

@yusukebe
Copy link
Member

Astro has the same issue. One user wrote a blog post about how to apply CSS to islands in the Astro application:

https://zellwk.com/blog/overcoming-astro-styling-frustrations/

Astro and Hono are using the same strategy for islands that apply additional tags like hono-island or astro-island. It causes the problem. This means to avoid this, we have to give up using the strategy. So I'm wondering if we can use a "comment" strategy: #158

However, if using the "comment" strategy is difficult or impossible, we have to keep using hono-island and make this CSS matter a "known issue".

@yusukebe
Copy link
Member

Hi @usualoma

I need your help! What do you think about this issue? I've been struggling to resolve it for a long time, but I can't. This means using hono-island tags will break HTML structures, so it causes unexpected behavior for applying CSS. Another way to implement the "comment" strategy is very difficult.

@usualoma
Copy link
Member

usualoma commented Apr 27, 2024

This is a difficult issue.

I understand that the current hono-island affects evaluating CSS.

To avoid the impact on evaluating CSS, I think a comment node would be appropriate, as you say, but we can't directly put it out in React.

If we were to do this, we could use the script element
It's like #158 (comment) or rewrite it with a regex in renderer.
https://github.com/honojs/middleware/blob/main/packages/react-renderer/src/react-renderer.ts#L57

If you ask whether it's possible or impossible, I think it's possible, but it's not so clean.

If this is only "to solve the CSS problem", then I feel that the current hono-island approach would be better.

#155 (which doesn't happen with hono/jsx/dom, but with React's hydrateRoot, I think) is worth considering if this solves the problem. (I haven't checked to see if this solves it)

Well, Maaaybe,

#158 (comment)

Although this is not clean, I think the performance is not so bad if we implement it with a bit of ingenuity. I think I'll give it a try.

@usualoma
Copy link
Member

@yusukebe

Ah, using React, would we be able to insert a component later in the middle of a DOM node such as

<div>
Before.
<ClientComponent />
After
</div>

Current honox is fine as <honox-island> is the target to hydrate, but if we use comments to search for targets, we lose a single hydrate destination I don't think this is possible with React's API. (Not even the current hono/jsx/dom can do this, but I think we can add the functionality)

Do you know if such hydration is possible in React?

@yusukebe
Copy link
Member

@usualoma Thank you for your response!

If this is only "to solve the CSS problem", then I feel that the current hono-island approach would be better.

I'm also thinking so. Since, as you said, using the comment strategy is "not clean," it will be better to use hono-island.

Do you know if such hydration is possible in React?

Hmm. Indeed, I think it is impossible...

By the way, Fresh is following the comment strategy:

https://fresh.deno.dev/docs/concepts/islands#passing-other-props-to-islands

<!--frsh-myisland_default:default:0-->
<div>
  Counter is at 0.
  <button>+</button>
  <!--frsh-slot-myisland_default:children-->
  <p>This text is rendered on the server</p>
  <!--/frsh-slot-myisland_default:children-->
</div>
<!--/frsh-myisland_default:default:0-->

But in the Fresh case, it uses only Preact, so it can add comments in the rendering phase. What we want to do is support any JSX UI library, at least hono/JSX, and React, so we have to do it in the Vite/JSX translation phase. Both are different.


Then, my current answer is to keep using hono-island though we have to resolve #155.

@usualoma
Copy link
Member

I have an idea to solve #155 but I think we need to use useContext on the server side.

Set up as follows,

export default defineConfig({
  plugins: [honox({
    useContextModule: 'react',.
    // this setting is used in server side
    // default is 'hono/jsx'
  })],
})

The following line is automatically inserted.

import { useContext } from 'react'

I don't want to add the server configuration item if possible, but I don't think there is any other way.

Do you think this is the way to do it?

@yusukebe
Copy link
Member

@usualoma

I don't want to add the server configuration item if possible, but I don't think there is any other way.

Hmm. I also don't want to do it.

One of my interesting ideas. How about using this method, which it can mark if it is a client component or not by adding $client attribute:

app.get('/', (c) => {
  return c.render(
    <>
      <Counter $client initial={3} />
    </>
  )
})

I've tried a super rough implementation as PoC:

https://github.com/yusukebe/hydrate-dollar-client

This demo can avoid a hydration error, and it is rendered as we expected. One of the good things is we can use named exported components, not just exported as a default. BUT, the interaction in the children component is not working. Anyway, can you see the repo?

@usualoma
Copy link
Member

I think it's an awesome idea! And you're right, the "Nemed exported component" problem is solved too!

hydrate-dollar-client

I think the implementation is also largely fine, but I think the biggest issue is here.
https://github.com/yusukebe/hydrate-dollar-client/blob/main/src/client.ts#L34
If I build the production version as is, I think all the code for the server will be included in the distribution files.

However, I think we need to think about solving the "nested islands" problem separately. I don't think the "hydrate-dollar-client" will solve this.

Also, if we want to use the "Nemed exported component", then the current '/app/islands/**/[a-zA-Z0-9[-]+. (tsx|ts)' as it is, we could wrap "functions exported with names beginning with a capital letter" in the same way as the default export. There should be no confusion, as utility functions are generally not exported with names beginning with a capital letter, and names beginning with a capital letter can be regarded as function components.

@usualoma
Copy link
Member

I don't want to add the server configuration item if possible, but I don't think there is any other way.

Hmm. I also don't want to do it.

Oh, I have a feeling that if I use tsconfig's, config.compilerOptions.jsxImportSource, I can run it in most environments without any additional configuration.
I'll try to write some code.

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