Skip to content

Commit 3096bf2

Browse files
authored
Fix Issues with Derived Fields not Retriggering (#274)
* Revert "fix(store): improve performance by ~4x (#236)" This reverts commit 4a8bdea. * chore: add changeset
1 parent de41b4e commit 3096bf2

6 files changed

Lines changed: 29 additions & 31 deletions

File tree

.changeset/common-shoes-mate.md

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
---
2+
'@tanstack/store': patch
3+
---
4+
5+
Fix Issues with Derived Fields not Retriggering

docs/reference/classes/Derived.md

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -95,7 +95,7 @@ Defined in: [derived.ts:68](https://github.com/TanStack/store/blob/main/packages
9595
checkIfRecalculationNeededDeeply(): void;
9696
```
9797

98-
Defined in: [derived.ts:178](https://github.com/TanStack/store/blob/main/packages/store/src/derived.ts#L178)
98+
Defined in: [derived.ts:162](https://github.com/TanStack/store/blob/main/packages/store/src/derived.ts#L162)
9999

100100
#### Returns
101101

@@ -141,7 +141,7 @@ prevVal: NonNullable<TState> | undefined;
141141
mount(): () => void;
142142
```
143143

144-
Defined in: [derived.ts:199](https://github.com/TanStack/store/blob/main/packages/store/src/derived.ts#L199)
144+
Defined in: [derived.ts:183](https://github.com/TanStack/store/blob/main/packages/store/src/derived.ts#L183)
145145

146146
#### Returns
147147

@@ -161,7 +161,7 @@ Defined in: [derived.ts:199](https://github.com/TanStack/store/blob/main/package
161161
recompute(): void;
162162
```
163163
164-
Defined in: [derived.ts:170](https://github.com/TanStack/store/blob/main/packages/store/src/derived.ts#L170)
164+
Defined in: [derived.ts:154](https://github.com/TanStack/store/blob/main/packages/store/src/derived.ts#L154)
165165
166166
#### Returns
167167
@@ -197,7 +197,7 @@ readonly (
197197
subscribe(listener): () => void;
198198
```
199199

200-
Defined in: [derived.ts:211](https://github.com/TanStack/store/blob/main/packages/store/src/derived.ts#L211)
200+
Defined in: [derived.ts:195](https://github.com/TanStack/store/blob/main/packages/store/src/derived.ts#L195)
201201

202202
#### Parameters
203203

@@ -223,7 +223,7 @@ Defined in: [derived.ts:211](https://github.com/TanStack/store/blob/main/package
223223
unregisterFromGraph(deps): void;
224224
```
225225
226-
Defined in: [derived.ts:147](https://github.com/TanStack/store/blob/main/packages/store/src/derived.ts#L147)
226+
Defined in: [derived.ts:134](https://github.com/TanStack/store/blob/main/packages/store/src/derived.ts#L134)
227227
228228
#### Parameters
229229

docs/reference/functions/batch.md

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,7 @@ title: batch
99
function batch(fn): void;
1010
```
1111

12-
Defined in: [scheduler.ts:133](https://github.com/TanStack/store/blob/main/packages/store/src/scheduler.ts#L133)
12+
Defined in: [scheduler.ts:142](https://github.com/TanStack/store/blob/main/packages/store/src/scheduler.ts#L142)
1313

1414
## Parameters
1515

docs/reference/variables/storeToDerived.md

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,7 @@ title: __storeToDerived
66
# Variable: \_\_storeToDerived
77

88
```ts
9-
const __storeToDerived: WeakMap<Store<unknown, (cb) => unknown>, Derived<unknown, readonly any[]>[]>;
9+
const __storeToDerived: WeakMap<Store<unknown, (cb) => unknown>, Set<Derived<unknown, readonly any[]>>>;
1010
```
1111

1212
Defined in: [scheduler.ts:19](https://github.com/TanStack/store/blob/main/packages/store/src/scheduler.ts#L19)

packages/store/src/derived.ts

Lines changed: 3 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -105,7 +105,6 @@ export class Derived<
105105
registerOnGraph(
106106
deps: ReadonlyArray<Derived<any> | Store<any>> = this.options.deps,
107107
) {
108-
const toSort = new Set<Array<Derived<unknown>>>()
109108
for (const dep of deps) {
110109
if (dep instanceof Derived) {
111110
// First register the intermediate derived value if it's not already registered
@@ -116,12 +115,10 @@ export class Derived<
116115
// Register the derived as related derived to the store
117116
let relatedLinkedDerivedVals = __storeToDerived.get(dep)
118117
if (!relatedLinkedDerivedVals) {
119-
relatedLinkedDerivedVals = [this as never]
118+
relatedLinkedDerivedVals = new Set()
120119
__storeToDerived.set(dep, relatedLinkedDerivedVals)
121-
} else if (!relatedLinkedDerivedVals.includes(this as never)) {
122-
relatedLinkedDerivedVals.push(this as never)
123-
toSort.add(relatedLinkedDerivedVals)
124120
}
121+
relatedLinkedDerivedVals.add(this as never)
125122

126123
// Register the store as a related store to this derived
127124
let relatedStores = __derivedToStore.get(this as never)
@@ -132,16 +129,6 @@ export class Derived<
132129
relatedStores.add(dep)
133130
}
134131
}
135-
for (const arr of toSort) {
136-
// First sort deriveds by dependency order
137-
arr.sort((a, b) => {
138-
// If a depends on b, b should go first
139-
if (a instanceof Derived && a.options.deps.includes(b)) return 1
140-
// If b depends on a, a should go first
141-
if (b instanceof Derived && b.options.deps.includes(a)) return -1
142-
return 0
143-
})
144-
}
145132
}
146133

147134
unregisterFromGraph(
@@ -153,10 +140,7 @@ export class Derived<
153140
} else if (dep instanceof Store) {
154141
const relatedLinkedDerivedVals = __storeToDerived.get(dep)
155142
if (relatedLinkedDerivedVals) {
156-
relatedLinkedDerivedVals.splice(
157-
relatedLinkedDerivedVals.indexOf(this as never),
158-
1,
159-
)
143+
relatedLinkedDerivedVals.delete(this as never)
160144
}
161145

162146
const relatedStores = __derivedToStore.get(this as never)

packages/store/src/scheduler.ts

Lines changed: 14 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
import type { Derived } from './derived'
1+
import { Derived } from './derived'
22
import type { Store } from './store'
33

44
/**
@@ -18,7 +18,7 @@ import type { Store } from './store'
1818
*/
1919
export const __storeToDerived = new WeakMap<
2020
Store<unknown>,
21-
Array<Derived<unknown>>
21+
Set<Derived<unknown>>
2222
>()
2323
export const __derivedToStore = new WeakMap<
2424
Derived<unknown>,
@@ -35,8 +35,17 @@ const __pendingUpdates = new Set<Store<unknown>>()
3535
// Add a map to store initial values before batch
3636
const __initialBatchValues = new Map<Store<unknown>, unknown>()
3737

38-
function __flush_internals(relatedVals: ReadonlyArray<Derived<unknown>>) {
39-
for (const derived of relatedVals) {
38+
function __flush_internals(relatedVals: Set<Derived<unknown>>) {
39+
// First sort deriveds by dependency order
40+
const sorted = Array.from(relatedVals).sort((a, b) => {
41+
// If a depends on b, b should go first
42+
if (a instanceof Derived && a.options.deps.includes(b)) return 1
43+
// If b depends on a, a should go first
44+
if (b instanceof Derived && b.options.deps.includes(a)) return -1
45+
return 0
46+
})
47+
48+
for (const derived of sorted) {
4049
if (__depsThatHaveWrittenThisTick.current.includes(derived)) {
4150
continue
4251
}
@@ -48,7 +57,7 @@ function __flush_internals(relatedVals: ReadonlyArray<Derived<unknown>>) {
4857
if (stores) {
4958
for (const store of stores) {
5059
const relatedLinkedDerivedVals = __storeToDerived.get(store)
51-
if (!relatedLinkedDerivedVals?.length) continue
60+
if (!relatedLinkedDerivedVals) continue
5261
__flush_internals(relatedLinkedDerivedVals)
5362
}
5463
}

0 commit comments

Comments
 (0)