Skip to content

Optimise parsed AST #3766

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
wants to merge 4 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
19 changes: 14 additions & 5 deletions src/compiler/compile/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ import { CompileOptions, Warning } from '../interfaces';
import Component from './Component';
import fuzzymatch from '../utils/fuzzymatch';
import get_name_from_filename from './utils/get_name_from_filename';
import optimise from '../parse/optimise/index';

const valid_options = [
'format',
Expand All @@ -26,7 +27,8 @@ const valid_options = [
'css',
'loopGuardTimeout',
'preserveComments',
'preserveWhitespace'
'preserveWhitespace',
'optimiseAst',
];

function validate_options(options: CompileOptions, warnings: Warning[]) {
Expand Down Expand Up @@ -68,7 +70,7 @@ function validate_options(options: CompileOptions, warnings: Warning[]) {
}

export default function compile(source: string, options: CompileOptions = {}) {
options = assign({ generate: 'dom', dev: false }, options);
options = assign({ generate: 'dom', dev: false, optimiseAst: true }, options);

const stats = new Stats();
const warnings = [];
Expand All @@ -79,6 +81,12 @@ export default function compile(source: string, options: CompileOptions = {}) {
const ast = parse(source, options);
stats.stop('parse');

if (options.optimiseAst) {
stats.start('optimise-ast');
optimise(ast);
stats.stop('optimise-ast');
}

stats.start('create component');
const component = new Component(
ast,
Expand All @@ -90,9 +98,10 @@ export default function compile(source: string, options: CompileOptions = {}) {
);
stats.stop('create component');

const js = options.generate === false
? null
: options.generate === 'ssr'
const js =
options.generate === false
? null
: options.generate === 'ssr'
? render_ssr(component, options)
: render_dom(component, options);

Expand Down
7 changes: 7 additions & 0 deletions src/compiler/compile/render_dom/wrappers/Element/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -311,6 +311,13 @@ export default class ElementWrapper extends Wrapper {
// @ts-ignore todo: should it be this.fragment.nodes[0].node.data instead?
b`${node}.textContent = ${string_literal(this.fragment.nodes[0].data)};`
);
} else if (
this.fragment.nodes.length === 1 &&
this.fragment.nodes[0].node.type === 'MustacheTag' &&
this.fragment.nodes[0].node.expression.node.type === 'TemplateLiteral') {
block.chunks.create.push(
b`${node}.textContent = ${this.fragment.nodes[0].node.expression.manipulate(block)};`
);
} else {
const state = {
quasi: {
Expand Down
13 changes: 10 additions & 3 deletions src/compiler/compile/render_dom/wrappers/shared/Tag.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ import Renderer from '../../Renderer';
import Block from '../../Block';
import MustacheTag from '../../../nodes/MustacheTag';
import RawMustacheTag from '../../../nodes/RawMustacheTag';
import { is_string } from './is_string';
import { Node } from 'estree';
import { changed } from './changed';

Expand All @@ -30,14 +31,20 @@ export default class Tag extends Wrapper {
update: ((value: Node) => (Node | Node[]))
) {
const dependencies = this.node.expression.dynamic_dependencies();
const should_extract = this.node.should_cache && dependencies.length > 0;
let snippet = this.node.expression.manipulate(block);
snippet = is_string(snippet) ? snippet : x`${snippet} + ""`;

if (should_extract) {
const fn_name = block.get_unique_name(`${this.var.name}_fn`);
block.add_variable(fn_name, x`(#ctx) => ${snippet}`);
snippet = x`${fn_name}(#ctx)`;
}

const value = this.node.should_cache && block.get_unique_name(`${this.var.name}_value`);
const content = this.node.should_cache ? value : snippet;

snippet = x`${snippet} + ""`;

if (this.node.should_cache) block.add_variable(value, snippet); // TODO may need to coerce snippet to string
if (this.node.should_cache) block.add_variable(value, snippet);

if (dependencies.length > 0) {
let condition = changed(dependencies);
Expand Down
3 changes: 3 additions & 0 deletions src/compiler/compile/render_dom/wrappers/shared/is_string.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
export function is_string(node) {
return node.type === 'TemplateLiteral' || (node.type === 'Literal' && typeof node.value === 'string');
}
1 change: 1 addition & 0 deletions src/compiler/interfaces.ts
Original file line number Diff line number Diff line change
Expand Up @@ -125,6 +125,7 @@ export interface CompileOptions {

preserveComments?: boolean;
preserveWhitespace?: boolean;
optimiseAst?: boolean;
}

export interface ParserOptions {
Expand Down
84 changes: 84 additions & 0 deletions src/compiler/parse/optimise/index.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,84 @@
import { walk } from 'estree-walker';
import { Text, MustacheTag } from '../../interfaces';

export default function optimise(ast) {
walk(ast, {
enter(node: any) {
if (node.type === 'Element') {
optimise_text_content(node.children);
}
},
});
}

const text_like_node_type = new Set(['MustacheTag', 'Text']);

function optimise_text_content(children) {
let start = 0;
let end = 0;

do {
while (
start < children.length &&
!text_like_node_type.has(children[start].type)
)
start++;

end = start;

while (end < children.length && text_like_node_type.has(children[end].type))
end++;

// based on heuristic,
// require more than 2 neighouring text contents to merge yields smaller content
if (end > start + 2) {
const merged = merge_text_siblings(children.slice(start, end));
children.splice(start, end - start, merged);
}
start = end;
} while (start < children.length);
}

function merge_text_siblings(children: Array<Text | MustacheTag>) {

const literal = {
type: 'TemplateLiteral',
expressions: [],
quasis: [],
};
const state = {
quasi: {
type: 'TemplateElement',
value: { raw: '' },
start: children[0].start,
end: children[0].start
},
};

for (const child of children) {
if (child.type === 'MustacheTag') {
literal.quasis.push(state.quasi);
literal.expressions.push(child.expression);
state.quasi = {
type: 'TemplateElement',
value: { raw: '' },
// @ts-ignore
start: child.expression.end + 1,
// @ts-ignore
end: child.expression.end + 1
};
} else if (child.type === 'Text') {
state.quasi.value.raw += child.data;
state.quasi.end = child.end;
}
}

literal.quasis.push(state.quasi);

return {
type: 'MustacheTag',
expression: literal,
start: children[0].start,
end: children[children.length - 1].end,
};
}
2 changes: 1 addition & 1 deletion test/js/index.js
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
import * as assert from "assert";
import * as fs from "fs";
import * as path from "path";
import * as kleur from "kleur";
import * as colors from "kleur";
import { loadConfig, svelte, shouldUpdateExpected } from "../helpers.js";

describe("js", () => {
Expand Down
4 changes: 2 additions & 2 deletions test/js/samples/capture-inject-dev-only/expected.js
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@ function create_fragment(ctx) {
return {
c() {
p = element("p");
t0 = text(ctx.foo);
t0 = text(ctx.foo + "");
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

it'd be good if we didn't have the + "" here, since it's coerced to a string anyway

t1 = space();
input = element("input");
dispose = listen(input, "input", ctx.input_input_handler);
Expand All @@ -38,7 +38,7 @@ function create_fragment(ctx) {
set_input_value(input, ctx.foo);
},
p(changed, ctx) {
if (changed.foo) set_data(t0, ctx.foo);
if (changed.foo) set_data(t0, ctx.foo + "");

if (changed.foo && input.value !== ctx.foo) {
set_input_value(input, ctx.foo);
Expand Down
4 changes: 2 additions & 2 deletions test/js/samples/collapses-text-around-comments/expected.js
Original file line number Diff line number Diff line change
Expand Up @@ -27,15 +27,15 @@ function create_fragment(ctx) {
return {
c() {
p = element("p");
t = text(ctx.foo);
t = text(ctx.foo + "");
attr(p, "class", "svelte-1a7i8ec");
},
m(target, anchor) {
insert(target, p, anchor);
append(p, t);
},
p(changed, ctx) {
if (changed.foo) set_data(t, ctx.foo);
if (changed.foo) set_data(t, ctx.foo + "");
},
i: noop,
o: noop,
Expand Down
4 changes: 2 additions & 2 deletions test/js/samples/component-store-access-invalidate/expected.js
Original file line number Diff line number Diff line change
Expand Up @@ -22,14 +22,14 @@ function create_fragment(ctx) {
return {
c() {
h1 = element("h1");
t = text(ctx.$foo);
t = text(ctx.$foo + "");
},
m(target, anchor) {
insert(target, h1, anchor);
append(h1, t);
},
p(changed, ctx) {
if (changed.$foo) set_data(t, ctx.$foo);
if (changed.$foo) set_data(t, ctx.$foo + "");
},
i: noop,
o: noop,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@ function create_fragment(ctx) {
return {
c() {
h1 = element("h1");
t0 = text(ctx.$foo);
t0 = text(ctx.$foo + "");
t1 = space();
button = element("button");
button.textContent = "reset";
Expand All @@ -40,7 +40,7 @@ function create_fragment(ctx) {
insert(target, button, anchor);
},
p(changed, ctx) {
if (changed.$foo) set_data(t0, ctx.$foo);
if (changed.$foo) set_data(t0, ctx.$foo + "");
},
i: noop,
o: noop,
Expand Down
18 changes: 7 additions & 11 deletions test/js/samples/debug-empty/expected.js
Original file line number Diff line number Diff line change
Expand Up @@ -19,18 +19,16 @@ const file = undefined;

function create_fragment(ctx) {
let h1;
let t0_fn = ctx => `Hello ${ctx.name}!`;
let t0_value = t0_fn(ctx);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

we should be able to hoist these functions out of the block altogether, I think?

(I still have an ambition to merge the create/update phases so that we can just 'update' everything to its initial state, rather than having this duplication at all. But that's a future nice-to-have)

let t0;
let t1;
let t2;
let t3;

const block = {
c: function create() {
h1 = element("h1");
t0 = text("Hello ");
t1 = text(ctx.name);
t2 = text("!");
t3 = space();
t0 = text(t0_value);
t1 = space();
debugger;
add_location(h1, file, 4, 0, 38);
},
Expand All @@ -40,19 +38,17 @@ function create_fragment(ctx) {
m: function mount(target, anchor) {
insert_dev(target, h1, anchor);
append_dev(h1, t0);
append_dev(h1, t1);
append_dev(h1, t2);
insert_dev(target, t3, anchor);
insert_dev(target, t1, anchor);
},
p: function update(changed, ctx) {
if (changed.name) set_data_dev(t1, ctx.name);
if (changed.name && t0_value !== (t0_value = t0_fn(ctx))) set_data_dev(t0, t0_value);
debugger;
},
i: noop,
o: noop,
d: function destroy(detaching) {
if (detaching) detach_dev(h1);
if (detaching) detach_dev(t3);
if (detaching) detach_dev(t1);
}
};

Expand Down
9 changes: 5 additions & 4 deletions test/js/samples/debug-foo-bar-baz-things/expected.js
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,8 @@ function get_each_context(ctx, list, i) {
// (8:0) {#each things as thing}
function create_each_block(ctx) {
let span;
let t0_value = ctx.thing.name + "";
let t0_fn = ctx => ctx.thing.name + "";
let t0_value = t0_fn(ctx);
let t0;
let t1;

Expand All @@ -51,7 +52,7 @@ function create_each_block(ctx) {
insert_dev(target, t1, anchor);
},
p: function update(changed, ctx) {
if (changed.things && t0_value !== (t0_value = ctx.thing.name + "")) set_data_dev(t0, t0_value);
if (changed.things && t0_value !== (t0_value = t0_fn(ctx))) set_data_dev(t0, t0_value);

if (changed.foo || changed.bar || changed.baz || changed.things) {
const { foo, bar, baz, thing } = ctx;
Expand Down Expand Up @@ -97,7 +98,7 @@ function create_fragment(ctx) {
t0 = space();
p = element("p");
t1 = text("foo: ");
t2 = text(ctx.foo);
t2 = text(ctx.foo + "");
add_location(p, file, 12, 0, 182);
},
l: function claim(nodes) {
Expand Down Expand Up @@ -137,7 +138,7 @@ function create_fragment(ctx) {
each_blocks.length = each_value.length;
}

if (changed.foo) set_data_dev(t2, ctx.foo);
if (changed.foo) set_data_dev(t2, ctx.foo + "");
},
i: noop,
o: noop,
Expand Down
9 changes: 5 additions & 4 deletions test/js/samples/debug-foo/expected.js
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,8 @@ function get_each_context(ctx, list, i) {
// (6:0) {#each things as thing}
function create_each_block(ctx) {
let span;
let t0_value = ctx.thing.name + "";
let t0_fn = ctx => ctx.thing.name + "";
let t0_value = t0_fn(ctx);
let t0;
let t1;

Expand All @@ -51,7 +52,7 @@ function create_each_block(ctx) {
insert_dev(target, t1, anchor);
},
p: function update(changed, ctx) {
if (changed.things && t0_value !== (t0_value = ctx.thing.name + "")) set_data_dev(t0, t0_value);
if (changed.things && t0_value !== (t0_value = t0_fn(ctx))) set_data_dev(t0, t0_value);

if (changed.foo) {
const { foo } = ctx;
Expand Down Expand Up @@ -97,7 +98,7 @@ function create_fragment(ctx) {
t0 = space();
p = element("p");
t1 = text("foo: ");
t2 = text(ctx.foo);
t2 = text(ctx.foo + "");
add_location(p, file, 10, 0, 131);
},
l: function claim(nodes) {
Expand Down Expand Up @@ -137,7 +138,7 @@ function create_fragment(ctx) {
each_blocks.length = each_value.length;
}

if (changed.foo) set_data_dev(t2, ctx.foo);
if (changed.foo) set_data_dev(t2, ctx.foo + "");
},
i: noop,
o: noop,
Expand Down
Loading