Skip to content

Commit

Permalink
improve whitespace handling
Browse files Browse the repository at this point in the history
  • Loading branch information
tanhauhau committed Apr 15, 2022
1 parent fbfe910 commit 5466925
Show file tree
Hide file tree
Showing 23 changed files with 102 additions and 330 deletions.
7 changes: 7 additions & 0 deletions src/compiler/compile/nodes/Text.ts
Expand Up @@ -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();
}
}
6 changes: 5 additions & 1 deletion src/compiler/compile/render_dom/wrappers/Element/index.ts
Expand Up @@ -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;

Expand Down
17 changes: 12 additions & 5 deletions src/compiler/compile/render_dom/wrappers/Text.ts
Expand Up @@ -7,7 +7,7 @@ import { Identifier } from 'estree';

export default class TextWrapper extends Wrapper {
node: Text;
data: string;
_data: string;
skip: boolean;
var: Identifier;

Expand All @@ -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) {
Expand Down
4 changes: 3 additions & 1 deletion src/compiler/compile/render_ssr/handlers/Text.ts
Expand Up @@ -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')
Expand Down
21 changes: 15 additions & 6 deletions test/helpers.ts
Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand All @@ -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
Expand All @@ -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
);
};
Expand Down
9 changes: 3 additions & 6 deletions test/js/samples/debug-ssr-foo/expected.js
Expand Up @@ -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;
6 changes: 2 additions & 4 deletions test/js/samples/ssr-preserve-comments/expected.js
Expand Up @@ -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;
4 changes: 3 additions & 1 deletion test/runtime/index.ts
Expand Up @@ -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) {
Expand Down
52 changes: 17 additions & 35 deletions 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>`;
}
33 changes: 9 additions & 24 deletions 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>
Expand All @@ -33,11 +22,9 @@ export default {
</span>
E
F
`
);
assert.equal(
elementDivWithPre.innerHTML,
`
</div>
<div id="div-with-pre">
<pre> A
B
<span>
Expand All @@ -47,7 +34,5 @@ export default {
E
F
</pre>
`
);
}
</div>`
};
17 changes: 17 additions & 0 deletions test/runtime/samples/textarea-content/_config.js
@@ -1,4 +1,21 @@
export default {
withoutNormalizeHtml: true,
// Unable to test `html` with `<textarea>` content
// as the textarea#value will not show within `innerHtml`
ssrHtml: `<textarea id="textarea"> A
B
</textarea> <div id="div-with-textarea"><textarea> A
B
</textarea></div> <div id="textarea-with-leading-newline"><textarea>leading newline</textarea> <textarea> leading newline and spaces</textarea> <textarea>
leading newlines</textarea></div> <div id="textarea-without-leading-newline"><textarea>without spaces</textarea> <textarea> with spaces </textarea> <textarea>
newline after leading space</textarea></div> <textarea id="textarea-with-multiple-leading-newlines">
multiple leading newlines</textarea> <div id="div-with-textarea-with-multiple-leading-newlines"><textarea>
multiple leading newlines</textarea></div>`,
test({ assert, target }) {
// Test for <textarea> tag
const elementTextarea = target.querySelector('#textarea');
Expand Down
18 changes: 9 additions & 9 deletions test/server-side-rendering/index.ts
Expand Up @@ -83,13 +83,7 @@ describe('ssr', () => {
if (css.code) fs.writeFileSync(`${dir}/_actual.css`, css.code);

try {
if (config.withoutNormalizeHtml) {
assert.strictEqual(html.trim().replace(/\r\n/g, '\n'), expectedHtml.trim().replace(/\r\n/g, '\n'));
} else {
(compileOptions.preserveComments
? assert.htmlEqualWithComments
: assert.htmlEqual)(html, expectedHtml);
}
assert.htmlEqualWithOptions(html, expectedHtml, { preserveComments: compileOptions.preserveComments, withoutNormalizeHtml: config.withoutNormalizeHtml });
} catch (error) {
if (shouldUpdateExpected()) {
fs.writeFileSync(`${dir}/_expected.html`, html);
Expand Down Expand Up @@ -212,9 +206,15 @@ describe('ssr', () => {
});

if (config.ssrHtml) {
assert.htmlEqual(html, config.ssrHtml);
assert.htmlEqualWithOptions(html, config.ssrHtml, {
preserveComments: compileOptions.preserveComments,
withoutNormalizeHtml: config.withoutNormalizeHtml
});
} else if (config.html) {
assert.htmlEqual(html, config.html);
assert.htmlEqualWithOptions(html, config.html, {
preserveComments: compileOptions.preserveComments,
withoutNormalizeHtml: config.withoutNormalizeHtml
});
}

if (config.test_ssr) {
Expand Down
2 changes: 0 additions & 2 deletions test/server-side-rendering/samples/pre-tag/.editorconfig

This file was deleted.

3 changes: 0 additions & 3 deletions test/server-side-rendering/samples/pre-tag/_config.js

This file was deleted.

37 changes: 0 additions & 37 deletions test/server-side-rendering/samples/pre-tag/_expected.html

This file was deleted.

0 comments on commit 5466925

Please sign in to comment.