Merge pull request #2190 from sveltejs/gh-2171

Stores accept mutable data

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') {

store.mjs

@@ -1,48 +1,21 @@
-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));
-	}
-
-	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) {
 	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;
+			if (!stop) return; // not ready
+			subscribers.forEach(s => s[1]());
+			subscribers.forEach(s => s[0](value));
+		}
 	}
 
 	function update(fn) {

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: `
+		<input>
+		<p>hello world</p>
+	`,
+
+	ssrHtml: `
+		<input value="world">
+		<p>hello world</p>
+	`,
+
+	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, `
+			<input>
+			<p>hello everybody</p>
+		`);
+
+		await component.user.set({ name: 'goodbye' });
+		assert.equal(input.value, 'goodbye');
+		assert.htmlEqual(target.innerHTML, `
+			<input>
+			<p>hello goodbye</p>
+		`);
+
+		assert.deepEqual(names, ['world', 'everybody', 'goodbye']);
+		unsubscribe();
+	},
 };

test/store/index.js

@@ -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', () => {