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

Nodes appear out of order on hydration with 3.38.1 #6279

Closed
intrikate opened this issue May 2, 2021 · 5 comments
Closed

Nodes appear out of order on hydration with 3.38.1 #6279

intrikate opened this issue May 2, 2021 · 5 comments
Labels

Comments

@intrikate
Copy link

Describe the bug
On hydration, nodes containing {braced expressions} have their children rendered out of order.

Note that it’s reproducible with 3.38.1.

To Reproduce
The quickest way, probably: https://github.com/intrikate/svelte-hydration-order-issue.

Otherwise, putting an expression and a span under a common parent seems to reliably trigger this behaviour. Something like <p>{1} 2 <span>3</span></p> will hydrate into ‘13 2 ’.

Expected behavior
The order of nodes should be preserved in hydration.

Information about your Svelte project:

envinfo System: OS: macOS 11.2.3 CPU: (4) x64 Intel(R) Core(TM) i5-4288U CPU @ 2.60GHz Memory: 64.55 MB / 8.00 GB Shell: 5.8 - /bin/zsh Binaries: Node: 14.16.0 - /usr/local/bin/node Yarn: 1.22.10 - /usr/local/bin/yarn npm: 6.14.11 - /usr/local/bin/npm Browsers: Chrome: 90.0.4430.93 Firefox Developer Edition: 89.0 Safari: 14.0.3 npmPackages: svelte: ^3.38.1 => 3.38.1

Severity
High.

Additional context
Like #6274, this behaviour is not present in 3.37.0, but, sadly, remains in 3.38.1.

@Litarvan
Copy link

Litarvan commented May 2, 2021

Having probably the same issue here, one braced expression is also duplicated on one of my components. Had to revert on 3.37.

@benmccann benmccann changed the title Nodes appear out of order on hydration since 3.38.0 Nodes appear out of order on hydration with 3.38.1 May 2, 2021
@jonatansberg
Copy link

I think I've figured out the cause of this.

The following snippet:

<p>{1} 2 <span>3</span></p>

Complies to:

// ... snip
		c() {
			p = element("p");
			t0 = text(t0_value);
			t1 = text(" 2 ");
			span = element("span");
			t2 = text("3");
		},
		l(nodes) {
			p = claim_element(nodes, "P", {});
			var p_nodes = children(p);
			t0 = claim_text(p_nodes, t0_value);
			t1 = claim_text(p_nodes, " 2 ");
			span = claim_element(p_nodes, "SPAN", {});
			var span_nodes = children(span);
			t2 = claim_text(span_nodes, "3");
			span_nodes.forEach(detach);
			p_nodes.forEach(detach);
		},
		m(target, anchor) {
			insert(target, p, anchor);
			append(p, t0);
			append(p, t1);
			append(p, span);
			append(span, t2);
		}
// ... snip

The rendered html results in the following DOM tree pre hydration:

P
  #text "1 2 "
  SPAN
    #text "3"

But Svelte expects:

P (p)
  #text "1" (t0)
  #text " 2 " (t1)
  SPAN (span)
    #text "3" (t3)

The first claim_text call in the component code will claim the first text node. Thus, the second claim_text call will fail as the first one has claimed all the text.

This means the call "falls through" and that a new text node (t1) is created. When that node is appended In the mount step (m in the component code above) it's inserted at the end rather than in the middle since the other append calls are no-ops.

Turning those append calls into inserts, or being smarter about text node hydration seems like possible ways to fix this.

@benmccann
Copy link
Member

benmccann commented May 3, 2021

Would turning it into an insert work if there's more text nodes?

<p>{1} 2 {3} 4<span>5</span></p>

Having claim_text handle text nodes that are next to each other as a single merged node might be difficult the way the code is structured? In the short-term, maybe we can just destroy and recreate the text nodes which would lose some of the performance benefit, but keep most of it since we're avoiding that with the other DOM nodes

@nitroboy720
Copy link

I think this is safe to close now since the change that caused this bug was reversed

@timeshift92

This comment has been minimized.

hbirler added a commit to hbirler/svelte that referenced this issue Jun 9, 2021
Unreverts the hydration optimisation from sveltejs#6204 and fixes the `claim_text` issue as described in sveltejs#6279 (comment)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

6 participants