Merge pull request #964 from sveltejs/prevent-cyclical-store-computations

prevent cyclical store computations, and computation duplication

store.js

@@ -48,19 +48,30 @@ assign(Store.prototype, {
 	_sortComputedProperties: function() {
 		var computed = this._computed;
 		var sorted = this._sortedComputedProperties = [];
+		var cycles;
 		var visited = blankObject();
 
 		function visit(key) {
+			if (cycles[key]) {
+				throw new Error('Cyclical dependency detected');
+			}
+
 			if (visited[key]) return;
+			visited[key] = true;
+
 			var c = computed[key];
 
 			if (c) {
+				cycles[key] = true;
 				c.deps.forEach(visit);
 				sorted.push(c);
 			}
 		}
 
-		for (var key in this._computed) visit(key);
+		for (var key in this._computed) {
+			cycles = blankObject();
+			visit(key);
+		}
 	},
 
 	compute: function(key, deps, fn) {

test/store/index.js

@@ -1,7 +1,27 @@
+import fs from 'fs';
 import assert from 'assert';
-import { Store, combineStores } from '../../store.js';
+import MagicString from 'magic-string';
+import { parse } from 'acorn';
+import { Store } from '../../store.js';
 
 describe('store', () => {
+	it('is written in ES5', () => {
+		const source = fs.readFileSync('store.js', 'utf-8');
+
+		const ast = parse(source, {
+			sourceType: 'module'
+		});
+
+		const magicString = new MagicString(source);
+		ast.body.forEach(node => {
+			if (/^(Im|Ex)port/.test(node.type)) magicString.remove(node.start, node.end);
+		});
+
+		parse(magicString.toString(), {
+			ecmaVersion: 5
+		});
+	});
+
 	describe('get', () => {
 		it('gets a specific key', () => {
 			const store = new Store({
@@ -142,5 +162,29 @@ describe('store', () => {
 				store.set({ bar: 'whatever' });
 			}, /'bar' is a read-only property/);
 		});
+
+		it('allows multiple dependents to depend on the same computed property', () => {
+			const store = new Store({
+				a: 1
+			});
+
+			store.compute('b', ['a'], a => a * 2);
+			store.compute('c', ['b'], b => b * 3);
+			store.compute('d', ['b'], b => b * 4);
+
+			assert.deepEqual(store.get(), { a: 1, b: 2, c: 6, d: 8 });
+
+			// bit cheeky, testing a private property, but whatever
+			assert.equal(store._sortedComputedProperties.length, 3);
+		});
+
+		it('prevents cyclical dependencies', () => {
+			const store = new Store();
+
+			assert.throws(() => {
+				store.compute('a', ['b'], b => b + 1);
+				store.compute('b', ['a'], a => a + 1);
+			}, /Cyclical dependency detected/);
+		});
 	});
 });