Skip to content

Commit

Permalink
[fix]: keep space in <pre> or when preserveWhitespace: true (svel…
Browse files Browse the repository at this point in the history
…tejs#6990)

Fixes sveltejs#6437
Fixes sveltejs#4731
Closes sveltejs#4737

Whitespace is now untouched inside <pre> tag and other tags if preserveWhitespace is true
  • Loading branch information
ota-meshi authored and himanshiLt committed Mar 3, 2022
1 parent 35c5f67 commit 78e85f2
Show file tree
Hide file tree
Showing 15 changed files with 361 additions and 16 deletions.
17 changes: 17 additions & 0 deletions src/compiler/compile/nodes/Text.ts
Expand Up @@ -43,4 +43,21 @@ export default class Text extends Node {

return parent_element.namespace || elements_without_text.has(parent_element.name);
}

keep_space(): boolean {
if (this.component.component_options.preserveWhitespace) return true;
return this.within_pre();
}

within_pre(): boolean {
let node = this.parent;
while (node) {
if (node.type === 'Element' && node.name === 'pre') {
return true;
}
node = node.parent;
}

return false;
}
}
4 changes: 2 additions & 2 deletions src/compiler/compile/render_dom/wrappers/Fragment.ts
Expand Up @@ -95,7 +95,7 @@ export default class FragmentWrapper {
next_sibling ? (next_sibling.node.type === 'Text' && /^\s/.test(next_sibling.node.data) && trimmable_at(child, next_sibling)) : !child.has_ancestor('EachBlock')
);

if (should_trim) {
if (should_trim && !child.keep_space()) {
data = trim_end(data);
if (!data) continue;
}
Expand Down Expand Up @@ -127,7 +127,7 @@ export default class FragmentWrapper {
if (strip_whitespace) {
const first = this.nodes[0] as Text;

if (first && first.node.type === 'Text') {
if (first && first.node.type === 'Text' && !first.node.keep_space()) {
first.data = trim_start(first.data);
if (!first.data) {
first.var = null;
Expand Down
10 changes: 1 addition & 9 deletions src/compiler/compile/render_dom/wrappers/Text.ts
Expand Up @@ -29,15 +29,7 @@ export default class TextWrapper extends Wrapper {
if (this.renderer.component.component_options.preserveWhitespace) return false;
if (/[\S\u00A0]/.test(this.data)) return false;

let node = this.parent && this.parent.node;
while (node) {
if (node.type === 'Element' && node.name === 'pre') {
return false;
}
node = node.parent;
}

return true;
return !this.node.within_pre();
}

render(block: Block, parent_node: Identifier, parent_nodes: Identifier) {
Expand Down
Expand Up @@ -26,7 +26,7 @@ export default function remove_whitespace_children(children: INode[], next?: INo
trimmable_at(child, next)
: !child.has_ancestor('EachBlock');

if (should_trim) {
if (should_trim && !child.keep_space()) {
data = trim_end(data);
if (!data) continue;
}
Expand All @@ -47,7 +47,7 @@ export default function remove_whitespace_children(children: INode[], next?: INo
}

const first = nodes[0];
if (first && first.type === 'Text') {
if (first && first.type === 'Text' && !first.keep_space()) {
first.data = trim_start(first.data);
if (!first.data) {
first.var = null;
Expand Down
57 changes: 57 additions & 0 deletions test/runtime/samples/pre-tag/_config.js
@@ -0,0 +1,57 @@
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');

// There is a slight difference in innerHTML because there is a difference in HTML optimization (in jsdom)
// depending on how the innerHTML is set.
// (There is no difference in the display.)
// Reassign innerHTML to add the same optimizations to innerHTML.

// eslint-disable-next-line no-self-assign
elementPre.innerHTML = elementPre.innerHTML;
// eslint-disable-next-line no-self-assign
elementDiv.innerHTML = elementDiv.innerHTML;
// eslint-disable-next-line no-self-assign
elementDivWithPre.innerHTML = elementDivWithPre.innerHTML;

assert.equal(
elementPre.innerHTML,
`
A
B
<span>
C
D
</span>
E
F
`
);
assert.equal(
elementDiv.innerHTML,
`A
B
<span>C
D</span>
E
F`
);
assert.equal(
elementDivWithPre.innerHTML,
`<pre> A
B
<span>
C
D
</span>
E
F
</pre>`
);
}
};
34 changes: 34 additions & 0 deletions test/runtime/samples/pre-tag/main.svelte
@@ -0,0 +1,34 @@
<pre id="pre">
A
B
<span>
C
D
</span>
E
F
</pre>

<div id="div">
A
B
<span>
C
D
</span>
E
F
</div>

<div id="div-with-pre">
<pre>
A
B
<span>
C
D
</span>
E
F
</pre>
</div>
66 changes: 66 additions & 0 deletions test/runtime/samples/preserve-whitespaces/_config.js
@@ -0,0 +1,66 @@
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');

// There is a slight difference in innerHTML because there is a difference in HTML optimization (in jsdom)
// depending on how the innerHTML is set.
// (There is no difference in the display.)
// Reassign innerHTML to add the same optimizations to innerHTML.

// eslint-disable-next-line no-self-assign
elementPre.innerHTML = elementPre.innerHTML;
// eslint-disable-next-line no-self-assign
elementDiv.innerHTML = elementDiv.innerHTML;
// eslint-disable-next-line no-self-assign
elementDivWithPre.innerHTML = elementDivWithPre.innerHTML;

assert.equal(
elementPre.innerHTML,
`
A
B
<span>
C
D
</span>
E
F
`
);
assert.equal(
elementDiv.innerHTML,
`
A
B
<span>
C
D
</span>
E
F
`
);
assert.equal(
elementDivWithPre.innerHTML,
`
<pre> A
B
<span>
C
D
</span>
E
F
</pre>
`
);
}
};
34 changes: 34 additions & 0 deletions test/runtime/samples/preserve-whitespaces/main.svelte
@@ -0,0 +1,34 @@
<pre id="pre">
A
B
<span>
C
D
</span>
E
F
</pre>

<div id="div">
A
B
<span>
C
D
</span>
E
F
</div>

<div id="div-with-pre">
<pre>
A
B
<span>
C
D
</span>
E
F
</pre>
</div>
10 changes: 7 additions & 3 deletions test/server-side-rendering/index.ts
Expand Up @@ -83,9 +83,13 @@ describe('ssr', () => {
if (css.code) fs.writeFileSync(`${dir}/_actual.css`, css.code);

try {
(compileOptions.preserveComments
? assert.htmlEqualWithComments
: assert.htmlEqual)(html, expectedHtml);
if (config.withoutNormalizeHtml) {
assert.strictEqual(html.trim(), expectedHtml.trim().replace(/\r\n/g, '\n'));
} else {
(compileOptions.preserveComments
? assert.htmlEqualWithComments
: assert.htmlEqual)(html, expectedHtml);
}
} catch (error) {
if (shouldUpdateExpected()) {
fs.writeFileSync(`${dir}/_expected.html`, html);
Expand Down
3 changes: 3 additions & 0 deletions test/server-side-rendering/samples/pre-tag/_config.js
@@ -0,0 +1,3 @@
export default {
withoutNormalizeHtml: true
};
30 changes: 30 additions & 0 deletions test/server-side-rendering/samples/pre-tag/_expected.html
@@ -0,0 +1,30 @@
<pre>
A
B
<span>
C
D
</span>
E
F
</pre>

<div>A
B
<span>C
D
</span>
E
F
</div>

<div><pre>
A
B
<span>
C
D
</span>
E
F
</pre></div>
34 changes: 34 additions & 0 deletions test/server-side-rendering/samples/pre-tag/main.svelte
@@ -0,0 +1,34 @@
<pre>
A
B
<span>
C
D
</span>
E
F
</pre>

<div>
A
B
<span>
C
D
</span>
E
F
</div>

<div>
<pre>
A
B
<span>
C
D
</span>
E
F
</pre>
</div>
@@ -0,0 +1,6 @@
export default {
withoutNormalizeHtml: true,
compileOptions: {
preserveWhitespace: true
}
};
@@ -0,0 +1,34 @@
<pre>
A
B
<span>
C
D
</span>
E
F
</pre>

<div>
A
B
<span>
C
D
</span>
E
F
</div>

<div>
<pre>
A
B
<span>
C
D
</span>
E
F
</pre>
</div>

0 comments on commit 78e85f2

Please sign in to comment.