solidjs / solid

A declarative, efficient, and flexible JavaScript library for building user interfaces.
https://solidjs.com
MIT License
31.94k stars 910 forks source link

Hydration key warning when using JSX prop multiple times #1485

Open mishimalisa opened 1 year ago

mishimalisa commented 1 year ago

Describe the bug

When using a component passed through props inside a classList, causes a warning about hydration keys.

Your Example Website or App

https://stackblitz.com/edit/solid-start-bare-ts-t4ifr6?file=src%2Froutes%2Findex.tsx

Steps to Reproduce the Bug or Issue

  1. start the project npm run dev
  2. open the console
  3. refresh the page
  4. see the warnings about hydration key on console

Expected behavior

As a user, I expect to not see hydration warnings.

Screenshots or Videos

No response

Platform

solid-js 1.6.9

Additional context

No response

ryansolid commented 1 year ago

Thanks for reporting. This is an interesting problem and one that is actually a little tricky to solve at the moment. There are ways to work around it but it definitely should be addressed.

Basically the problem is that attributes and inserts happen in the DOM in opposite order than what we compile for SSR. Most of the time this doesn't matter because order doesn't matter for most things. Except for HTML templates. We lazy evaluate everything and if used in a single location it also doesn't matter. It also doesn't matter if used if inserted in multiple places even. However, if an attribute reads a prop that contains HTML and it is also inserted we get this mismatch. To solve this might require changing compilation in a fairly significant way. I have some ideas but will have to test them.

In the meanwhile, the easiest way to work around this is using the in operator because it will only read it once.

function Button(props: { iconRight?: JSX.Element }) {
  return (
    <button classList={{ 'has-icon': "iconRight" in props }}>
      My button {props.iconRight}
    </button>
  );
}

The other way is to wrap the prop with children helper

function Button(props: { iconRight?: JSX.Element }) {
  const icon = children(() => props.iconRight)
  return (
    <button classList={{ 'has-icon': !!icon() }}>
      My button {icon()}
    </button>
  );
}
ryansolid commented 1 year ago

I should add that given the way Solid works you don't really want to be accessing this multiple times anyway as it is rendering twice. So while I don't like the hydration error, in general, this is a pattern that should be avoided. This is strongly putting me in the thinking this is more of a 2.0 change up than worth me sort of hacking around to make this work right now. I could brute force it at the cost of performance everywhere (adding extra delayed wrappers), but the mismatch is more fundamental. Like why do we need a children helper here in the first place? That's the level I'm thinking I'm at.

hansoksendahl commented 1 year ago

@ryansolid I have run into this error several times while building a design system based on Kobalte.

So is using the in operator the preferred way to conditionally render a component based on the presence of a prop in Solid.js at the moment?

howagain commented 1 year ago

This has been a tricky bug to track down. I believe this is causing some race condition with hydration between Solid and Astro and the IDE gives no warnings.

Because it's intermittent, it's very hard to debug and there's not a lot of guidance on this.

Right now the tutorial for the children tutorial doesn't make it clear that there's a problem associated with using JSX components inside of props, instead it's offered as a performance boost only.

Maybe in the meantime, until 2.0 could address this pattern of using JSX in props, we could give a linting warning about this pattern to encourage the use of the children helper.

For future search users, I see the error as Uncaught (in promise) TypeError: Cannot read properties of null (reading 'nextSibling') or Uncaught (in promise) TypeError: Cannot read properties of null (reading 'firstChild') and usually when I have throttling on or I revisit a no-cache html page with a cached JS file. Still don't understand it fully, so this might not be the cause, but this JSX prop pattern without the check seems related as those are the components where the tree traversal for hydration fails.

I believe that the issue is because of the parent component with some dynamic children doesn't know about its children creating a null child as a <!--#--> marker that is later resolved. So the hydration script of getNextMarker sees the comment as the firstChild and thinks there are no children but the hydration script is looking for the dynamic child components to hydrate because that's what the SSR script rendered on the server. Resulting in a mismatch and breaking of hydration because the tree traversal didn't match. I probably have some details wrong but think the general idea is correct. But the thing I'm stuck on is I don't know why it is a race condition and only happens sometimes.

Screen Shot 2023-08-18 at 8 16 04 AM

Edit: After more investigation this might actually be because of a race condition inside Astro, where the closing <!--/-->tag does not get streamed down before the hydration script is executed. It's a very confusing bug.