Skip to content

Commit d30077b

Browse files
divdavemdivdavem
committed
fix: fixing asymmetric diamond dependency problem
1 parent ae35b18 commit d30077b

File tree

2 files changed

+489
-20
lines changed

2 files changed

+489
-20
lines changed

‎src/index.spec.ts

Lines changed: 367 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -244,6 +244,34 @@ describe('stores', () => {
244244
unsubscribe();
245245
});
246246

247+
it('should not call again listeners when only revalidating', () => {
248+
class BasicStore extends Store<object> {
249+
public invalidate(): void {
250+
super.invalidate();
251+
}
252+
public revalidate(): void {
253+
super.revalidate();
254+
}
255+
public set(value: object): void {
256+
super.set(value);
257+
}
258+
}
259+
const initialValue = {};
260+
const newValue = {};
261+
const store = new BasicStore(initialValue);
262+
const calls: object[] = [];
263+
const unsubscribe = store.subscribe((v) => calls.push(v));
264+
expect(calls.length).toBe(1);
265+
expect(calls[0]).toBe(initialValue);
266+
store.invalidate();
267+
store.revalidate();
268+
expect(calls.length).toBe(1);
269+
store.set(newValue);
270+
expect(calls.length).toBe(2);
271+
expect(calls[1]).toBe(newValue);
272+
unsubscribe();
273+
});
274+
247275
it('should be compatible with rxjs "from" (writable)', () => {
248276
const store = writable(0);
249277
const observable = from(store);
@@ -664,6 +692,22 @@ describe('stores', () => {
664692
unsubscribe();
665693
});
666694

695+
it('should prevent the asymmetric diamond dependency problem', () => {
696+
const a = writable(0);
697+
const b = derived(a, (a) => `b${a}`);
698+
const c = derived([a, b], ([a, b]) => `${a}${b}`);
699+
700+
const values: string[] = [];
701+
702+
const unsubscribe = c.subscribe((value) => {
703+
values.push(value);
704+
});
705+
expect(values).toEqual(['0b0']);
706+
a.set(1);
707+
expect(values).toEqual(['0b0', '1b1']);
708+
unsubscribe();
709+
});
710+
667711
it('should be able to use destructuring', () => {
668712
const store = writable(0);
669713
const derivedStore = derived(store, (value) => {
@@ -835,6 +879,329 @@ describe('stores', () => {
835879
unsubscribe();
836880
});
837881

882+
it('should not call again listeners when the value finally did not change', () => {
883+
const a = writable(0);
884+
const values: number[] = [];
885+
const unsubscribe = a.subscribe((value) => {
886+
values.push(value);
887+
});
888+
expect(values).toEqual([0]);
889+
batch(() => {
890+
a.set(0);
891+
expect(get(a)).toEqual(0);
892+
a.set(1);
893+
expect(get(a)).toEqual(1);
894+
a.set(0);
895+
expect(get(a)).toEqual(0);
896+
});
897+
expect(values).toEqual([0]);
898+
expect(get(a)).toEqual(0);
899+
batch(() => {
900+
a.set(1);
901+
expect(get(a)).toEqual(1);
902+
a.set(0);
903+
expect(get(a)).toEqual(0);
904+
a.set(1);
905+
expect(get(a)).toEqual(1);
906+
expect(values).toEqual([0]);
907+
});
908+
expect(values).toEqual([0, 1]);
909+
expect(get(a)).toEqual(1);
910+
unsubscribe();
911+
});
912+
913+
it('should not call again listeners when the original value finally did not change with a derived', () => {
914+
const a = writable(0);
915+
const b = derived(a, (a) => a + 1);
916+
const values: number[] = [];
917+
const unsubscribe = b.subscribe((value) => {
918+
values.push(value);
919+
});
920+
expect(values).toEqual([1]);
921+
batch(() => {
922+
a.set(0);
923+
expect(get(b)).toEqual(1);
924+
a.set(1);
925+
expect(get(b)).toEqual(1); // batch prevents the current value from being computed
926+
a.set(0);
927+
expect(get(b)).toEqual(1);
928+
});
929+
expect(values).toEqual([1]);
930+
expect(get(b)).toEqual(1);
931+
batch(() => {
932+
a.set(1);
933+
expect(get(b)).toEqual(1); // batch prevents the current value from being computed
934+
a.set(0);
935+
expect(get(b)).toEqual(1);
936+
a.set(1);
937+
expect(get(b)).toEqual(1); // batch prevents the current value from being computed
938+
expect(values).toEqual([1]);
939+
});
940+
expect(values).toEqual([1, 2]);
941+
expect(get(b)).toEqual(2);
942+
unsubscribe();
943+
});
944+
945+
it('should not call again listeners when the derived value finally did not change', () => {
946+
const a = writable(0);
947+
const isEven = derived(a, (a) => a % 2 === 0);
948+
const values: boolean[] = [];
949+
const unsubscribe = isEven.subscribe((value) => {
950+
values.push(value);
951+
});
952+
expect(values).toEqual([true]);
953+
expect(get(isEven)).toEqual(true);
954+
batch(() => {
955+
a.set(0); // isEven = true
956+
expect(get(isEven)).toEqual(true);
957+
a.set(1); // isEven = false (without batch)
958+
expect(get(isEven)).toEqual(true); // batch prevents the current value from being computed
959+
a.set(2); // isEven = true again
960+
expect(get(isEven)).toEqual(true);
961+
});
962+
expect(values).toEqual([true]);
963+
expect(get(isEven)).toEqual(true);
964+
batch(() => {
965+
a.set(3); // isEven = false (without batch)
966+
expect(get(isEven)).toEqual(true); // batch prevents the current value from being computed
967+
a.set(4); // isEven = true
968+
expect(get(isEven)).toEqual(true);
969+
a.set(5); // isEven = false
970+
expect(get(isEven)).toEqual(true);
971+
expect(values).toEqual([true]);
972+
});
973+
expect(values).toEqual([true, false]);
974+
expect(get(isEven)).toEqual(false);
975+
unsubscribe();
976+
});
977+
978+
it('should work when first subscribing to a store inside batch', () => {
979+
const a = writable(0);
980+
const values: number[] = [];
981+
let unsubscribe!: () => void;
982+
batch(() => {
983+
a.set(1);
984+
unsubscribe = a.subscribe((v) => values.push(v));
985+
expect(values).toEqual([1]);
986+
});
987+
expect(values).toEqual([1]);
988+
a.set(2);
989+
expect(values).toEqual([1, 2]);
990+
unsubscribe();
991+
});
992+
993+
it('should work when doing a second subscription to a store inside batch', () => {
994+
const a = writable(0);
995+
const values1: number[] = [];
996+
const values2: number[] = [];
997+
const unsubscribe1 = a.subscribe((v) => values1.push(v));
998+
let unsubscribe2!: () => void;
999+
expect(values1).toEqual([0]);
1000+
batch(() => {
1001+
a.set(1);
1002+
unsubscribe2 = a.subscribe((v) => values2.push(v));
1003+
expect(values1).toEqual([0]);
1004+
expect(values2).toEqual([1]);
1005+
});
1006+
expect(values1).toEqual([0, 1]);
1007+
expect(values2).toEqual([1]);
1008+
a.set(2);
1009+
expect(values1).toEqual([0, 1, 2]);
1010+
expect(values2).toEqual([1, 2]);
1011+
unsubscribe1();
1012+
unsubscribe2();
1013+
});
1014+
1015+
it('should work when first subscribing to a derived store inside batch', () => {
1016+
const a = writable(0);
1017+
const b = derived(a, (a) => a + 1);
1018+
const values: number[] = [];
1019+
let unsubscribe!: () => void;
1020+
batch(() => {
1021+
a.set(1);
1022+
unsubscribe = b.subscribe((v) => values.push(v));
1023+
expect(values).toEqual([2]);
1024+
});
1025+
expect(values).toEqual([2]);
1026+
a.set(2);
1027+
expect(values).toEqual([2, 3]);
1028+
unsubscribe();
1029+
});
1030+
1031+
it('should work when doing a second subscription to a derived store inside batch', () => {
1032+
const a = writable(0);
1033+
const b = derived(a, (a) => a + 1);
1034+
const values1: number[] = [];
1035+
const values2: number[] = [];
1036+
const unsubscribe1 = b.subscribe((v) => values1.push(v));
1037+
expect(values1).toEqual([1]);
1038+
let unsubscribe2!: () => void;
1039+
batch(() => {
1040+
a.set(1);
1041+
unsubscribe2 = b.subscribe((v) => values2.push(v));
1042+
expect(values1).toEqual([1]);
1043+
expect(values2).toEqual([1]); // gets the previous value of store b to avoid extra computation before the end of batch
1044+
});
1045+
expect(values1).toEqual([1, 2]);
1046+
expect(values2).toEqual([1, 2]);
1047+
a.set(2);
1048+
expect(values1).toEqual([1, 2, 3]);
1049+
expect(values2).toEqual([1, 2, 3]);
1050+
unsubscribe1();
1051+
unsubscribe2();
1052+
});
1053+
1054+
it('should work when doing a first subscription to a derived store of an already subscribed store inside batch', () => {
1055+
const a = writable(0);
1056+
const b = derived(a, (a) => a + 1, -1);
1057+
const values1: number[] = [];
1058+
const values2: number[] = [];
1059+
const unsubscribe1 = a.subscribe((v) => values1.push(v));
1060+
let unsubscribe2!: () => void;
1061+
expect(values1).toEqual([0]);
1062+
batch(() => {
1063+
a.set(1);
1064+
unsubscribe2 = b.subscribe((v) => values2.push(v));
1065+
expect(values1).toEqual([0]);
1066+
expect(values2).toEqual([-1]); // gets the initial value of store b to avoid temporary computation
1067+
});
1068+
expect(values1).toEqual([0, 1]);
1069+
expect(values2).toEqual([-1, 2]);
1070+
a.set(2);
1071+
expect(values1).toEqual([0, 1, 2]);
1072+
expect(values2).toEqual([-1, 2, 3]);
1073+
unsubscribe1();
1074+
unsubscribe2();
1075+
});
1076+
1077+
it('should work when doing a first subscription to a derived store of two stores with an already subscribed one inside batch', () => {
1078+
const a = writable(0);
1079+
const b = writable(0);
1080+
const c = derived([a, b], ([a, b]) => `a${a}b${b}`, 'init');
1081+
const values1: number[] = [];
1082+
const values2: string[] = [];
1083+
const unsubscribe1 = a.subscribe((v) => values1.push(v));
1084+
expect(values1).toEqual([0]);
1085+
let unsubscribe2!: () => void;
1086+
batch(() => {
1087+
a.set(1);
1088+
b.set(1);
1089+
unsubscribe2 = c.subscribe((v) => values2.push(v));
1090+
expect(values2).toEqual(['init']); // gets the initial value of store c to avoid a temporary computation
1091+
a.set(2);
1092+
b.set(2);
1093+
expect(values1).toEqual([0]);
1094+
expect(values2).toEqual(['init']);
1095+
});
1096+
expect(values1).toEqual([0, 2]);
1097+
expect(values2).toEqual(['init', 'a2b2']);
1098+
a.set(3);
1099+
expect(values1).toEqual([0, 2, 3]);
1100+
expect(values2).toEqual(['init', 'a2b2', 'a3b2']);
1101+
unsubscribe1();
1102+
unsubscribe2();
1103+
});
1104+
1105+
it('should work with a derived store of a derived store that is invalidated but finally does not change', () => {
1106+
const a = writable(0);
1107+
const b = writable(0);
1108+
const cFn = jasmine.createSpy('derived', (a) => `a${a}`).and.callThrough();
1109+
const c = derived(a, cFn);
1110+
const dFn = jasmine.createSpy('derived', ([b, c]) => `b${b}c${c}`).and.callThrough();
1111+
const d = derived([b, c], dFn);
1112+
const values: string[] = [];
1113+
const unsubscribe = d.subscribe((v) => values.push(v));
1114+
expect(values).toEqual(['b0ca0']);
1115+
expect(cFn).toHaveBeenCalledTimes(1);
1116+
expect(dFn).toHaveBeenCalledTimes(1);
1117+
batch(() => {
1118+
a.set(1);
1119+
a.set(0);
1120+
b.set(1);
1121+
expect(cFn).toHaveBeenCalledTimes(1);
1122+
expect(dFn).toHaveBeenCalledTimes(1);
1123+
expect(values).toEqual(['b0ca0']);
1124+
});
1125+
expect(cFn).toHaveBeenCalledTimes(1); // should not call again cFn as a went back to its initial value
1126+
expect(dFn).toHaveBeenCalledTimes(2);
1127+
expect(values).toEqual(['b0ca0', 'b1ca0']);
1128+
a.set(2);
1129+
expect(cFn).toHaveBeenCalledTimes(2);
1130+
expect(dFn).toHaveBeenCalledTimes(3);
1131+
expect(values).toEqual(['b0ca0', 'b1ca0', 'b1ca2']);
1132+
unsubscribe();
1133+
});
1134+
1135+
it('should work with a derived store of a derived store that is invalidated and finally changes', () => {
1136+
const a = writable(0);
1137+
const b = writable(0);
1138+
const cFn = jasmine.createSpy('derived', (a) => `a${a}`).and.callThrough();
1139+
const c = derived(a, cFn);
1140+
const dFn = jasmine.createSpy('dFn', ([b, c]) => `b${b}c${c}`).and.callThrough();
1141+
const d = derived([b, c], dFn);
1142+
const values: string[] = [];
1143+
const unsubscribe = d.subscribe((v) => values.push(v));
1144+
expect(values).toEqual(['b0ca0']);
1145+
expect(cFn).toHaveBeenCalledTimes(1);
1146+
expect(dFn).toHaveBeenCalledTimes(1);
1147+
batch(() => {
1148+
a.set(1);
1149+
a.set(0);
1150+
b.set(1);
1151+
a.set(1);
1152+
expect(cFn).toHaveBeenCalledTimes(1);
1153+
expect(dFn).toHaveBeenCalledTimes(1);
1154+
expect(values).toEqual(['b0ca0']);
1155+
});
1156+
expect(cFn).toHaveBeenCalledTimes(2);
1157+
expect(dFn).toHaveBeenCalledTimes(2);
1158+
expect(values).toEqual(['b0ca0', 'b1ca1']);
1159+
a.set(2);
1160+
expect(cFn).toHaveBeenCalledTimes(3);
1161+
expect(dFn).toHaveBeenCalledTimes(3);
1162+
expect(values).toEqual(['b0ca0', 'b1ca1', 'b1ca2']);
1163+
unsubscribe();
1164+
});
1165+
1166+
it('should work with a derived store of an async derived store that is invalidated but does not set any new value', () => {
1167+
const a = writable(0);
1168+
const b = writable(0);
1169+
const cFn = jasmine
1170+
.createSpy('derived', (a, set) => {
1171+
// set is only called if a is even, otherwise the old value is kept
1172+
if (a % 2 === 0) set(`a${a}`);
1173+
})
1174+
.and.callThrough();
1175+
const c = derived(a, cFn, 'i');
1176+
const dFn = jasmine.createSpy('derived', ([b, c]) => `b${b}c${c}`).and.callThrough();
1177+
const d = derived([b, c], dFn);
1178+
const values: string[] = [];
1179+
const unsubscribe = d.subscribe((v) => values.push(v));
1180+
expect(values).toEqual(['b0ca0']);
1181+
expect(cFn).toHaveBeenCalledTimes(1);
1182+
expect(dFn).toHaveBeenCalledTimes(1);
1183+
batch(() => {
1184+
a.set(1);
1185+
a.set(0);
1186+
a.set(1); // will be ignored by c
1187+
expect(cFn).toHaveBeenCalledTimes(1);
1188+
expect(dFn).toHaveBeenCalledTimes(1);
1189+
expect(values).toEqual(['b0ca0']);
1190+
});
1191+
expect(cFn).toHaveBeenCalledTimes(2); // called again because a changed
1192+
expect(dFn).toHaveBeenCalledTimes(1); // not called again because c did not change
1193+
expect(values).toEqual(['b0ca0']);
1194+
b.set(1);
1195+
expect(cFn).toHaveBeenCalledTimes(2); // not called again because a did not change
1196+
expect(dFn).toHaveBeenCalledTimes(2); // called again because b changed
1197+
expect(values).toEqual(['b0ca0', 'b1ca0']);
1198+
a.set(2);
1199+
expect(cFn).toHaveBeenCalledTimes(3); // called again because a changed
1200+
expect(dFn).toHaveBeenCalledTimes(3); // called again because c changed
1201+
expect(values).toEqual(['b0ca0', 'b1ca0', 'b1ca2']);
1202+
unsubscribe();
1203+
});
1204+
8381205
it('should not call an unregistered listener', () => {
8391206
const store = writable(0);
8401207
const calls: number[] = [];

0 commit comments

Comments
 (0)