Skip to content

Commit 3a49110

Browse files
tanhauhauConduitry
authored and
taylorzane
committed
feat order attributes + actions too (sveltejs#4156)
Co-authored-by: Conduitry <git@chor.date>
1 parent d600b35 commit 3a49110

File tree

8 files changed

+149
-62
lines changed

8 files changed

+149
-62
lines changed

‎src/compiler/compile/render_dom/wrappers/Element/index.ts

Lines changed: 27 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -16,15 +16,16 @@ import { dimensions } from '../../../../utils/patterns';
1616
import Binding from './Binding';
1717
import InlineComponentWrapper from '../InlineComponent';
1818
import add_to_set from '../../../utils/add_to_set';
19-
import add_event_handlers from '../shared/add_event_handlers';
20-
import add_actions from '../shared/add_actions';
19+
import { add_event_handler } from '../shared/add_event_handlers';
20+
import { add_action } from '../shared/add_actions';
2121
import create_debugging_comment from '../shared/create_debugging_comment';
2222
import { get_slot_definition } from '../shared/get_slot_definition';
2323
import bind_this from '../shared/bind_this';
2424
import { is_head } from '../shared/is_head';
2525
import { Identifier } from 'estree';
2626
import EventHandler from './EventHandler';
2727
import { extract_names } from 'periscopic';
28+
import Action from '../../../nodes/Action';
2829

2930
const events = [
3031
{
@@ -380,7 +381,6 @@ export default class ElementWrapper extends Wrapper {
380381
this.add_directives_in_order(block);
381382
this.add_transitions(block);
382383
this.add_animation(block);
383-
this.add_actions(block);
384384
this.add_classes(block);
385385
this.add_manual_style_scoping(block);
386386

@@ -441,6 +441,8 @@ export default class ElementWrapper extends Wrapper {
441441
bindings: Binding[];
442442
}
443443

444+
type OrderedAttribute = EventHandler | BindingGroup | Binding | Action;
445+
444446
const bindingGroups = events
445447
.map(event => ({
446448
events: event.event_names,
@@ -452,29 +454,37 @@ export default class ElementWrapper extends Wrapper {
452454

453455
const this_binding = this.bindings.find(b => b.node.name === 'this');
454456

455-
function getOrder (item: EventHandler | BindingGroup | Binding) {
457+
function getOrder (item: OrderedAttribute) {
456458
if (item instanceof EventHandler) {
457459
return item.node.start;
458460
} else if (item instanceof Binding) {
459461
return item.node.start;
462+
} else if (item instanceof Action) {
463+
return item.start;
460464
} else {
461465
return item.bindings[0].node.start;
462466
}
463467
}
464468

465-
const ordered: Array<EventHandler | BindingGroup | Binding> = [].concat(bindingGroups, this.event_handlers, this_binding).filter(Boolean);
466-
467-
ordered.sort((a, b) => getOrder(a) - getOrder(b));
468-
469-
ordered.forEach(bindingGroupOrEventHandler => {
470-
if (bindingGroupOrEventHandler instanceof EventHandler) {
471-
add_event_handlers(block, this.var, [bindingGroupOrEventHandler]);
472-
} else if (bindingGroupOrEventHandler instanceof Binding) {
473-
this.add_this_binding(block, bindingGroupOrEventHandler);
474-
} else {
475-
this.add_bindings(block, bindingGroupOrEventHandler);
476-
}
477-
});
469+
([
470+
...bindingGroups,
471+
...this.event_handlers,
472+
this_binding,
473+
...this.node.actions
474+
] as OrderedAttribute[])
475+
.filter(Boolean)
476+
.sort((a, b) => getOrder(a) - getOrder(b))
477+
.forEach(item => {
478+
if (item instanceof EventHandler) {
479+
add_event_handler(block, this.var, item);
480+
} else if (item instanceof Binding) {
481+
this.add_this_binding(block, item);
482+
} else if (item instanceof Action) {
483+
add_action(block, this.var, item);
484+
} else {
485+
this.add_bindings(block, item);
486+
}
487+
});
478488
}
479489

480490
add_bindings(block: Block, bindingGroup) {
@@ -701,10 +711,6 @@ export default class ElementWrapper extends Wrapper {
701711
`);
702712
}
703713

704-
add_event_handlers(block: Block) {
705-
add_event_handlers(block, this.var, this.event_handlers);
706-
}
707-
708714
add_transitions(
709715
block: Block
710716
) {
@@ -866,10 +872,6 @@ export default class ElementWrapper extends Wrapper {
866872
`);
867873
}
868874

869-
add_actions(block: Block) {
870-
add_actions(block, this.var, this.node.actions);
871-
}
872-
873875
add_classes(block: Block) {
874876
const has_spread = this.node.attributes.some(attr => attr.is_spread);
875877
this.node.classes.forEach(class_directive => {

‎src/compiler/compile/render_dom/wrappers/shared/add_actions.ts

Lines changed: 26 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -7,42 +7,40 @@ export default function add_actions(
77
target: string,
88
actions: Action[]
99
) {
10-
actions.forEach(action => {
11-
const { expression } = action;
12-
let snippet;
13-
let dependencies;
14-
15-
if (expression) {
16-
snippet = expression.manipulate(block);
17-
dependencies = expression.dynamic_dependencies();
18-
}
10+
actions.forEach(action => add_action(block, target, action));
11+
}
1912

20-
const id = block.get_unique_name(
21-
`${action.name.replace(/[^a-zA-Z0-9_$]/g, '_')}_action`
22-
);
13+
export function add_action(block: Block, target: string, action: Action) {
14+
const { expression } = action;
15+
let snippet;
16+
let dependencies;
2317

24-
block.add_variable(id);
18+
if (expression) {
19+
snippet = expression.manipulate(block);
20+
dependencies = expression.dynamic_dependencies();
21+
}
2522

26-
const fn = block.renderer.reference(action.name);
23+
const id = block.get_unique_name(
24+
`${action.name.replace(/[^a-zA-Z0-9_$]/g, '_')}_action`
25+
);
2726

28-
block.chunks.mount.push(
29-
b`${id} = ${fn}.call(null, ${target}, ${snippet}) || {};`
30-
);
27+
block.add_variable(id);
28+
29+
const fn = block.renderer.reference(action.name);
3130

32-
if (dependencies && dependencies.length > 0) {
33-
let condition = x`@is_function(${id}.update)`;
31+
block.event_listeners.push(
32+
x`@action_destroyer(${id} = ${fn}.call(null, ${target}, ${snippet}))`
33+
);
3434

35-
if (dependencies.length > 0) {
36-
condition = x`${condition} && ${block.renderer.dirty(dependencies)}`;
37-
}
35+
if (dependencies && dependencies.length > 0) {
36+
let condition = x`${id} && @is_function(${id}.update)`;
3837

39-
block.chunks.update.push(
40-
b`if (${condition}) ${id}.update.call(null, ${snippet});`
41-
);
38+
if (dependencies.length > 0) {
39+
condition = x`${condition} && ${block.renderer.dirty(dependencies)}`;
4240
}
4341

44-
block.chunks.destroy.push(
45-
b`if (${id} && @is_function(${id}.destroy)) ${id}.destroy();`
42+
block.chunks.update.push(
43+
b`if (${condition}) ${id}.update.call(null, ${snippet});`
4644
);
47-
});
45+
}
4846
}

‎src/compiler/compile/render_dom/wrappers/shared/add_event_handlers.ts

Lines changed: 9 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -6,5 +6,13 @@ export default function add_event_handlers(
66
target: string,
77
handlers: EventHandler[]
88
) {
9-
handlers.forEach(handler => handler.render(block, target));
9+
handlers.forEach(handler => add_event_handler(block, target, handler));
10+
}
11+
12+
export function add_event_handler(
13+
block: Block,
14+
target: string,
15+
handler: EventHandler
16+
) {
17+
handler.render(block, target);
1018
}

‎src/runtime/internal/utils.ts

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -120,4 +120,8 @@ export function set_store_value(store, ret, value = ret) {
120120
return ret;
121121
}
122122

123-
export const has_prop = (obj, prop) => Object.prototype.hasOwnProperty.call(obj, prop);
123+
export const has_prop = (obj, prop) => Object.prototype.hasOwnProperty.call(obj, prop);
124+
125+
export function action_destroyer(action_result) {
126+
return action_result && is_function(action_result.destroy) ? action_result.destroy : noop;
127+
}

‎test/js/samples/action-custom-event-handler/expected.js

Lines changed: 6 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
/* generated by Svelte vX.Y.Z */
22
import {
33
SvelteComponent,
4+
action_destroyer,
45
detach,
56
element,
67
init,
@@ -13,24 +14,25 @@ import {
1314
function create_fragment(ctx) {
1415
let button;
1516
let foo_action;
17+
let dispose;
1618

1719
return {
1820
c() {
1921
button = element("button");
2022
button.textContent = "foo";
23+
dispose = action_destroyer(foo_action = foo.call(null, button, /*foo_function*/ ctx[1]));
2124
},
2225
m(target, anchor) {
2326
insert(target, button, anchor);
24-
foo_action = foo.call(null, button, /*foo_function*/ ctx[1]) || ({});
2527
},
2628
p(ctx, [dirty]) {
27-
if (is_function(foo_action.update) && dirty & /*bar*/ 1) foo_action.update.call(null, /*foo_function*/ ctx[1]);
29+
if (foo_action && is_function(foo_action.update) && dirty & /*bar*/ 1) foo_action.update.call(null, /*foo_function*/ ctx[1]);
2830
},
2931
i: noop,
3032
o: noop,
3133
d(detaching) {
3234
if (detaching) detach(button);
33-
if (foo_action && is_function(foo_action.destroy)) foo_action.destroy();
35+
dispose();
3436
}
3537
};
3638
}
@@ -40,7 +42,7 @@ function handleFoo(bar) {
4042
}
4143

4244
function foo(node, callback) {
43-
45+
4446
}
4547

4648
function instance($$self, $$props, $$invalidate) {

‎test/js/samples/action/expected.js

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,36 +1,37 @@
11
/* generated by Svelte vX.Y.Z */
22
import {
33
SvelteComponent,
4+
action_destroyer,
45
attr,
56
detach,
67
element,
78
init,
89
insert,
9-
is_function,
1010
noop,
1111
safe_not_equal
1212
} from "svelte/internal";
1313

1414
function create_fragment(ctx) {
1515
let a;
1616
let link_action;
17+
let dispose;
1718

1819
return {
1920
c() {
2021
a = element("a");
2122
a.textContent = "Test";
2223
attr(a, "href", "#");
24+
dispose = action_destroyer(link_action = link.call(null, a));
2325
},
2426
m(target, anchor) {
2527
insert(target, a, anchor);
26-
link_action = link.call(null, a) || ({});
2728
},
2829
p: noop,
2930
i: noop,
3031
o: noop,
3132
d(detaching) {
3233
if (detaching) detach(a);
33-
if (link_action && is_function(link_action.destroy)) link_action.destroy();
34+
dispose();
3435
}
3536
};
3637
}

‎test/runtime/samples/apply-directives-in-order-2/_config.js

Lines changed: 38 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,38 @@
1+
const value = [];
2+
export default {
3+
props: {
4+
value,
5+
},
6+
7+
async test({ assert, component, target, window }) {
8+
const inputs = target.querySelectorAll('input');
9+
10+
const event = new window.Event('input');
11+
12+
for (const input of inputs) {
13+
input.value = 'h';
14+
await input.dispatchEvent(event);
15+
}
16+
17+
assert.deepEqual(value, [
18+
'1',
19+
'2',
20+
'3',
21+
'4',
22+
'5',
23+
'6',
24+
'7',
25+
'8',
26+
'9',
27+
'10',
28+
'11',
29+
'12',
30+
'13',
31+
'14',
32+
'15',
33+
'16',
34+
'17',
35+
'18',
36+
]);
37+
},
38+
};

‎test/runtime/samples/apply-directives-in-order-2/main.svelte

Lines changed: 34 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,34 @@
1+
<script>
2+
export let value = [];
3+
4+
function one(elem) { elem.addEventListener('input', () => { value.push('1'); }); }
5+
function four(elem) { elem.addEventListener('input', () => { value.push('4'); }); }
6+
function eight(elem) { elem.addEventListener('input', () => { value.push('8'); }); }
7+
function twelve(elem) { elem.addEventListener('input', () => { value.push('12'); }); }
8+
function fifteen(elem) { elem.addEventListener('input', () => { value.push('15'); }); }
9+
function seventeen(elem) { elem.addEventListener('input', () => { value.push('17'); }); }
10+
11+
const foo = {
12+
set two(v) { value.push('2'); },
13+
set six(v) { value.push('6'); },
14+
set nine(v) { value.push('9'); },
15+
set eleven(v) { value.push('11'); },
16+
set thirteen(v) { value.push('13'); },
17+
set sixteen(v) { value.push('16'); },
18+
}
19+
20+
function three() { value.push('3'); }
21+
function five() { value.push('5'); }
22+
function seven() { value.push('7'); }
23+
function ten() { value.push('10'); }
24+
function fourteen() { value.push('14'); }
25+
function eighteen() { value.push('18'); }
26+
27+
</script>
28+
29+
<input use:one bind:value={foo.two} on:input={three} />
30+
<input use:four on:input={five} bind:value={foo.six} />
31+
<input on:input={seven} use:eight bind:value={foo.nine} />
32+
<input on:input={ten} bind:value={foo.eleven} use:twelve />
33+
<input bind:value={foo.thirteen} on:input={fourteen} use:fifteen />
34+
<input bind:value={foo.sixteen} use:seventeen on:input={eighteen} />

0 commit comments

Comments
 (0)