From 875f49f71d96faf052eced6a4413f644f809500a Mon Sep 17 00:00:00 2001 From: Richard Harris Date: Sat, 9 Mar 2019 16:22:22 -0500 Subject: [PATCH 1/3] allow stores to work with mutable data - fixes #2171 --- store.mjs | 24 +++++++++++++----------- test/store/index.js | 19 ++++++++++++++++++- 2 files changed, 31 insertions(+), 12 deletions(-) diff --git a/store.mjs b/store.mjs index ba1b89a5c3..67906538c1 100644 --- a/store.mjs +++ b/store.mjs @@ -1,14 +1,15 @@ -import { run_all, noop, get_store_value } from './internal'; +import { run_all, noop, get_store_value, safe_not_equal } from './internal'; export function readable(start, value) { const subscribers = []; let stop; - function set(newValue) { - if (newValue === value) return; - value = newValue; - subscribers.forEach(s => s[1]()); - subscribers.forEach(s => s[0](value)); + function set(new_value) { + if (safe_not_equal(value, new_value)) { + value = new_value; + subscribers.forEach(s => s[1]()); + subscribers.forEach(s => s[0](value)); + } } return { @@ -38,11 +39,12 @@ export function writable(value, start = noop) { let stop; const subscribers = []; - function set(newValue) { - if (newValue === value) return; - value = newValue; - subscribers.forEach(s => s[1]()); - subscribers.forEach(s => s[0](value)); + function set(new_value) { + if (safe_not_equal(value, new_value)) { + value = new_value; + subscribers.forEach(s => s[1]()); + subscribers.forEach(s => s[0](value)); + } } function update(fn) { diff --git a/test/store/index.js b/test/store/index.js index 0bf3ee93e0..69bcebe843 100644 --- a/test/store/index.js +++ b/test/store/index.js @@ -1,7 +1,7 @@ import * as assert from 'assert'; import { readable, writable, derive, get } from '../../store.js'; -describe('store', () => { +describe.only('store', () => { describe('writable', () => { it('creates a writable store', () => { const count = writable(0); @@ -42,6 +42,23 @@ describe('store', () => { unsubscribe2(); assert.equal(called, 0); }); + + it('does not assume immutable data', () => { + const obj = {}; + let called = 0; + + const store = writable(obj); + + store.subscribe(value => { + called += 1; + }); + + store.set(obj); + assert.equal(called, 2); + + store.update(obj => obj); + assert.equal(called, 3); + }); }); describe('readable', () => { From 4e322c7fe16826a94cdffb3d695b5f271d8f2233 Mon Sep 17 00:00:00 2001 From: Richard Harris Date: Sat, 9 Mar 2019 16:34:30 -0500 Subject: [PATCH 2/3] express readable in terms of writable --- store.mjs | 35 +++-------------------------------- test/store/index.js | 2 +- 2 files changed, 4 insertions(+), 33 deletions(-) diff --git a/store.mjs b/store.mjs index 67906538c1..faafe31575 100644 --- a/store.mjs +++ b/store.mjs @@ -1,38 +1,8 @@ import { run_all, noop, get_store_value, safe_not_equal } from './internal'; export function readable(start, value) { - const subscribers = []; - let stop; - - function set(new_value) { - if (safe_not_equal(value, new_value)) { - value = new_value; - subscribers.forEach(s => s[1]()); - subscribers.forEach(s => s[0](value)); - } - } - - return { - subscribe(run, invalidate = noop) { - if (subscribers.length === 0) { - stop = start(set); - } - - const subscriber = [run, invalidate]; - subscribers.push(subscriber); - run(value); - - return function() { - const index = subscribers.indexOf(subscriber); - if (index !== -1) subscribers.splice(index, 1); - - if (subscribers.length === 0) { - stop && stop(); - stop = null; - } - }; - } - }; + const { set, subscribe } = writable(value, () => start(set)); + return { subscribe }; } export function writable(value, start = noop) { @@ -42,6 +12,7 @@ export function writable(value, start = noop) { function set(new_value) { if (safe_not_equal(value, new_value)) { value = new_value; + if (!stop) return; // not ready subscribers.forEach(s => s[1]()); subscribers.forEach(s => s[0](value)); } diff --git a/test/store/index.js b/test/store/index.js index 69bcebe843..f993c3a253 100644 --- a/test/store/index.js +++ b/test/store/index.js @@ -1,7 +1,7 @@ import * as assert from 'assert'; import { readable, writable, derive, get } from '../../store.js'; -describe.only('store', () => { +describe('store', () => { describe('writable', () => { it('creates a writable store', () => { const count = writable(0); From 6bb95267110a53df55185f0822eed97cf4301277 Mon Sep 17 00:00:00 2001 From: Richard Harris Date: Sat, 9 Mar 2019 16:57:02 -0500 Subject: [PATCH 3/3] deep store bindings --- .../render-dom/wrappers/Element/Binding.ts | 28 ++++++------- .../samples/binding-store-deep/_config.js | 42 +++++++++++++++++-- 2 files changed, 51 insertions(+), 19 deletions(-) diff --git a/src/compile/render-dom/wrappers/Element/Binding.ts b/src/compile/render-dom/wrappers/Element/Binding.ts index 0dd41b6e62..55345247ca 100644 --- a/src/compile/render-dom/wrappers/Element/Binding.ts +++ b/src/compile/render-dom/wrappers/Element/Binding.ts @@ -213,6 +213,12 @@ function getBindingGroup(renderer: Renderer, value: Node) { return index; } +function mutate_store(store, value, tail) { + return tail + ? `${store}.update($$value => ($$value${tail} = ${value}, $$value));` + : `${store}.set(${value});`; +} + function getEventHandler( binding: BindingWrapper, renderer: Renderer, @@ -223,36 +229,26 @@ function getEventHandler( const value = getValueFromDom(renderer, binding.parent, binding); const store = binding.object[0] === '$' ? binding.object.slice(1) : null; - if (store && binding.node.expression.node.type === 'MemberExpression') { - // TODO is there a way around this? Mutating an object doesn't work, - // because stores check for referential equality (i.e. immutable data). - // But we can't safely or easily clone objects. So for now, we bail - renderer.component.error(binding.node.expression.node.property, { - code: 'invalid-store-binding', - message: 'Cannot bind to a nested property of a store' - }); + let tail = ''; + if (binding.node.expression.node.type === 'MemberExpression') { + const { start, end } = get_tail(binding.node.expression.node); + tail = renderer.component.source.slice(start, end); } if (binding.node.isContextual) { - let tail = ''; - if (binding.node.expression.node.type === 'MemberExpression') { - const { start, end } = get_tail(binding.node.expression.node); - tail = renderer.component.source.slice(start, end); - } - const { object, property, snippet } = block.bindings.get(name); return { usesContext: true, mutation: store - ? `${store}.set(${value});` + ? mutate_store(store, value, tail) : `${snippet}${tail} = ${value};`, contextual_dependencies: new Set([object, property]) }; } const mutation = store - ? `${store}.set(${value});` + ? mutate_store(store, value, tail) : `${snippet} = ${value};`; if (binding.node.expression.node.type === 'MemberExpression') { diff --git a/test/runtime/samples/binding-store-deep/_config.js b/test/runtime/samples/binding-store-deep/_config.js index e43df6cfae..8bdd41818b 100644 --- a/test/runtime/samples/binding-store-deep/_config.js +++ b/test/runtime/samples/binding-store-deep/_config.js @@ -1,5 +1,41 @@ export default { - error(assert, err) { - assert.equal(err.message, `Cannot bind to a nested property of a store`); - } + html: ` + +

hello world

+ `, + + ssrHtml: ` + +

hello world

+ `, + + async test({ assert, component, target, window }) { + const input = target.querySelector('input'); + assert.equal(input.value, 'world'); + + const event = new window.Event('input'); + + const names = []; + const unsubscribe = component.user.subscribe(user => { + names.push(user.name); + }); + + input.value = 'everybody'; + await input.dispatchEvent(event); + + assert.htmlEqual(target.innerHTML, ` + +

hello everybody

+ `); + + await component.user.set({ name: 'goodbye' }); + assert.equal(input.value, 'goodbye'); + assert.htmlEqual(target.innerHTML, ` + +

hello goodbye

+ `); + + assert.deepEqual(names, ['world', 'everybody', 'goodbye']); + unsubscribe(); + }, };