sveltejs / svelte

web development for the rest of us
https://svelte.dev
MIT License
79.79k stars 4.24k forks source link

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

Closed pngwn closed 5 years ago

pngwn commented 5 years ago

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 commented 5 years ago

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 commented 5 years ago

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 commented 5 years ago

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 commented 5 years ago

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 commented 5 years ago

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.

Conduitry commented 5 years ago

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.