From b433440533d9535cfb7aec87ec0d7dbcb8fe2372 Mon Sep 17 00:00:00 2001 From: Tan Li Hau Date: Tue, 24 Dec 2019 19:13:36 +0800 Subject: [PATCH 1/2] feat order attributes + actions too --- .../render_dom/wrappers/Element/index.ts | 52 +++++++++--------- .../render_dom/wrappers/shared/add_actions.ts | 54 +++++++++---------- .../wrappers/shared/add_event_handlers.ts | 10 +++- src/runtime/internal/utils.ts | 10 +++- .../action-custom-event-handler/expected.js | 8 +-- test/js/samples/action/expected.js | 7 +-- .../apply-directives-in-order-2/_config.js | 38 +++++++++++++ .../apply-directives-in-order-2/main.svelte | 34 ++++++++++++ 8 files changed, 152 insertions(+), 61 deletions(-) create mode 100644 test/runtime/samples/apply-directives-in-order-2/_config.js create mode 100644 test/runtime/samples/apply-directives-in-order-2/main.svelte diff --git a/src/compiler/compile/render_dom/wrappers/Element/index.ts b/src/compiler/compile/render_dom/wrappers/Element/index.ts index 0478702c7c6e..ef330224027e 100644 --- a/src/compiler/compile/render_dom/wrappers/Element/index.ts +++ b/src/compiler/compile/render_dom/wrappers/Element/index.ts @@ -16,8 +16,8 @@ import { dimensions } from '../../../../utils/patterns'; import Binding from './Binding'; import InlineComponentWrapper from '../InlineComponent'; import add_to_set from '../../../utils/add_to_set'; -import add_event_handlers from '../shared/add_event_handlers'; -import add_actions from '../shared/add_actions'; +import { add_event_handler } from '../shared/add_event_handlers'; +import { add_action } from '../shared/add_actions'; import create_debugging_comment from '../shared/create_debugging_comment'; import { get_slot_definition } from '../shared/get_slot_definition'; import bind_this from '../shared/bind_this'; @@ -25,6 +25,7 @@ import { is_head } from '../shared/is_head'; import { Identifier } from 'estree'; import EventHandler from './EventHandler'; import { extract_names } from 'periscopic'; +import Action from '../../../nodes/Action'; const events = [ { @@ -380,7 +381,6 @@ export default class ElementWrapper extends Wrapper { this.add_directives_in_order(block); this.add_transitions(block); this.add_animation(block); - this.add_actions(block); this.add_classes(block); this.add_manual_style_scoping(block); @@ -441,6 +441,8 @@ export default class ElementWrapper extends Wrapper { bindings: Binding[]; } + type OrderedAttribute = EventHandler | BindingGroup | Binding | Action; + const bindingGroups = events .map(event => ({ events: event.event_names, @@ -452,29 +454,37 @@ export default class ElementWrapper extends Wrapper { const this_binding = this.bindings.find(b => b.node.name === 'this'); - function getOrder (item: EventHandler | BindingGroup | Binding) { + function getOrder (item: OrderedAttribute) { if (item instanceof EventHandler) { return item.node.start; } else if (item instanceof Binding) { return item.node.start; + } else if (item instanceof Action) { + return item.start; } else { return item.bindings[0].node.start; } } - const ordered: Array = [].concat(bindingGroups, this.event_handlers, this_binding).filter(Boolean); - - ordered.sort((a, b) => getOrder(a) - getOrder(b)); - - ordered.forEach(bindingGroupOrEventHandler => { - if (bindingGroupOrEventHandler instanceof EventHandler) { - add_event_handlers(block, this.var, [bindingGroupOrEventHandler]); - } else if (bindingGroupOrEventHandler instanceof Binding) { - this.add_this_binding(block, bindingGroupOrEventHandler); - } else { - this.add_bindings(block, bindingGroupOrEventHandler); - } - }); + ([ + ...bindingGroups, + ...this.event_handlers, + this_binding, + ...this.node.actions + ] as OrderedAttribute[]) + .filter(Boolean) + .sort((a, b) => getOrder(a) - getOrder(b)) + .forEach(item => { + if (item instanceof EventHandler) { + add_event_handler(block, this.var, item); + } else if (item instanceof Binding) { + this.add_this_binding(block, item); + } else if (item instanceof Action) { + add_action(block, this.var, item); + } else { + this.add_bindings(block, item); + } + }); } add_bindings(block: Block, bindingGroup) { @@ -701,10 +711,6 @@ export default class ElementWrapper extends Wrapper { `); } - add_event_handlers(block: Block) { - add_event_handlers(block, this.var, this.event_handlers); - } - add_transitions( block: Block ) { @@ -866,10 +872,6 @@ export default class ElementWrapper extends Wrapper { `); } - add_actions(block: Block) { - add_actions(block, this.var, this.node.actions); - } - add_classes(block: Block) { const has_spread = this.node.attributes.some(attr => attr.is_spread); this.node.classes.forEach(class_directive => { diff --git a/src/compiler/compile/render_dom/wrappers/shared/add_actions.ts b/src/compiler/compile/render_dom/wrappers/shared/add_actions.ts index e8f1ecb916e8..9b5ce7a1475d 100644 --- a/src/compiler/compile/render_dom/wrappers/shared/add_actions.ts +++ b/src/compiler/compile/render_dom/wrappers/shared/add_actions.ts @@ -7,42 +7,40 @@ export default function add_actions( target: string, actions: Action[] ) { - actions.forEach(action => { - const { expression } = action; - let snippet; - let dependencies; - - if (expression) { - snippet = expression.manipulate(block); - dependencies = expression.dynamic_dependencies(); - } + actions.forEach(action => add_action(block, target, action)); +} - const id = block.get_unique_name( - `${action.name.replace(/[^a-zA-Z0-9_$]/g, '_')}_action` - ); +export function add_action(block: Block, target: string, action: Action) { + const { expression } = action; + let snippet; + let dependencies; - block.add_variable(id); + if (expression) { + snippet = expression.manipulate(block); + dependencies = expression.dynamic_dependencies(); + } - const fn = block.renderer.reference(action.name); + const id = block.get_unique_name( + `${action.name.replace(/[^a-zA-Z0-9_$]/g, '_')}_action` + ); - block.chunks.mount.push( - b`${id} = ${fn}.call(null, ${target}, ${snippet}) || {};` - ); + block.add_variable(id); + + const fn = block.renderer.reference(action.name); - if (dependencies && dependencies.length > 0) { - let condition = x`@is_function(${id}.update)`; + block.event_listeners.push( + x`@destroy_action(${id} = ${fn}.call(null, ${target}, ${snippet}))` + ); - if (dependencies.length > 0) { - condition = x`${condition} && ${block.renderer.dirty(dependencies)}`; - } + if (dependencies && dependencies.length > 0) { + let condition = x`${id} && @is_function(${id}.update)`; - block.chunks.update.push( - b`if (${condition}) ${id}.update.call(null, ${snippet});` - ); + if (dependencies.length > 0) { + condition = x`${condition} && ${block.renderer.dirty(dependencies)}`; } - block.chunks.destroy.push( - b`if (${id} && @is_function(${id}.destroy)) ${id}.destroy();` + block.chunks.update.push( + b`if (${condition}) ${id}.update.call(null, ${snippet});` ); - }); + } } diff --git a/src/compiler/compile/render_dom/wrappers/shared/add_event_handlers.ts b/src/compiler/compile/render_dom/wrappers/shared/add_event_handlers.ts index ca4f5a24c795..23a37715ccce 100644 --- a/src/compiler/compile/render_dom/wrappers/shared/add_event_handlers.ts +++ b/src/compiler/compile/render_dom/wrappers/shared/add_event_handlers.ts @@ -6,5 +6,13 @@ export default function add_event_handlers( target: string, handlers: EventHandler[] ) { - handlers.forEach(handler => handler.render(block, target)); + handlers.forEach(handler => add_event_handler(block, target, handler)); +} + +export function add_event_handler( + block: Block, + target: string, + handler: EventHandler +) { + handler.render(block, target); } diff --git a/src/runtime/internal/utils.ts b/src/runtime/internal/utils.ts index b844f1dd4c20..227454039cfc 100644 --- a/src/runtime/internal/utils.ts +++ b/src/runtime/internal/utils.ts @@ -120,4 +120,12 @@ export function set_store_value(store, ret, value = ret) { return ret; } -export const has_prop = (obj, prop) => Object.prototype.hasOwnProperty.call(obj, prop); \ No newline at end of file +export const has_prop = (obj, prop) => Object.prototype.hasOwnProperty.call(obj, prop); + +export function destroy_action(action_result) { + return () => { + if (action_result && is_function(action_result.destroy)) { + return action_result.destroy(); + } + }; +} \ No newline at end of file diff --git a/test/js/samples/action-custom-event-handler/expected.js b/test/js/samples/action-custom-event-handler/expected.js index 6c3a2a5b0b30..0f5d85804cd7 100644 --- a/test/js/samples/action-custom-event-handler/expected.js +++ b/test/js/samples/action-custom-event-handler/expected.js @@ -1,6 +1,7 @@ /* generated by Svelte vX.Y.Z */ import { SvelteComponent, + destroy_action, detach, element, init, @@ -13,24 +14,25 @@ import { function create_fragment(ctx) { let button; let foo_action; + let dispose; return { c() { button = element("button"); button.textContent = "foo"; + dispose = destroy_action(foo_action = foo.call(null, button, /*foo_function*/ ctx[1])); }, m(target, anchor) { insert(target, button, anchor); - foo_action = foo.call(null, button, /*foo_function*/ ctx[1]) || ({}); }, p(ctx, [dirty]) { - if (is_function(foo_action.update) && dirty & /*bar*/ 1) foo_action.update.call(null, /*foo_function*/ ctx[1]); + if (foo_action && is_function(foo_action.update) && dirty & /*bar*/ 1) foo_action.update.call(null, /*foo_function*/ ctx[1]); }, i: noop, o: noop, d(detaching) { if (detaching) detach(button); - if (foo_action && is_function(foo_action.destroy)) foo_action.destroy(); + dispose(); } }; } diff --git a/test/js/samples/action/expected.js b/test/js/samples/action/expected.js index 78fc4855c624..fe5d0824a0bb 100644 --- a/test/js/samples/action/expected.js +++ b/test/js/samples/action/expected.js @@ -2,11 +2,11 @@ import { SvelteComponent, attr, + destroy_action, detach, element, init, insert, - is_function, noop, safe_not_equal } from "svelte/internal"; @@ -14,23 +14,24 @@ import { function create_fragment(ctx) { let a; let link_action; + let dispose; return { c() { a = element("a"); a.textContent = "Test"; attr(a, "href", "#"); + dispose = destroy_action(link_action = link.call(null, a)); }, m(target, anchor) { insert(target, a, anchor); - link_action = link.call(null, a) || ({}); }, p: noop, i: noop, o: noop, d(detaching) { if (detaching) detach(a); - if (link_action && is_function(link_action.destroy)) link_action.destroy(); + dispose(); } }; } diff --git a/test/runtime/samples/apply-directives-in-order-2/_config.js b/test/runtime/samples/apply-directives-in-order-2/_config.js new file mode 100644 index 000000000000..a74ce41cb66f --- /dev/null +++ b/test/runtime/samples/apply-directives-in-order-2/_config.js @@ -0,0 +1,38 @@ +const value = []; +export default { + props: { + value, + }, + + async test({ assert, component, target, window }) { + const inputs = target.querySelectorAll('input'); + + const event = new window.Event('input'); + + for (const input of inputs) { + input.value = 'h'; + await input.dispatchEvent(event); + } + + assert.deepEqual(value, [ + '1', + '2', + '3', + '4', + '5', + '6', + '7', + '8', + '9', + '10', + '11', + '12', + '13', + '14', + '15', + '16', + '17', + '18', + ]); + }, +}; diff --git a/test/runtime/samples/apply-directives-in-order-2/main.svelte b/test/runtime/samples/apply-directives-in-order-2/main.svelte new file mode 100644 index 000000000000..e91c4b6a1497 --- /dev/null +++ b/test/runtime/samples/apply-directives-in-order-2/main.svelte @@ -0,0 +1,34 @@ + + + + + + + + From 218d23782c11d98d53be74ea3f030521d2d3bb62 Mon Sep 17 00:00:00 2001 From: Conduitry Date: Tue, 24 Dec 2019 10:24:23 -0500 Subject: [PATCH 2/2] tidy --- .../compile/render_dom/wrappers/shared/add_actions.ts | 2 +- src/runtime/internal/utils.ts | 8 ++------ test/js/samples/action-custom-event-handler/expected.js | 6 +++--- test/js/samples/action/expected.js | 4 ++-- 4 files changed, 8 insertions(+), 12 deletions(-) diff --git a/src/compiler/compile/render_dom/wrappers/shared/add_actions.ts b/src/compiler/compile/render_dom/wrappers/shared/add_actions.ts index 9b5ce7a1475d..3d8c4fbbc0a3 100644 --- a/src/compiler/compile/render_dom/wrappers/shared/add_actions.ts +++ b/src/compiler/compile/render_dom/wrappers/shared/add_actions.ts @@ -29,7 +29,7 @@ export function add_action(block: Block, target: string, action: Action) { const fn = block.renderer.reference(action.name); block.event_listeners.push( - x`@destroy_action(${id} = ${fn}.call(null, ${target}, ${snippet}))` + x`@action_destroyer(${id} = ${fn}.call(null, ${target}, ${snippet}))` ); if (dependencies && dependencies.length > 0) { diff --git a/src/runtime/internal/utils.ts b/src/runtime/internal/utils.ts index 227454039cfc..c94a135869f2 100644 --- a/src/runtime/internal/utils.ts +++ b/src/runtime/internal/utils.ts @@ -122,10 +122,6 @@ export function set_store_value(store, ret, value = ret) { export const has_prop = (obj, prop) => Object.prototype.hasOwnProperty.call(obj, prop); -export function destroy_action(action_result) { - return () => { - if (action_result && is_function(action_result.destroy)) { - return action_result.destroy(); - } - }; +export function action_destroyer(action_result) { + return action_result && is_function(action_result.destroy) ? action_result.destroy : noop; } \ No newline at end of file diff --git a/test/js/samples/action-custom-event-handler/expected.js b/test/js/samples/action-custom-event-handler/expected.js index 0f5d85804cd7..da42603895c9 100644 --- a/test/js/samples/action-custom-event-handler/expected.js +++ b/test/js/samples/action-custom-event-handler/expected.js @@ -1,7 +1,7 @@ /* generated by Svelte vX.Y.Z */ import { SvelteComponent, - destroy_action, + action_destroyer, detach, element, init, @@ -20,7 +20,7 @@ function create_fragment(ctx) { c() { button = element("button"); button.textContent = "foo"; - dispose = destroy_action(foo_action = foo.call(null, button, /*foo_function*/ ctx[1])); + dispose = action_destroyer(foo_action = foo.call(null, button, /*foo_function*/ ctx[1])); }, m(target, anchor) { insert(target, button, anchor); @@ -42,7 +42,7 @@ function handleFoo(bar) { } function foo(node, callback) { - + } function instance($$self, $$props, $$invalidate) { diff --git a/test/js/samples/action/expected.js b/test/js/samples/action/expected.js index fe5d0824a0bb..dc3ebb5cf820 100644 --- a/test/js/samples/action/expected.js +++ b/test/js/samples/action/expected.js @@ -1,8 +1,8 @@ /* generated by Svelte vX.Y.Z */ import { SvelteComponent, + action_destroyer, attr, - destroy_action, detach, element, init, @@ -21,7 +21,7 @@ function create_fragment(ctx) { a = element("a"); a.textContent = "Test"; attr(a, "href", "#"); - dispose = destroy_action(link_action = link.call(null, a)); + dispose = action_destroyer(link_action = link.call(null, a)); }, m(target, anchor) { insert(target, a, anchor);