From ce704677078c48b41d9e03efc47e6696d1215685 Mon Sep 17 00:00:00 2001 From: Richard Harris Date: Sun, 14 Apr 2019 12:29:30 -0400 Subject: [PATCH 1/4] failing tests for #2356 --- .../component-binding-blowback-d/One.svelte | 17 +++++++++++++ .../component-binding-blowback-d/Two.svelte | 3 +++ .../component-binding-blowback-d/_config.js | 23 +++++++++++++++++ .../component-binding-blowback-d/main.svelte | 14 +++++++++++ .../component-binding-blowback-e/One.svelte | 17 +++++++++++++ .../component-binding-blowback-e/Two.svelte | 3 +++ .../component-binding-blowback-e/_config.js | 23 +++++++++++++++++ .../component-binding-blowback-e/main.svelte | 14 +++++++++++ .../component-binding-blowback-f/One.svelte | 17 +++++++++++++ .../component-binding-blowback-f/Two.svelte | 9 +++++++ .../component-binding-blowback-f/_config.js | 25 +++++++++++++++++++ .../component-binding-blowback-f/main.svelte | 14 +++++++++++ 12 files changed, 179 insertions(+) create mode 100644 test/runtime/samples/component-binding-blowback-d/One.svelte create mode 100644 test/runtime/samples/component-binding-blowback-d/Two.svelte create mode 100644 test/runtime/samples/component-binding-blowback-d/_config.js create mode 100644 test/runtime/samples/component-binding-blowback-d/main.svelte create mode 100644 test/runtime/samples/component-binding-blowback-e/One.svelte create mode 100644 test/runtime/samples/component-binding-blowback-e/Two.svelte create mode 100644 test/runtime/samples/component-binding-blowback-e/_config.js create mode 100644 test/runtime/samples/component-binding-blowback-e/main.svelte create mode 100644 test/runtime/samples/component-binding-blowback-f/One.svelte create mode 100644 test/runtime/samples/component-binding-blowback-f/Two.svelte create mode 100644 test/runtime/samples/component-binding-blowback-f/_config.js create mode 100644 test/runtime/samples/component-binding-blowback-f/main.svelte diff --git a/test/runtime/samples/component-binding-blowback-d/One.svelte b/test/runtime/samples/component-binding-blowback-d/One.svelte new file mode 100644 index 0000000000..e2e86b1bf7 --- /dev/null +++ b/test/runtime/samples/component-binding-blowback-d/One.svelte @@ -0,0 +1,17 @@ + + +{#each list as item} + +{/each} + + \ No newline at end of file diff --git a/test/runtime/samples/component-binding-blowback-d/Two.svelte b/test/runtime/samples/component-binding-blowback-d/Two.svelte new file mode 100644 index 0000000000..4f55f14124 --- /dev/null +++ b/test/runtime/samples/component-binding-blowback-d/Two.svelte @@ -0,0 +1,3 @@ + \ No newline at end of file diff --git a/test/runtime/samples/component-binding-blowback-d/_config.js b/test/runtime/samples/component-binding-blowback-d/_config.js new file mode 100644 index 0000000000..fe1164f100 --- /dev/null +++ b/test/runtime/samples/component-binding-blowback-d/_config.js @@ -0,0 +1,23 @@ +export default { + html: ` + + + +

{"value":"x"}

+

+ `, + + async test({ assert, target, window }) { + const button = target.querySelectorAll('button')[1]; + + await button.dispatchEvent(new window.Event('click')); + + assert.htmlEqual(target.innerHTML, ` + + + +

{"value":"x"}

+

{"value":"x"}

