Skip to content
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.

Sign up for GitHub

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

[feat] use a simpler insert and append function when not compile with hydratable #6525

Merged
merged 3 commits into from
Jul 21, 2021
Merged

[feat] use a simpler insert and append function when not compile with hydratable #6525

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
fcd784b
use a simpler insert and append function when not compile with hydrat…
tanhauhau Jul 13, 2021
39b8072
Merge branch 'master' into pr/6525
Conduitry Jul 21, 2021
42d8a6b
fix merge
Conduitry Jul 21, 2021
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
8 changes: 8 additions & 0 deletions src/compiler/compile/Component.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -267,6 +267,14 @@ export default class Component {
} else {
let name = node.name.slice(1);

if (compile_options.hydratable) {
if (internal_exports.has(`${name}_hydration`)) {
name += '_hydration';
} else if (internal_exports.has(`${name}Hydration`)) {
Copy link
Member

Choose a reason for hiding this comment

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

Question for understanding: What's this about? There's only methods ending with _hydration

Copy link
Member Author

@tanhauhau tanhauhau Jul 14, 2021 •
edited
Loading

Choose a reason for hiding this comment

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

  1. the function you are looking at here, is the magic that turns
@insert(...)

into

import { insert } from 'svelte/internal';
insert(...);
  1. svelte/internal exposes a lot of methods, which includes insert and insert_dev, see https://github.com/sveltejs/svelte/blob/master/src/runtime/internal/dev.ts

the function you see here, decides on whether to use insert or insert_dev from @insert by checking the compileOptions.dev, similarly, we could decide to use insert or insert_hydration based on compileOptions.hydratable

  1. to determine the list of exports from svelte/internal, and then decide whether is there a xxx_dev exported, we build the svelte/internal and write them into a file internal_exports.ts, see

svelte/rollup.config.js

Lines 86 to 92 in 931738e

writeBundle(bundle) {
if (dir === 'internal') {
const mod = bundle['index.mjs'];
if (mod) {
fs.writeFileSync('src/compiler/compile/internal_exports.ts', `// This file is automatically generated\nexport default new Set(${JSON.stringify(mod.exports)});`);
}
}

for @xxx, the function above check whether xxx_hydration is present within internal_exports, if so, import xxx_hydration instead of xxx. it then checks if xxx_hydration_dev is present, if so it will use xxx_hydration_dev instead of xxx_hydration.

Copy link
Member

@dummdidumm dummdidumm Jul 14, 2021 •
edited
Loading

Choose a reason for hiding this comment

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

Thanks for the thourough answer! What I don't understand though is why there's this inner if clause for each of these cases. The methods are called append_hydration_dev, but the if clauses look to me like there's also appendHydrationDev which is not the case, or am I misunderstanding something there?
Or in code:
Why this

								if (compile_options.hydratable) {
									if (internal_exports.has(`${name}_hydration`)) {
										name += '_hydration';
									} else if (internal_exports.has(`${name}Hydration`)) {
										name += 'Hydration';
									}
								}

and not this?

								if (compile_options.hydratable) {
									if (internal_exports.has(`${name}_hydration`)) {
										name += '_hydration';
									}
								}

Edit: aaah got the answer, it's the classes

Copy link
Member Author

Choose a reason for hiding this comment

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

Yea.. there's SvelteComponentDev

svelte/src/runtime/internal/dev.ts

Line 120 in 931738e

export class SvelteComponentDev extends SvelteComponent {

and HtmlTagHydration

Copy link
Member

Choose a reason for hiding this comment

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

this seems like it could be worth a code comment

name += 'Hydration';
}
}

if (compile_options.dev) {
if (internal_exports.has(`${name}_dev`)) {
name += '_dev';
Expand Down
12 changes: 11 additions & 1 deletion src/runtime/internal/dev.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import { custom_event, append, insert, detach, listen, attr } from './dom';
import { custom_event, append, append_hydration, insert, insert_hydration, detach, listen, attr } from './dom';
import { SvelteComponent } from './Component';

export function dispatch_dev<T=any>(type: string, detail?: T) {
Expand All @@ -10,11 +10,21 @@ export function append_dev(target: Node, node: Node) {
append(target, node);
}

export function append_hydration_dev(target: Node, node: Node) {
dispatch_dev('SvelteDOMInsert', { target, node });
append_hydration(target, node);
}

export function insert_dev(target: Node, node: Node, anchor?: Node) {
dispatch_dev('SvelteDOMInsert', { target, node, anchor });
insert(target, node, anchor);
}

export function insert_hydration_dev(target: Node, node: Node, anchor?: Node) {
dispatch_dev('SvelteDOMInsert', { target, node, anchor });
insert_hydration(target, node, anchor);
}

export function detach_dev(node: Node) {
dispatch_dev('SvelteDOMRemove', { node });
detach(node);
Expand Down
56 changes: 42 additions & 14 deletions src/runtime/internal/dom.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -112,7 +112,11 @@ function init_hydrate(target: NodeEx) {
}
}

export function append(target: NodeEx, node: NodeEx) {
export function append(target: Node, node: Node) {
target.appendChild(node);
}

export function append_hydration(target: NodeEx, node: NodeEx) {
if (is_hydrating) {
init_hydrate(target);

Expand All @@ -129,9 +133,13 @@ export function append(target: NodeEx, node: NodeEx) {
}
}

export function insert(target: NodeEx, node: NodeEx, anchor?: NodeEx) {
export function insert(target: Node, node: Node, anchor?: Node) {
target.insertBefore(node, anchor || null);
}

export function insert_hydration(target: NodeEx, node: NodeEx, anchor?: NodeEx) {
if (is_hydrating && !anchor) {
append(target, node);
append_hydration(target, node);
} else if (node.parentNode !== target || node.nextSibling != anchor) {
target.insertBefore(node, anchor || null);
}
Expand Down Expand Up @@ -404,12 +412,12 @@ export function claim_html_tag(nodes) {
const start_index = find_comment(nodes, 'HTML_TAG_START', 0);
const end_index = find_comment(nodes, 'HTML_TAG_END', start_index);
if (start_index === end_index) {
return new HtmlTag();
return new HtmlTagHydration();
}
const html_tag_nodes = nodes.splice(start_index, end_index + 1);
detach(html_tag_nodes[0]);
detach(html_tag_nodes[html_tag_nodes.length - 1]);
return new HtmlTag(html_tag_nodes.slice(1, html_tag_nodes.length - 1));
return new HtmlTagHydration(html_tag_nodes.slice(1, html_tag_nodes.length - 1));
}

export function set_data(text, data) {
Expand Down Expand Up @@ -543,27 +551,24 @@ export class HtmlTag {
e: HTMLElement;
// html tag nodes
n: ChildNode[];
// hydration claimed nodes
l: ChildNode[] | void;
// target
t: HTMLElement;
// anchor
a: HTMLElement;

constructor(claimed_nodes?: ChildNode[]) {
constructor() {
this.e = this.n = null;
this.l = claimed_nodes;
}

c(html: string) {
this.h(html);
}

m(html: string, target: HTMLElement, anchor: HTMLElement = null) {
if (!this.e) {
this.e = element(target.nodeName as keyof HTMLElementTagNameMap);
this.t = target;
if (this.l) {
this.n = this.l;
} else {
this.h(html);
}
this.c(html);
}

this.i(anchor);
Expand Down Expand Up @@ -591,6 +596,29 @@ export class HtmlTag {
}
}

export class HtmlTagHydration extends HtmlTag {
// hydration claimed nodes
l: ChildNode[] | void;

constructor(claimed_nodes?: ChildNode[]) {
super();
this.e = this.n = null;
this.l = claimed_nodes;
}
c(html: string) {
if (this.l) {
this.n = this.l;
} else {
super.c(html);
}
}
i(anchor) {
for (let i = 0; i < this.n.length; i += 1) {
insert_hydration(this.t, this.n[i], anchor);
}
}
}

export function attribute_to_object(attributes: NamedNodeMap) {
const result = {};
for (const attribute of attributes) {
Expand Down
8 changes: 4 additions & 4 deletions test/js/samples/hydrated-void-element/expected.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ import {
detach,
element,
init,
insert,
insert_hydration,
noop,
safe_not_equal,
space
Expand Down Expand Up @@ -39,9 +39,9 @@ function create_fragment(ctx) {
attr(img, "alt", "donuts");
},
m(target, anchor) {
insert(target, img, anchor);
insert(target, t, anchor);
insert(target, div, anchor);
insert_hydration(target, img, anchor);
insert_hydration(target, t, anchor);
insert_hydration(target, div, anchor);
},
p: noop,
i: noop,
Expand Down
8 changes: 4 additions & 4 deletions test/js/samples/src-attribute-check/expected.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ import {
detach,
element,
init,
insert,
insert_hydration,
noop,
safe_not_equal,
space
Expand Down Expand Up @@ -40,9 +40,9 @@ function create_fragment(ctx) {
if (img1.src !== (img1_src_value = "" + (/*slug*/ ctx[1] + ".jpg"))) attr(img1, "src", img1_src_value);
},
m(target, anchor) {
insert(target, img0, anchor);
insert(target, t, anchor);
insert(target, img1, anchor);
insert_hydration(target, img0, anchor);
insert_hydration(target, t, anchor);
insert_hydration(target, img1, anchor);
},
p(ctx, [dirty]) {
if (dirty & /*url*/ 1 && img0.src !== (img0_src_value = /*url*/ ctx[0])) {
Expand Down