Skip to content

When SSR output starts with a comment, hydration fails #12532

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

Closed
buhrmi opened this issue Jul 22, 2024 · 12 comments
Closed

When SSR output starts with a comment, hydration fails #12532

buhrmi opened this issue Jul 22, 2024 · 12 comments
Assignees
Labels
Milestone

Comments

@buhrmi
Copy link
Contributor

buhrmi commented Jul 22, 2024

Describe the bug

Since next.179, when the SSR output starts with a comment (for example because the root block is an if-block, resulting in <!--[--> as the first node in the SSR output), hydration fails. Does not happen in next.178

Reproduction

Here is a minimal reproduction in the REPL: https://svelte-5-preview.vercel.app/#H4sIAAAAAAAAA31Q0WrDMAz8Fc17aAxJ856lYWMfMGgem0JDonRmjWNsZaMY__vsuF0LhT0YofPpdDrLBnFCw4qdZbIdkRXsTSmWMjqr0JhvPBH63kyz7gJSmk4LRVUjGxKjmjSBhU5jS7htf2oplEJK4fPcaw-Bg0FPI6yi0Orlbux9GtXld52HZn1PCq-bpCEY2i-s6y1sHtYkCYdNBYldBkij7FEXENFDBBsqn7Jsl2VV0PiYSc0LsvdIZBzSWA3SrBJq9RGJg72OXy5JgsPU3xoJ4HgkuFAc5950md-ykfY12rnaT7jzMY5TLwaBPStIz-jSv9SD-n-x22cxwDLTSLhd4vfkYvDYo_be_QLjIk6A3QEAAA==

I've done some digging, and it seems that in next.178 the append is called, which is no-op during hydration. In next.179, the child function is called, which tries to append a node to the comment node.

next.178:
image

next.179:
image

Logs

No response

System Info

N/A

Severity

blocking an upgrade

@trueadm trueadm added the bug label Jul 22, 2024
@trueadm trueadm added this to the 5.0 milestone Jul 22, 2024
@trueadm
Copy link
Contributor

trueadm commented Jul 22, 2024

Raw snippets are meant to only return a single element. Do you have a repro that doesn't use a raw snippet?

@trueadm trueadm self-assigned this Jul 22, 2024
@buhrmi
Copy link
Contributor Author

buhrmi commented Jul 22, 2024

https://github.com/buhrmi/ssr-starts-with-comment (Stackblitz)

In this repo I have "faked" SSR by hardcoding the output in the index.html.

This repo gives a different error message in the console though. Not sure why.

@trueadm
Copy link
Contributor

trueadm commented Jul 24, 2024

I updated to the latest version and the issue seems to have been fixed. :)

@trueadm trueadm closed this as completed Jul 24, 2024
@buhrmi
Copy link
Contributor Author

buhrmi commented Jul 24, 2024

eh? I upgraded the repo to next.198 but it's still failing.

image

@trueadm
Copy link
Contributor

trueadm commented Jul 24, 2024

I used pnpm rather than npm and upgrading to the latest version fixed the issue for me. I wonder if this is some other issue? I'll check it out with npm tomorrow as it's clear something still isn't right :)

@trueadm trueadm reopened this Jul 24, 2024
@buhrmi
Copy link
Contributor Author

buhrmi commented Jul 24, 2024

Okay so when I open the repo in Stackblitz, and wait for a while to boot up, the preview window renders "Hello" for a second, and then hydration happens (and fails) and the "Hello" disappears.

Running pnpm install && pnpm run dev doesn't fix it for me.

@trueadm
Copy link
Contributor

trueadm commented Jul 25, 2024

The SSR string in the index.html here is wrong, it should be:

<div id="app"><!--[--><!--[-->Hello<!--]--><!--]--></div>

That's what I get when I render the app using render. Note there are two sets of anchor markers.

@buhrmi
Copy link
Contributor Author

buhrmi commented Jul 25, 2024

Ah, you're right. Okay. Let me re-check why it's not working on my local project. Sorry for the false bug report.

@buhrmi buhrmi closed this as completed Jul 25, 2024
@buhrmi
Copy link
Contributor Author

buhrmi commented Jul 25, 2024

Hmm, but even if so, shouldn't hydrate rather issue a dom mismatch warning instead of failing completely?

@trueadm
Copy link
Contributor

trueadm commented Jul 25, 2024

Hmm, but even if so, shouldn't hydrate rather issue a dom mismatch warning instead of failing completely?

Good point

#12604

@buhrmi
Copy link
Contributor Author

buhrmi commented Jul 25, 2024

When I disable HMR the error goes away. Haven't investigated why that is, but I can live with it for now. Not even sure anymore if it's a Svelte related bug or a me-related bug.

@buhrmi buhrmi closed this as completed Jul 25, 2024
@buhrmi
Copy link
Contributor Author

buhrmi commented Jul 25, 2024

#12604 doesn't catch the error by the way. hydrate_node is null here:

hydrate_node.nodeType !== 8 ||

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

No branches or pull requests

2 participants