Merge pull request #1405 from sveltejs/gh-1399-alt

alternative approach to #1399

store.js

@@ -49,29 +49,30 @@ assign(Store.prototype, {
 	_sortComputedProperties: function() {
 		var computed = this._computed;
 		var sorted = this._sortedComputedProperties = [];
-		var cycles;
 		var visited = blankObject();
+		var currentKey;
 
 		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);
+				c.deps.forEach(dep => {
+					if (dep === currentKey) {
+						throw new Error(`Cyclical dependency detected between ${dep} <-> ${key}`);
+					}
+
+					visit(dep);
+				});
+
+				if (!visited[key]) {
+					visited[key] = true;
+					sorted.push(c);
+				}
 			}
 		}
 
 		for (var key in this._computed) {
-			cycles = blankObject();
-			visit(key);
+			visit(currentKey = key);
 		}
 	},
 

test/store/index.js

@@ -141,8 +141,27 @@ describe('store', () => {
 
 			assert.throws(() => {
 				store.compute('a', ['b'], b => b + 1);
-				store.compute('b', ['a'], a => a + 1);
-			}, /Cyclical dependency detected/);
+				store.compute('b', ['c'], c => c + 1);
+				store.compute('c', ['a'], a => a + 1);
+			}, /Cyclical dependency detected between a <-> c/);
+		});
+
+		it('does not falsely report cycles', () => {
+			const store = new Store();
+
+			store.compute('dep4', ['dep1', 'dep2', 'dep3'], (...args) => ['dep4'].concat(...args));
+			store.compute('dep1', ['source'], (...args) => ['dep1'].concat(...args));
+			store.compute('dep2', ['dep1'], (...args) => ['dep2'].concat(...args));
+			store.compute('dep3', ['dep1', 'dep2'], (...args) => ['dep3'].concat(...args));
+			store.set({source: 'source'});
+
+			assert.deepEqual(store.get().dep4, [
+				'dep4',
+				'dep1', 'source',
+				'dep2', 'dep1', 'source',
+				'dep3', 'dep1', 'source',
+				'dep2', 'dep1', 'source'
+			]);
 		});
 	});