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

#3278 causes hydration bugs #3283

Open
zakstucke opened this issue Nov 23, 2024 · 7 comments
Open

#3278 causes hydration bugs #3283

zakstucke opened this issue Nov 23, 2024 · 7 comments

Comments

@zakstucke
Copy link
Contributor

Describe the bug
Sorry I haven't got a repro, but I've realised rc2, at least for me, has a new hydration bug caused by this commit:
8b258b0

Hopefully a look at the changes in that commit might instantly show you what's happened

ERROR: 
      /leptos/tachys/src/html/element/mod.rs:352:14
      Panic: called `Option::unwrap()` on a `None` value
@gbj
Copy link
Collaborator

gbj commented Nov 23, 2024

I'm unsurprised that this change caused an issue, but can't take any meaningful action without a reproduction, unfortunately.

@gbj
Copy link
Collaborator

gbj commented Nov 23, 2024

In case it's helpful in narrowing things down to reproduce it, here's a description of what that change means.

Previously, reading from a resource would only trigger Suspense if the resource's value was None.

However, this meant that a resource that was re-loading would not trigger Suspense to fall back.

The change means that reading from a resource always adds the resource to that Suspense's set of tasks, and spawns a task to wait for the resource to be ready, and then removes it from the set of tasks.

This does not cause to seem a hydration issue in any of the examples in the repo. (I have manually tested some but not all of them, and the automated tests on those that have them didn't find anything.)

Note also that 3de0414 should add a more helpful error message for the hydration error you pasted above, which may help narrow it down.

@zakstucke
Copy link
Contributor Author

@gbj might have a PR to solve this, still blind to the simple repro though.

@winteler
Copy link

winteler commented Nov 24, 2024

Hey @gbj,

Just wanted to mention that I also found an hydration bug with nested <Transition/>. Interestingly, I can reproduce this error with rc1 but it also seems to be fixed when using the PR (rev = "adf22c9d983d49ecbddd59acd9c41c6422678406").

I'll just give more details in case it can help understanding the issue or if someone with a similar case comes here. Here is the error I got:

A hydration error occurred while trying to hydrate an element defined at app/src/icons.rs:214:5.

The framework expected an HTML <div> element, but found this instead:  
<!--  -->

In this case, app/src/icons.rs:214:5 is my loading icon used in the Transition fallback and the error appeared only when the content of the second <Transition/> was empty, e.g. a <Show> with when = false. To confirm that the issue is indeed related to the fallback, I first removed it from the <Transition/> and then put the same fallback in the <Transition> and <Show>. In both cases, the hydration error disappeared.

In the process of trying (and failing '^^) to make a minimal reproduction, I also tried to move the component with the nested <Transition/> to other places (higher in the tree basically), and saw that the hydration error then happened inconsistently, indicating it's probably a race condition.

Please let me know if you want any other information :)

@Nutomic
Copy link

Nutomic commented Nov 25, 2024

I seem to be getting the same problem. Interestingly it only happens with <Transition>, but goes away if I replace occurences of <Transition> with <Suspense>. Maybe there is a relevant difference between these two?

Edit: Nevermind the crashes came back again. And it seems the erorr message points to completely random places.

@gbj
Copy link
Collaborator

gbj commented Nov 25, 2024

@Nutomic @winteler Thank for your comments but unfortunately there is no action that can be taken unless someone provides a reproducible example, at which point I'm very happy to try to fix it, and everyone can check whether it is indeed the same problem or several different problems.

@zakstucke
Copy link
Contributor Author

zakstucke commented Nov 30, 2024

@gbj I have a repro, it's very strange but this is it 😅 :

use leptos::prelude::*;

#[component]
pub fn Counters() -> impl IntoView {
    view! {
        <Transition>
            <p>Hello</p>
        </Transition>
        <Inner />
    }
}

#[component]
pub fn Inner() -> impl IntoView {
    let resource =
        Resource::new(|| (), |_| async move { vec!["foo".to_string()] });
    resource.get_untracked();
    view! { <div>World</div> }
}
counter_isomorphic.wasm:0x122712 A hydration error occurred while trying to hydrate an element defined at {unknown}.

The framework expected a marker node, but found this instead:  <p>​Hello​</p>​ 

The hydration mismatch may have occurred slightly earlier, but this is the first time the framework found a node of an unexpected type.

counter_isomorphic.js:407 panicked at /Users/zak/z/code/leptos/tachys/src/hydration.rs:181:9:
Unrecoverable hydration error. Please read the error message directly above this for more details.

So clearly this is probably user-misuse (although i'd like for this to work but understand if it shouldn't) that kind've makes sense is broken, but only actually caused any errors after #3278.

So it seems the real issue is the Resource being used outside suspense warning not working/showing, it looks like because the system thinks it's suspensed correctly due to the <Transition/> above it in the outer component.
If you remove the Transition component, the warning shows correctly.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants