Skip to content

Commit

Permalink
remove whitespace in between elements, blocks and components
Browse files Browse the repository at this point in the history
  • Loading branch information
tanhauhau committed Feb 22, 2020
1 parent bce71d4 commit 89b7cf6
Show file tree
Hide file tree
Showing 6 changed files with 125 additions and 58 deletions.
29 changes: 29 additions & 0 deletions src/compiler/compile/nodes/Text.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,18 @@ import Component from '../Component';
import TemplateScope from './shared/TemplateScope';
import { INode } from './interfaces';

// Whitespace inside one of these elements will not result in
// a whitespace node being created in any circumstances. (This
// list is almost certainly very incomplete)
const elements_without_text = new Set([
'audio',
'datalist',
'dl',
'optgroup',
'select',
'video',
]);

export default class Text extends Node {
type: 'Text';
data: string;
Expand All @@ -13,4 +25,21 @@ export default class Text extends Node {
this.data = info.data;
this.synthetic = info.synthetic || false;
}

should_skip() {
if (/\S/.test(this.data)) return false;

const parent_element = this.find_nearest(/(?:Element|InlineComponent|Head)/);
if (!parent_element) return false;

if (parent_element.type === 'Head') return true;
if (parent_element.type === 'InlineComponent') return parent_element.children.length === 1 && this === parent_element.children[0];

// svg namespace exclusions
if (/svg$/.test(parent_element.namespace)) {
if (this.prev && this.prev.type === "Element" && this.prev.name === "tspan") return false;
}

return parent_element.namespace || elements_without_text.has(parent_element.name);
}
}
6 changes: 1 addition & 5 deletions src/compiler/compile/render_dom/wrappers/Fragment.ts
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ import { INode } from '../../nodes/interfaces';
import Renderer from '../Renderer';
import Block from '../Block';
import { trim_start, trim_end } from '../../../utils/trim';
import { link } from '../../../utils/link';
import { Identifier } from 'estree';

