Skip to content

Commit

Permalink
Fix stale computed values (#300)
Browse files Browse the repository at this point in the history
* Prevent stale computed values

* Remove atom levels

The previous commit's solution to the stale atom problem also solves the
diamond problem without needing special handling of atom levels.

* Optimize computed dependency check using epochs

* Update size-limit

---------

Co-authored-by: Andrey Sitnik <andrey@sitnik.ru>
  • Loading branch information
russelldavis and ai committed May 5, 2024
1 parent aeacd88 commit f1e5184
Show file tree
Hide file tree
Showing 4 changed files with 65 additions and 33 deletions.
41 changes: 13 additions & 28 deletions atom/index.js
Original file line number Diff line number Diff line change
@@ -1,8 +1,9 @@
import { clean } from '../clean-stores/index.js'

let listenerQueue = []
export let epoch = 0

export let atom = (initialValue, level) => {
export let atom = (initialValue) => {
let listeners = []
let $atom = {
get() {
Expand All @@ -11,16 +12,15 @@ export let atom = (initialValue, level) => {
}
return $atom.value
},
l: level || 0,
lc: 0,
listen(listener, listenerLevel) {
$atom.lc = listeners.push(listener, listenerLevel || $atom.l) / 2
listen(listener) {
$atom.lc = listeners.push(listener)

return () => {
let index = listeners.indexOf(listener)
let queueIndex = listenerQueue.indexOf(listener)
if (~index) {
listeners.splice(index, 2)
listeners.splice(index, 1)
if (!--$atom.lc) $atom.off()
}
if (~queueIndex) {
Expand All @@ -29,39 +29,24 @@ export let atom = (initialValue, level) => {
}
},
notify(oldValue, changedKey) {
epoch++
let runListenerQueue = !listenerQueue.length
for (let i = 0; i < listeners.length; i += 2) {
for (let listener of listeners) {
listenerQueue.push(
listeners[i],
listeners[i + 1],
listener,
$atom.value,
oldValue,
changedKey
)
}

if (runListenerQueue) {
for (let i = 0; i < listenerQueue.length; i += 5) {
let skip
for (let j = i + 1; !skip && (j += 5) < listenerQueue.length; ) {
if (listenerQueue[j] < listenerQueue[i + 1]) {
skip = listenerQueue.push(
listenerQueue[i],
listenerQueue[i + 1],
listenerQueue[i + 2],
listenerQueue[i + 3],
listenerQueue[i + 4]
)
}
}

if (!skip) {
for (let i = 0; i < listenerQueue.length; i += 4) {
listenerQueue[i](
listenerQueue[i + 1],
listenerQueue[i + 2],
listenerQueue[i + 3],
listenerQueue[i + 4]
listenerQueue[i + 3]
)
}
}
listenerQueue.length = 0
}
Expand All @@ -76,8 +61,8 @@ export let atom = (initialValue, level) => {
$atom.notify(oldValue)
}
},
subscribe(listener, listenerLevel) {
let unbind = $atom.listen(listener, listenerLevel)
subscribe(listener) {
let unbind = $atom.listen(listener)
listener($atom.value)
return unbind
},
Expand Down
14 changes: 11 additions & 3 deletions computed/index.js
Original file line number Diff line number Diff line change
@@ -1,12 +1,15 @@
import { atom } from '../atom/index.js'
import { atom, epoch } from '../atom/index.js'
import { onMount } from '../lifecycle/index.js'

let computedStore = (stores, cb, batched) => {
if (!Array.isArray(stores)) stores = [stores]

let previousArgs
let currentRunId = 0
let currentEpoch
let set = () => {
if (currentEpoch === epoch) return
currentEpoch = epoch
let args = stores.map($store => $store.get())
if (
previousArgs === undefined ||
Expand All @@ -27,7 +30,12 @@ let computedStore = (stores, cb, batched) => {
}
}
}
let $computed = atom(undefined, Math.max(...stores.map($s => $s.l)) + 1)
let $computed = atom(undefined)
let get = $computed.get
$computed.get = () => {
set()
return get()
}

let timer
let run = batched
Expand All @@ -38,7 +46,7 @@ let computedStore = (stores, cb, batched) => {
: set

onMount($computed, () => {
let unbinds = stores.map($store => $store.listen(run, -1 / $computed.l))
let unbinds = stores.map($store => $store.listen(run))
set()
return () => {
for (let unbind of unbinds) unbind()
Expand Down
39 changes: 39 additions & 0 deletions computed/index.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -547,3 +547,42 @@ test('cleans up on unmount', async () => {
equal($derived.lc, 0)
equal($source.lc, 0)
})

test('stale computed in other computed', () => {
let $atom = atom(1)
let values: (number | string)[] = []
let $computed1 = computed($atom, () => {values.push($computed2.get())})
let $computed2 = computed($atom, value => value * 2)
$computed1.subscribe(() => {})
$atom.set(2)
deepStrictEqual(values, [2, 4])
})

test('stale computed in listener', () => {
let $event = atom()
let $atom = atom(1)
let $computed = computed($atom, value => value * 2)
$computed.listen(() => {})
let values: (number | string)[] = []
$event.listen(() => {
$atom.set(2)
values.push($computed.get())
})
$event.set("foo")
deepStrictEqual(values, [4])
})

test('stale computed via nested dependency', () => {
let $event = atom()
let $atom = atom(1)
let $computed1 = computed($atom, value => value * 2)
let $computed2 = computed($computed1, value => value * 3)
$computed2.listen(() => {})
let values: (number | string)[] = []
$event.listen(() => {
$atom.set(2)
values.push($computed2.get())
})
$event.set("foo")
deepStrictEqual(values, [12])
})
4 changes: 2 additions & 2 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -92,14 +92,14 @@
"import": {
"./index.js": "{ atom }"
},
"limit": "302 B"
"limit": "235 B"
},
{
"name": "Popular Set",
"import": {
"./index.js": "{ map, computed, }"
},
"limit": "831 B"
"limit": "769 B"
}
],
"clean-publish": {
Expand Down

0 comments on commit f1e5184

Please sign in to comment.