improve whitespace handling

Author: tanhauhau

src/compiler/compile/nodes/Text.ts

@@ -60,4 +60,11 @@ export default class Text extends Node {
 
 		return false;
 	}
+
+	use_space(): boolean {
+		if (this.component.compile_options.preserveWhitespace) return false;
+		if (/[\S\u00A0]/.test(this.data)) return false;
+
+		return !this.within_pre();
+	}
 }

src/compiler/compile/render_dom/wrappers/Element/index.ts

@@ -1118,7 +1118,11 @@ function to_html(wrappers: Array<ElementWrapper | TextWrapper | MustacheTagWrapp
 			// Don't add the <pre>/<textare> newline logic here because pre/textarea.innerHTML
 			// would keep the leading newline, too, only someParent.innerHTML = '..<pre/textarea>..' won't
 
-			if ((wrapper as TextWrapper).use_space()) state.quasi.value.raw += ' ';
+			if (wrapper.use_space()) {
+				// use space instead of the text content
+				state.quasi.value.raw += ' ';
+				return;
+			}
 
 			const parent = wrapper.node.parent as Element;
 

src/compiler/compile/render_dom/wrappers/Text.ts

@@ -7,7 +7,7 @@ import { Identifier } from 'estree';
 
 export default class TextWrapper extends Wrapper {
 	node: Text;
-	data: string;
+	_data: string;
 	skip: boolean;
 	var: Identifier;
 
@@ -21,15 +21,22 @@ export default class TextWrapper extends Wrapper {
 		super(renderer, block, parent, node);
 
 		this.skip = this.node.should_skip();
-		this.data = data;
+		this._data = data;
 		this.var = (this.skip ? null : x`t`) as unknown as Identifier;
 	}
 
 	use_space() {
-		if (this.renderer.component.component_options.preserveWhitespace) return false;
-		if (/[\S\u00A0]/.test(this.data)) return false;
+		return this.node.use_space();
+	}
 
-		return !this.node.within_pre();
+	set data(value: string) {
+		// when updating `this.data` during optimisation
+		// propagate the changes over to the underlying node
+		// so that the node.use_space reflects on the latest `data` value
+		this.node.data = this._data = value;
+	}
+	get data() {
+		return this._data;
 	}
 
 	render(block: Block, parent_node: Identifier, parent_nodes: Identifier) {

src/compiler/compile/render_ssr/handlers/Text.ts

@@ -5,7 +5,9 @@ import Element from '../../nodes/Element';
 
 export default function(node: Text, renderer: Renderer, _options: RenderOptions) {
 	let text = node.data;
-	if (
+	if (node.use_space()) {
+		text = ' ';
+	} else if (
 		!node.parent ||
 		node.parent.type !== 'Element' ||
 		((node.parent as Element).name !== 'script' && (node.parent as Element).name !== 'style')

test/helpers.ts

@@ -4,7 +4,7 @@ import glob from 'tiny-glob/sync';
 import * as path from 'path';
 import * as fs from 'fs';
 import * as colors from 'kleur';
-export const assert = (assert$1 as unknown) as typeof assert$1 & { htmlEqual: (actual, expected, message?) => void, htmlEqualWithComments: (actual, expected, message?) => void };
+export const assert = (assert$1 as unknown) as typeof assert$1 & { htmlEqual: (actual, expected, message?) => void, htmlEqualWithOptions: (actual, expected, options, message?) => void };
 
 // for coverage purposes, we need to test source files,
 // but for sanity purposes, we need to test dist files
@@ -140,7 +140,7 @@ function cleanChildren(node) {
 	}
 }
 
-export function normalizeHtml(window, html, { removeDataSvelte = false, preserveComments = false }: { removeDataSvelte?: boolean, preserveComments?: boolean} = {}) {
+export function normalizeHtml(window, html, { removeDataSvelte = false, preserveComments = false }: { removeDataSvelte?: boolean, preserveComments?: boolean }) {
 	try {
 		const node = window.document.createElement('div');
 		node.innerHTML = html
@@ -155,7 +155,12 @@ export function normalizeHtml(window, html, { removeDataSvelte = false, preserve
 	}
 }
 
-export function setupHtmlEqual(options?: { removeDataSvelte?: boolean }) {
+export function normalizeNewline(html: string) {
+	// return html.trim().replace(/\r\n/g, '\n')
+	return html.replace(/\r\n/g, '\n');
+}
+
+export function setupHtmlEqual(options: { removeDataSvelte?: boolean } = {}) {
 	const window = env();
 
 	// eslint-disable-next-line no-import-assign
@@ -167,10 +172,14 @@ export function setupHtmlEqual(options?: { removeDataSvelte?: boolean }) {
 		);
 	};
 	// eslint-disable-next-line no-import-assign
-	assert.htmlEqualWithComments = (actual, expected, message) => {
+	assert.htmlEqualWithOptions = (actual, expected, { preserveComments, withoutNormalizeHtml }, message) => {
 		assert.deepEqual(
-			normalizeHtml(window, actual, { ...options, preserveComments: true }),
-			normalizeHtml(window, expected, { ...options, preserveComments: true }),
+			withoutNormalizeHtml
+				? normalizeNewline(actual).replace(/(\sdata-svelte="[^"]+")/g, options.removeDataSvelte ? '' : '$1')
+				: normalizeHtml(window, actual, { ...options, preserveComments }),
+			withoutNormalizeHtml
+				? normalizeNewline(expected).replace(/(\sdata-svelte="[^"]+")/g, options.removeDataSvelte ? '' : '$1')
+				: normalizeHtml(window, expected, { ...options, preserveComments }),
 			message
 		);
 	};

test/js/samples/debug-ssr-foo/expected.js

@@ -8,11 +8,8 @@ const Component = create_ssr_component(($$result, $$props, $$bindings, slots) =>
 	if ($$props.foo === void 0 && $$bindings.foo && foo !== void 0) $$bindings.foo(foo);
 
 	return `${each(things, thing => {
-		return `<span>${escape(thing.name)}</span>
-	${debug(null, 7, 2, { foo })}`;
-	})}
-
-<p>foo: ${escape(foo)}</p>`;
+		return `<span>${escape(thing.name)}</span> ${debug(null, 7, 2, { foo })}`;
+	})} <p>foo: ${escape(foo)}</p>`;
 });
 
-export default Component;
+export default Component;

test/js/samples/ssr-preserve-comments/expected.js

@@ -2,9 +2,7 @@
 import { create_ssr_component } from "svelte/internal";
 
 const Component = create_ssr_component(($$result, $$props, $$bindings, slots) => {
-	return `<div>content</div>
-<!-- comment -->
-<div>more content</div>`;
+	return `<div>content</div> <!-- comment --> <div>more content</div>`;
 });
 
-export default Component;
+export default Component;

test/runtime/index.ts

@@ -201,7 +201,9 @@ describe('runtime', () => {
 					}
 
 					if (config.html) {
-						assert.htmlEqual(target.innerHTML, config.html);
+						assert.htmlEqualWithOptions(target.innerHTML, config.html, {
+							withoutNormalizeHtml: config.withoutNormalizeHtml
+						});
 					}
 
 					if (config.test) {

test/runtime/samples/pre-tag/_config.js

@@ -1,55 +1,37 @@
 export default {
-	test({ assert, target }) {
-		// Test for <pre> tag
-		const elementPre = target.querySelector('#pre');
-		// Test for non <pre> tag
-		const elementDiv = target.querySelector('#div');
-		// Test for <pre> tag in non <pre> tag
-		const elementDivWithPre = target.querySelector('#div-with-pre');
-		// Test for <pre> tag with leading newline
-		const elementPreWithLeadingNewline = target.querySelector('#pre-with-leading-newline');
-		const elementPreWithoutLeadingNewline = target.querySelector('#pre-without-leading-newline');
-		const elementPreWithMultipleLeadingNewline = target.querySelector('#pre-with-multiple-leading-newlines');
+	withoutNormalizeHtml: true,
+	html: get_html(false),
+	ssrHtml: get_html(true)
+};
 
-		assert.equal(
-			elementPre.innerHTML,
-			`  A
+function get_html(ssr) {
+	// ssr rendered HTML has an extra newline prefixed within `<pre>` tag,
+	// if the <pre> tag starts with `\n`
+	// because when browser parses the SSR rendered HTML, it will ignore the 1st '\n' character
+	return `<pre id="pre">  A
   B
   <span>
     C
     D
   </span>
   E
   F
-`
-		);
-		assert.equal(
-			elementDiv.innerHTML,
-			`A
+</pre> <div id="div">A
   B
   <span>C
     D</span>
   E
-  F`
-		);
-		assert.equal(
-			elementDivWithPre.innerHTML,
-			`<pre>    A
+  F</div> <div id="div-with-pre"><pre>    A
     B
     <span>
       C
       D
     </span>
     E
     F
-  </pre>`
-		);
-		assert.equal(elementPreWithLeadingNewline.children[0].innerHTML, 'leading newline');
-		assert.equal(elementPreWithLeadingNewline.children[1].innerHTML, '  leading newline and spaces');
-		assert.equal(elementPreWithLeadingNewline.children[2].innerHTML, '\nleading newlines');
-		assert.equal(elementPreWithoutLeadingNewline.children[0].innerHTML, 'without spaces');
-		assert.equal(elementPreWithoutLeadingNewline.children[1].innerHTML, '  with spaces  ');
-		assert.equal(elementPreWithoutLeadingNewline.children[2].innerHTML, ' \nnewline after leading space');
-		assert.equal(elementPreWithMultipleLeadingNewline.innerHTML, '\n\nmultiple leading newlines');
-	}
-};
+  </pre></div> <div id="pre-with-leading-newline"><pre>leading newline</pre> <pre>  leading newline and spaces</pre> <pre>${ssr ? '\n' : ''}
+leading newlines</pre></div> <div id="pre-without-leading-newline"><pre>without spaces</pre> <pre>  with spaces  </pre> <pre> 
+newline after leading space</pre></div> <pre id="pre-with-multiple-leading-newlines">${ssr ? '\n' : ''}
+
+multiple leading newlines</pre>`;
+}

test/runtime/samples/preserve-whitespaces/_config.js

@@ -1,30 +1,19 @@
 export default {
 	compileOptions: {
 		preserveWhitespace: true
-	},
-	test({ assert, target }) {
-		// Test for <pre> tag
-		const elementPre = target.querySelector('#pre');
-		// Test for non <pre> tag
-		const elementDiv = target.querySelector('#div');
-		// Test for <pre> tag in non <pre> tag
-		const elementDivWithPre = target.querySelector('#div-with-pre');
+  },
 
-		assert.equal(
-			elementPre.innerHTML,
-			`  A
+  html: `<pre id="pre">  A
   B
   <span>
     C
     D
   </span>
   E
   F
-`
-		);
-		assert.equal(
-			elementDiv.innerHTML,
-			`
+</pre>
+
+<div id="div">
   A
   B
   <span>
@@ -33,11 +22,9 @@ export default {
   </span>
   E
   F
-`
-		);
-		assert.equal(
-			elementDivWithPre.innerHTML,
-			`
+</div>
+
+<div id="div-with-pre">
   <pre>    A
     B
     <span>
@@ -47,7 +34,5 @@ export default {
     E
     F
   </pre>
-`
-		);
-	}
+</div>`
 };