+ `); + } +}; diff --git a/test/runtime/samples/component-binding-blowback-d/main.svelte b/test/runtime/samples/component-binding-blowback-d/main.svelte new file mode 100644 index 0000000000..5690b33813 --- /dev/null +++ b/test/runtime/samples/component-binding-blowback-d/main.svelte @@ -0,0 +1,14 @@ + + + + + +

{obj.a.map(JSON.stringify)}

+

{obj.b.map(JSON.stringify)}

\ No newline at end of file diff --git a/test/runtime/samples/component-binding-blowback-e/One.svelte b/test/runtime/samples/component-binding-blowback-e/One.svelte new file mode 100644 index 0000000000..e2e86b1bf7 --- /dev/null +++ b/test/runtime/samples/component-binding-blowback-e/One.svelte @@ -0,0 +1,17 @@ + + +{#each list as item} + +{/each} + + \ No newline at end of file diff --git a/test/runtime/samples/component-binding-blowback-e/Two.svelte b/test/runtime/samples/component-binding-blowback-e/Two.svelte new file mode 100644 index 0000000000..9bc34c321f --- /dev/null +++ b/test/runtime/samples/component-binding-blowback-e/Two.svelte @@ -0,0 +1,3 @@ + \ No newline at end of file diff --git a/test/runtime/samples/component-binding-blowback-e/_config.js b/test/runtime/samples/component-binding-blowback-e/_config.js new file mode 100644 index 0000000000..37f687a4f6 --- /dev/null +++ b/test/runtime/samples/component-binding-blowback-e/_config.js @@ -0,0 +1,23 @@ +export default { + html: ` + + + +

{"value":{"x":true}}

+

+ `, + + async test({ assert, target, window }) { + const button = target.querySelectorAll('button')[1]; + + await button.dispatchEvent(new window.Event('click')); + + assert.htmlEqual(target.innerHTML, ` + + + +

{"value":{"x":true}}

+

{"value":{"x":true}}

+ `); + } +}; diff --git a/test/runtime/samples/component-binding-blowback-e/main.svelte b/test/runtime/samples/component-binding-blowback-e/main.svelte new file mode 100644 index 0000000000..5690b33813 --- /dev/null +++ b/test/runtime/samples/component-binding-blowback-e/main.svelte @@ -0,0 +1,14 @@ + + + + + +

{obj.a.map(JSON.stringify)}

+

{obj.b.map(JSON.stringify)}

\ No newline at end of file diff --git a/test/runtime/samples/component-binding-blowback-f/One.svelte b/test/runtime/samples/component-binding-blowback-f/One.svelte new file mode 100644 index 0000000000..e2e86b1bf7 --- /dev/null +++ b/test/runtime/samples/component-binding-blowback-f/One.svelte @@ -0,0 +1,17 @@ + + +{#each list as item} + +{/each} + + \ No newline at end of file diff --git a/test/runtime/samples/component-binding-blowback-f/Two.svelte b/test/runtime/samples/component-binding-blowback-f/Two.svelte new file mode 100644 index 0000000000..28d3ae830f --- /dev/null +++ b/test/runtime/samples/component-binding-blowback-f/Two.svelte @@ -0,0 +1,9 @@ + \ No newline at end of file diff --git a/test/runtime/samples/component-binding-blowback-f/_config.js b/test/runtime/samples/component-binding-blowback-f/_config.js new file mode 100644 index 0000000000..d73f3e3943 --- /dev/null +++ b/test/runtime/samples/component-binding-blowback-f/_config.js @@ -0,0 +1,25 @@ +export default { + skip: 1, + + html: ` + + + +

{"value":{"x":true}}

+

+ `, + + async test({ assert, target, window }) { + const button = target.querySelectorAll('button')[1]; + + await button.dispatchEvent(new window.Event('click')); + + assert.htmlEqual(target.innerHTML, ` + + + +

{"value":{"x":true}}

+

{"value":{"x":true}}

