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

Svelte adds whitespace to template literals when dev: false #2813

Closed
pngwn opened this issue May 18, 2019 · 6 comments
Closed

Svelte adds whitespace to template literals when dev: false #2813

pngwn opened this issue May 18, 2019 · 6 comments
Labels

Comments

@pngwn
Copy link
Member

pngwn commented May 18, 2019

When dev: false and Svelte generates template literals to set the innerHTML of an element, it seems to be adding whitespace. Specifically it seems to be aligning the template literal to the indentation level of its parent function in the generated code. If this is a nested function then one or more tabs will be added to the beginning of each line.

Example input:

<span class="hljs-function"><span class="hljs-keyword">function</span> <span class="hljs-title">thingy</span>(<span class="hljs-params
">str</span>) </span>{
 <span class="hljs-keyword">return</span> str.toUpperCase();
}

Output:

		return {
			c() {
				pre.innerHTML = `<code class="language-js"><span class="hljs-function"><span class="hljs-keyword">function</span> <span class="hljs-title">thingy</span>(<span class="hljs-params">str</span>) </span>{
			 <span class="hljs-keyword">return</span> str.toUpperCase();
			}
			</code>`;
			}
		}

This does not happen when dev: true.

For the case of code blocks (wrapped in pre tags) this is obviously more of an issue, in some cases it doesn't make much of a difference.

@pngwn pngwn changed the title Svelte is adds whitespace to template literals when dev: false Svelte adds whitespace to template literals when dev: false May 18, 2019
@Conduitry Conduitry added the bug label May 24, 2019
@pngwn
Copy link
Member Author

pngwn commented May 26, 2019

Further to the above, Svelte only generates the incorrect code in dev mode when hydratable is false due to the different code generated.

This REPL better illustrates the problem. The rendered code is fine but I copy/ pasted the example output and attached it manually added to illustrate the problem. If you look at the generated code (in the output tab) you can see that the indentation is intact (when it should be flattened).

@Conduitry
Copy link
Member

I was just pleasantly surprised to see that Svelte is apparently already correctly handling indentation of multi-line template strings in user code. So I guess it's just the code generated for the optimized static innerHTML, which is weird (maybe because it's not getting inserted via a replaceable snippet reference, which happens after deindent does its thing?), but I guess means this is less of a problem.

@Conduitry
Copy link
Member

I was sweeping through the open bugs for ones that might be fixed by the structured codegen PR, and I realized I'm really not sure at all what exactly the bug is here. I'm fiddling with the different REPLs here, and I can't actually see any behavior that looks wrong. Do you have a super simple example that shows this?

@Conduitry
Copy link
Member

Repro from #3653:

<p>
Hello
<br>
world
</p>

The template string that this optimizes to in prod mode contains extra whitespace on each line. It looks like this particular bug will be addressed by the codegen PR. @pngwn Is what you were seeing more than just this?

@pngwn
Copy link
Member Author

pngwn commented Oct 2, 2019

This bug is the extra indentation when Svelte sets the innerHTML you can only see it from the output, not the REPL itself. Normally it doesn't cause an issue but with template literals (i.e. for code snippets in pre tags) if obviously causes rendering errors.

In this REPL if you compare line 60-65 with line 23-27 you can see the difference. If this is what you mean by whitespace then, yeah, that is what I was seeing.

AlexxNB added a commit to AlexxNB/svelte-chota that referenced this issue Oct 14, 2019
@Conduitry
Copy link
Member

Fixed by #3539. I'm closing this without writing a test because writing a test sounds annoying to test for (because the HTML comparer in the tests ignores whitespace) and it seems unlikely this would resurface.

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