const wrappers = {
Expand All @@ -38,11 +39,6 @@ const wrappers = {
Window
};

function link(next: Wrapper, prev: Wrapper) {
prev.next = next;
if (next) next.prev = prev;
}

function trimmable_at(child: INode, next_sibling: Wrapper): boolean {
// Whitespace is trimmable if one of the following is true:
// The child and its sibling share a common nearest each block (not at an each block boundary)
Expand Down
32 changes: 1 addition & 31 deletions src/compiler/compile/render_dom/wrappers/Text.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,36 +5,6 @@ import Wrapper from './shared/Wrapper';
import { x } from 'code-red';
import { Identifier } from 'estree';

// Whitespace inside one of these elements will not result in
// a whitespace node being created in any circumstances. (This
// list is almost certainly very incomplete)
const elements_without_text = new Set([
'audio',
'datalist',
'dl',
'optgroup',
'select',
'video',
]);

// TODO this should probably be in Fragment
function should_skip(node: Text) {
if (/\S/.test(node.data)) return false;

const parent_element = node.find_nearest(/(?:Element|InlineComponent|Head)/);
if (!parent_element) return false;

if (parent_element.type === 'Head') return true;
if (parent_element.type === 'InlineComponent') return parent_element.children.length === 1 && node === parent_element.children[0];

// svg namespace exclusions
if (/svg$/.test(parent_element.namespace)) {
if (node.prev && node.prev.type === "Element" && node.prev.name === "tspan") return false;
}

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

export default class TextWrapper extends Wrapper {
node: Text;
data: string;
Expand All @@ -50,7 +20,7 @@ export default class TextWrapper extends Wrapper {
) {
super(renderer, block, parent, node);

this.skip = should_skip(this.node);
this.skip = this.node.should_skip();
this.data = data;
this.var = (this.skip ? null : x`t`) as unknown as Identifier;
}
Expand Down
82 changes: 79 additions & 3 deletions src/compiler/compile/render_ssr/handlers/Element.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,13 +4,19 @@ import { get_slot_scope } from './shared/get_slot_scope';
import { boolean_attributes } from './shared/boolean_attributes';
import Renderer, { RenderOptions } from '../Renderer';
import Element from '../../nodes/Element';
import { INode } from '../../nodes/interfaces';
import { x } from 'code-red';
import Expression from '../../nodes/shared/Expression';
import fix_attribute_casing from '../../render_dom/wrappers/Element/fix_attribute_casing';
import { trim_end, trim_start } from '../../../utils/trim';
import { link } from '../../../utils/link';

export default function(node: Element, renderer: Renderer, options: RenderOptions & {
slot_scopes: Map<any, any>;
}) {

const children = remove_whitespace_children(node.children, node.next);

// awkward special case
let node_contents;

Expand Down Expand Up @@ -134,7 +140,7 @@ export default function(node: Element, renderer: Renderer, options: RenderOption
if (node_contents !== undefined) {
if (contenteditable) {
renderer.push();
renderer.render(node.children, options);
renderer.render(children, options);
const result = renderer.pop();

renderer.add_expression(x`($$value => $$value === void 0 ? ${result} : $$value)(${node_contents})`);
Expand All @@ -146,7 +152,7 @@ export default function(node: Element, renderer: Renderer, options: RenderOption
renderer.add_string(`</${node.name}>`);
}
} else if (slot && nearest_inline_component) {
renderer.render(node.children, options);
renderer.render(children, options);

if (!is_void(node.name)) {
renderer.add_string(`</${node.name}>`);
Expand All @@ -164,10 +170,80 @@ export default function(node: Element, renderer: Renderer, options: RenderOption
output: renderer.pop()
});
} else {
renderer.render(node.children, options);
renderer.render(children, options);

if (!is_void(node.name)) {
renderer.add_string(`</${node.name}>`);
}
}
}

// similar logic from `compile/render_dom/wrappers/Fragment`
// We want to remove trailing whitespace inside an element/component/block,
// *unless* there is no whitespace between this node and its next sibling
function remove_whitespace_children(children: INode[], next?: INode): INode[] {
const nodes: INode[] = [];
let last_child: INode;
let i = children.length;
while (i--) {
const child = children[i];

if (child.type === 'Text') {
if (child.should_skip()) {
continue;
}

let { data } = child;

if (nodes.length === 0) {
const should_trim = next
? next.type === 'Text' &&
/^\s/.test(next.data) &&
trimmable_at(child, next)
: !child.has_ancestor('EachBlock');

if (should_trim) {
data = trim_end(data);
if (!data) continue;
}
}

// glue text nodes (which could e.g. be separated by comments) together
if (last_child && last_child.type === 'Text') {
last_child.data = data + last_child.data;
continue;
}

nodes.unshift(child);
link(last_child, last_child = child);
} else {
nodes.unshift(child);
link(last_child, last_child = child);
}
}

const first = nodes[0];
if (first && first.type === 'Text') {
first.data = trim_start(first.data);
if (!first.data) {
first.var = null;
nodes.shift();

if (nodes[0]) {
nodes[0].prev = null;
}
}
}

return nodes;
}

function trimmable_at(child: INode, next_sibling: INode): boolean {
// Whitespace is trimmable if one of the following is true:
// The child and its sibling share a common nearest each block (not at an each block boundary)
// The next sibling's previous node is an each block
return (
next_sibling.find_nearest(/EachBlock/) ===
child.find_nearest(/EachBlock/) || next_sibling.prev.type === 'EachBlock'
);
}
4 changes: 4 additions & 0 deletions src/compiler/utils/link.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
export function link<T extends { next?: T; prev?: T }>(next: T, prev: T) {
prev.next = next;
if (next) next.prev = prev;
}
30 changes: 11 additions & 19 deletions test/runtime/samples/binding-textarea/_config.js
Original file line number Diff line number Diff line change
Expand Up @@ -3,35 +3,27 @@ export default {
value: 'some text',
},

html: `
<textarea></textarea>
<p>some text</p>
`,

ssrHtml: `
<textarea>some text</textarea>
<p>some text</p>
`,

async test({ assert, component, target, window }) {
async test({ assert, component, target, window, compileOptions }) {
const textarea = target.querySelector('textarea');
const p = target.querySelector('p');
assert.equal(textarea.value, 'some text');
assert.equal(p.textContent, 'some text');
assert.equal(textarea.attributes.length, 0);

const event = new window.Event('input');

textarea.value = 'hello';
await textarea.dispatchEvent(event);

assert.htmlEqual(target.innerHTML, `
<textarea></textarea>
<p>hello</p>
`);
assert.equal(textarea.value, 'hello');
assert.equal(p.textContent, 'hello');
assert.equal(textarea.attributes.length, 0);

component.value = 'goodbye';
assert.equal(textarea.value, 'goodbye');
assert.htmlEqual(target.innerHTML, `
<textarea></textarea>
<p>goodbye</p>
`);

assert.equal(textarea.value, 'goodbye');
assert.equal(p.textContent, 'goodbye');
assert.equal(textarea.attributes.length, 0);
},
};

0 comments on commit 89b7cf6

Please sign in to comment.