+ `); + } +}; diff --git a/test/runtime/samples/component-binding-blowback-f/main.svelte b/test/runtime/samples/component-binding-blowback-f/main.svelte new file mode 100644 index 0000000000..5690b33813 --- /dev/null +++ b/test/runtime/samples/component-binding-blowback-f/main.svelte @@ -0,0 +1,14 @@ + + + + + +

{obj.a.map(JSON.stringify)}

+

{obj.b.map(JSON.stringify)}

\ No newline at end of file From b03cfcf09d6c2a7a4af7984ca90c7479df7369f5 Mon Sep 17 00:00:00 2001 From: Richard Harris Date: Sun, 14 Apr 2019 12:30:14 -0400 Subject: [PATCH 2/4] only fire binding callbacks when values change --- src/internal/Component.js | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/src/internal/Component.js b/src/internal/Component.js index 4a7293ddae..a0be4b9895 100644 --- a/src/internal/Component.js +++ b/src/internal/Component.js @@ -85,11 +85,10 @@ export function init(component, options, instance, create_fragment, not_equal, p $$.ctx = instance ? instance(component, props, (key, value) => { - if ($$.bound[key]) $$.bound[key](value); - if ($$.ctx) { const changed = not_equal(value, $$.ctx[key]); if (ready && changed) { + if ($$.bound[key]) $$.bound[key](value); make_dirty(component, key); } From 5535110066bd04cdc631fc46242f45037bf02354 Mon Sep 17 00:00:00 2001 From: Richard Harris Date: Sun, 14 Apr 2019 14:20:18 -0400 Subject: [PATCH 3/4] wait until flush complete before unlocking bindings - fixes #2356 --- .../wrappers/InlineComponent/index.ts | 20 +++++++------------ src/internal/Component.js | 12 +++-------- src/internal/scheduler.js | 17 ++++++++++++---- .../component-binding-blowback-d/One.svelte | 5 +++-- .../component-binding-blowback-d/Two.svelte | 3 ++- .../component-binding-blowback-d/_config.js | 6 +++--- .../component-binding-blowback-d/main.svelte | 4 ++-- .../component-binding-blowback-e/One.svelte | 5 +++-- .../component-binding-blowback-e/Two.svelte | 3 ++- .../component-binding-blowback-e/_config.js | 6 +++--- .../component-binding-blowback-e/main.svelte | 4 ++-- .../component-binding-blowback-f/One.svelte | 5 +++-- .../component-binding-blowback-f/Two.svelte | 3 ++- .../component-binding-blowback-f/_config.js | 16 ++++++++++----- .../component-binding-blowback-f/main.svelte | 4 ++-- 15 files changed, 61 insertions(+), 52 deletions(-) diff --git a/src/compile/render-dom/wrappers/InlineComponent/index.ts b/src/compile/render-dom/wrappers/InlineComponent/index.ts index bc13014a29..870296a035 100644 --- a/src/compile/render-dom/wrappers/InlineComponent/index.ts +++ b/src/compile/render-dom/wrappers/InlineComponent/index.ts @@ -112,7 +112,6 @@ export default class InlineComponentWrapper extends Wrapper { const statements: string[] = []; const updates: string[] = []; - const postupdates: string[] = []; let props; const name_changes = block.get_unique_name(`${name}_changes`); @@ -311,8 +310,6 @@ export default class InlineComponentWrapper extends Wrapper { } `); - postupdates.push(updating); - const contextual_dependencies = Array.from(binding.expression.contextual_dependencies); const dependencies = Array.from(binding.expression.dependencies); @@ -333,9 +330,9 @@ export default class InlineComponentWrapper extends Wrapper { block.builders.init.add_block(deindent` function ${name}(value) { - if (ctx.${name}.call(null, value, ctx)) { - ${updating} = true; - } + ctx.${name}.call(null, value, ctx); + ${updating} = true; + @add_flush_callback(() => ${updating} = false); } `); @@ -343,9 +340,9 @@ export default class InlineComponentWrapper extends Wrapper { } else { block.builders.init.add_block(deindent` function ${name}(value) { - if (ctx.${name}.call(null, value)) { - ${updating} = true; - } + ctx.${name}.call(null, value); + ${updating} = true; + @add_flush_callback(() => ${updating} = false); } `); } @@ -353,7 +350,7 @@ export default class InlineComponentWrapper extends Wrapper { const body = deindent` function ${name}(${args.join(', ')}) { ${lhs} = value; - return ${component.invalidate(dependencies[0])} + ${component.invalidate(dependencies[0])}; } `; @@ -452,8 +449,6 @@ export default class InlineComponentWrapper extends Wrapper { else if (${switch_value}) { ${name}.$set(${name_changes}); } - - ${postupdates.length > 0 && `${postupdates.join(' = ')} = false;`} `); } @@ -497,7 +492,6 @@ export default class InlineComponentWrapper extends Wrapper { block.builders.update.add_block(deindent` ${updates} ${name}.$set(${name_changes}); - ${postupdates.length > 0 && `${postupdates.join(' = ')} = false;`} `); } diff --git a/src/internal/Component.js b/src/internal/Component.js index a0be4b9895..530a48da59 100644 --- a/src/internal/Component.js +++ b/src/internal/Component.js @@ -85,15 +85,9 @@ export function init(component, options, instance, create_fragment, not_equal, p $$.ctx = instance ? instance(component, props, (key, value) => { - if ($$.ctx) { - const changed = not_equal(value, $$.ctx[key]); - if (ready && changed) { - if ($$.bound[key]) $$.bound[key](value); - make_dirty(component, key); - } - - $$.ctx[key] = value; - return changed; + if ($$.ctx && not_equal($$.ctx[key], $$.ctx[key] = value)) { + if ($$.bound[key]) $$.bound[key](value); + if (ready) make_dirty(component, key); } }) : props; diff --git a/src/internal/scheduler.js b/src/internal/scheduler.js index b2b5bd5200..bc4f01197b 100644 --- a/src/internal/scheduler.js +++ b/src/internal/scheduler.js @@ -7,6 +7,7 @@ export const intros = { enabled: false }; let update_promise; const binding_callbacks = []; const render_callbacks = []; +const flush_callbacks = []; export function schedule_update() { if (!update_promise) { @@ -15,10 +16,6 @@ export function schedule_update() { } } -export function add_render_callback(fn) { - render_callbacks.push(fn); -} - export function tick() { schedule_update(); return update_promise; @@ -28,6 +25,14 @@ export function add_binding_callback(fn) { binding_callbacks.push(fn); } +export function add_render_callback(fn) { + render_callbacks.push(fn); +} + +export function add_flush_callback(fn) { + flush_callbacks.push(fn); +} + export function flush() { const seen_callbacks = new Set(); @@ -56,6 +61,10 @@ export function flush() { } } while (dirty_components.length); + while (flush_callbacks.length) { + flush_callbacks.pop()(); + } + update_promise = null; } diff --git a/test/runtime/samples/component-binding-blowback-d/One.svelte b/test/runtime/samples/component-binding-blowback-d/One.svelte index e2e86b1bf7..f86da72ff2 100644 --- a/test/runtime/samples/component-binding-blowback-d/One.svelte +++ b/test/runtime/samples/component-binding-blowback-d/One.svelte @@ -2,14 +2,15 @@ import Two from './Two.svelte'; export let list; + export let i; function handle_click() { list = [...list, {}]; } -{#each list as item} - +{#each list as item, j} + {/each} -

{"value":"x"}

+

{"value":"0:0"}

`, @@ -16,8 +16,8 @@ export default { -

{"value":"x"}

-

{"value":"x"}

+

{"value":"0:0"}

+

{"value":"1:0"}

`); } }; diff --git a/test/runtime/samples/component-binding-blowback-d/main.svelte b/test/runtime/samples/component-binding-blowback-d/main.svelte index 5690b33813..d6e9412618 100644 --- a/test/runtime/samples/component-binding-blowback-d/main.svelte +++ b/test/runtime/samples/component-binding-blowback-d/main.svelte @@ -7,8 +7,8 @@ }; - - + +

{obj.a.map(JSON.stringify)}

{obj.b.map(JSON.stringify)}

\ No newline at end of file diff --git a/test/runtime/samples/component-binding-blowback-e/One.svelte b/test/runtime/samples/component-binding-blowback-e/One.svelte index e2e86b1bf7..f86da72ff2 100644 --- a/test/runtime/samples/component-binding-blowback-e/One.svelte +++ b/test/runtime/samples/component-binding-blowback-e/One.svelte @@ -2,14 +2,15 @@ import Two from './Two.svelte'; export let list; + export let i; function handle_click() { list = [...list, {}]; } -{#each list as item} - +{#each list as item, j} + {/each} -

{"value":{"x":true}}

+

{"value":{"i":0,"j":0}}

`, @@ -16,8 +16,8 @@ export default { -

{"value":{"x":true}}

-

{"value":{"x":true}}

+

{"value":{"i":0,"j":0}}

+

{"value":{"i":1,"j":0}}

`); } }; diff --git a/test/runtime/samples/component-binding-blowback-e/main.svelte b/test/runtime/samples/component-binding-blowback-e/main.svelte index 5690b33813..d6e9412618 100644 --- a/test/runtime/samples/component-binding-blowback-e/main.svelte +++ b/test/runtime/samples/component-binding-blowback-e/main.svelte @@ -7,8 +7,8 @@ }; - - + +

{obj.a.map(JSON.stringify)}

{obj.b.map(JSON.stringify)}

\ No newline at end of file diff --git a/test/runtime/samples/component-binding-blowback-f/One.svelte b/test/runtime/samples/component-binding-blowback-f/One.svelte index e2e86b1bf7..f86da72ff2 100644 --- a/test/runtime/samples/component-binding-blowback-f/One.svelte +++ b/test/runtime/samples/component-binding-blowback-f/One.svelte @@ -2,14 +2,15 @@ import Two from './Two.svelte'; export let list; + export let i; function handle_click() { list = [...list, {}]; } -{#each list as item} - +{#each list as item, j} + {/each} -

{"value":{"x":true}}

+

{"value":{"i":0,"j":0}}

+

+ `, + + ssrHtml: ` + + + +

{}

`, @@ -18,8 +24,8 @@ export default { -

{"value":{"x":true}}

-

{"value":{"x":true}}

+

{"value":{"i":0,"j":0}}

+

{"value":{"i":1,"j":0}}

`); } }; diff --git a/test/runtime/samples/component-binding-blowback-f/main.svelte b/test/runtime/samples/component-binding-blowback-f/main.svelte index 5690b33813..d6e9412618 100644 --- a/test/runtime/samples/component-binding-blowback-f/main.svelte +++ b/test/runtime/samples/component-binding-blowback-f/main.svelte @@ -7,8 +7,8 @@ }; - - + +

{obj.a.map(JSON.stringify)}

{obj.b.map(JSON.stringify)}

\ No newline at end of file From 016078d76e6f5582ac6f299770ff85bdc2cbad64 Mon Sep 17 00:00:00 2001 From: Richard Harris Date: Sun, 14 Apr 2019 15:16:40 -0400 Subject: [PATCH 4/4] oops, messed up the merge --- src/compile/render-dom/wrappers/InlineComponent/index.ts | 1 - 1 file changed, 1 deletion(-) diff --git a/src/compile/render-dom/wrappers/InlineComponent/index.ts b/src/compile/render-dom/wrappers/InlineComponent/index.ts index 9a93034772..d36eafaf46 100644 --- a/src/compile/render-dom/wrappers/InlineComponent/index.ts +++ b/src/compile/render-dom/wrappers/InlineComponent/index.ts @@ -344,7 +344,6 @@ export default class InlineComponentWrapper extends Wrapper { ctx.${name}.call(null, ${value}); ${updating} = true; @add_flush_callback(() => ${updating} = false); - } } `); }