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

Whitespace inside elements is not being preserved inside loops #713

Closed
TehShrike opened this issue Jul 18, 2017 · 6 comments · Fixed by #3059
Closed

Whitespace inside elements is not being preserved inside loops #713

TehShrike opened this issue Jul 18, 2017 · 6 comments · Fixed by #3059
Labels
Projects

Comments

@TehShrike
Copy link
Member

Very related to bug #608.

Reproduction: https://svelte.technology/repl?version=1.25.0&gist=10dd087fc24ff3dbf326c06b04e0611a

Rich-Harris added a commit that referenced this issue Jul 25, 2017
don't strip whitespace at the end of an each block
@Rich-Harris
Copy link
Member

thanks, fixed in 1.26

@TehShrike
Copy link
Member Author

So, weird thing - it doesn't look like this was actually fixed: https://svelte.technology/repl?version=1.34.0&gist=10dd087fc24ff3dbf326c06b04e0611a

I'm not sure how this is possible, since that test case in #718 seems to match it pretty exactly, but that repl still shows the issue even in 1.26.0.

@PaulBGD PaulBGD reopened this Aug 30, 2017
@Conduitry
Copy link
Member

Conduitry commented Sep 1, 2017

In that gist, if you take out the second paragraph about "^ Those letters should all have a space between them", then spaces do appear between the letters, which is kind of hilarious.

edit: Also, perhaps more helpful for debugging, if you make sure there's no whitespace between the paragraph the each block (so you have {{/each}}<p> or {{/each}}foo<p>), the spaces appear. This seems in line with what's intended, given the comment

 	// We want to remove trailing whitespace inside an element/component/block,
 	// *unless* there is no whitespace between this node and its next sibling

but I don't know what the goal of this rule is.

@Rich-Harris Rich-Harris added the bug label Sep 1, 2017
@arxpoetica arxpoetica added this to Triage in Roadmap Jan 10, 2018
@arxpoetica arxpoetica moved this from Triage to Immediate priorities in Roadmap Jan 10, 2018
@Conduitry
Copy link
Member

Been taking another look at this, and I'm still confused. I'm not sure what exactly the nextSibling is meant to be in relation to the current Node. In @TehShrike's REPL example above, it's the '\n\n' after the EachBlock. Since the nextSibling exists and is a Text node beginning with whitespace, we strip the whitespace at the end of the <span>. It seems like in this case for the shouldTrim to be correct, nextSibling would have to be null.

@Conduitry
Copy link
Member

v3 repro from #2905:

{#each ['dog', 'hen', 'fox', 'cat', 'pig', 'ant', 'cow', 'koi'] as value}
	<span>{value} </span>
{/each}
<!-- Type or insert anything here to break spacing of list. -->

@Conduitry
Copy link
Member

Fixed in 3.5.4 (finally!!)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
No open projects
Roadmap
  
Immediate priorities
Development

Successfully merging a pull request may close this issue.

4 participants