Skip to content

Commit

Permalink
Fix issues with removing listeners from the queue (#302)
Browse files Browse the repository at this point in the history
  • Loading branch information
russelldavis committed May 6, 2024
1 parent 74461a4 commit 02fdc75
Show file tree
Hide file tree
Showing 3 changed files with 108 additions and 11 deletions.
24 changes: 15 additions & 9 deletions atom/index.js
Original file line number Diff line number Diff line change
@@ -1,6 +1,8 @@
import { clean } from '../clean-stores/index.js'

let listenerQueue = []
let lqIndex = 0
const QUEUE_ITEMS_PER_LISTENER = 4
export let epoch = 0

export let atom = (initialValue) => {
Expand All @@ -17,15 +19,19 @@ export let atom = (initialValue) => {
$atom.lc = listeners.push(listener)

return () => {
for (let i = lqIndex + QUEUE_ITEMS_PER_LISTENER; i < listenerQueue.length;) {
if (listenerQueue[i] === listener) {
listenerQueue.splice(i, QUEUE_ITEMS_PER_LISTENER)
} else {
i += QUEUE_ITEMS_PER_LISTENER
}
}

let index = listeners.indexOf(listener)
let queueIndex = listenerQueue.indexOf(listener)
if (~index) {
listeners.splice(index, 1)
if (!--$atom.lc) $atom.off()
}
if (~queueIndex) {
listenerQueue.splice(index, 4)
}
}
},
notify(oldValue, changedKey) {
Expand All @@ -41,11 +47,11 @@ export let atom = (initialValue) => {
}

if (runListenerQueue) {
for (let i = 0; i < listenerQueue.length; i += 4) {
listenerQueue[i](
listenerQueue[i + 1],
listenerQueue[i + 2],
listenerQueue[i + 3]
for (lqIndex = 0; lqIndex < listenerQueue.length; lqIndex += QUEUE_ITEMS_PER_LISTENER) {
listenerQueue[lqIndex](
listenerQueue[lqIndex + 1],
listenerQueue[lqIndex + 2],
listenerQueue[lqIndex + 3]
)
}
listenerQueue.length = 0
Expand Down
91 changes: 91 additions & 0 deletions atom/index.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -287,6 +287,97 @@ test('does not run queued listeners after they are unsubscribed', () => {
deepStrictEqual(events, ['a1', 'b1', 'a2', 'c2'])
})

test('does not run queued listeners after they are unsubscribed when queue index is different from listener index', () => {
let events: string[] = []
let $storeA = atom<number>(0)
let $storeB = atom<number>(0)

$storeA.listen(value => {
events.push(`a1_${value}`)
$storeB.set(1)
unbindB()
})

$storeA.listen(value => {
events.push(`a2_${value}`)
})

let unbindB = $storeB.listen(value => {
events.push(`b1_${value}`)
})

$storeA.set(1)
deepStrictEqual(events, ['a1_1', 'a2_1'])
})

test('does not run queued listeners after they are unsubscribed after the store is modified multiple times during the same batch', () => {
let events: string[] = []
let $storeA = atom<number>(0)
let $storeB = atom<number>(0)

$storeA.listen(value => {
events.push(`a1_${value}`)
$storeB.set(1)
$storeB.set(2)
unbindB()
})

$storeA.listen(value => {
events.push(`a2_${value}`)
})

let unbindB = $storeB.listen(value => {
events.push(`b1_${value}`)
})

$storeA.set(1)
deepStrictEqual(events, ['a1_1', 'a2_1'])
})

test('runs the right listeners after a listener in the queue that has already been called is unsubscribed', () => {
let events: string[] = []
let $store = atom<number>(0)

let unbindA = $store.listen(value => {
events.push(`a${value}`)
})

$store.listen(value => {
events.push(`b${value}`)
unbindA()
})

$store.listen(value => {
events.push(`c${value}`)
})

$store.set(1)
deepStrictEqual(events, ['a1', 'b1', 'c1'])

events.length = 0
$store.set(2)
deepStrictEqual(events, ['b2', 'c2'])
})

test('unsubscribe works with listenerQueue when atom value contains other listener function', () => {
let events: string[] = []
let $store = atom<any>()

$store.listen(() => {
events.push('a')
unbindC()
})
$store.listen(() => {
events.push('b')
})
let listenerC = (): void => {
events.push('c')
}
let unbindC = $store.listen(listenerC)
$store.set(listenerC)
deepStrictEqual(events, ['a', 'b'])
})

test('prevents notifying when new value is referentially equal to old one', () => {
let events: (string | undefined)[] = []

Expand Down
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": "250 B"
"limit": "265 B"
},
{
"name": "Popular Set",
"import": {
"./index.js": "{ map, computed, }"
},
"limit": "786 B"
"limit": "806 B"
}
],
"clean-publish": {
Expand Down

0 comments on commit 02fdc75

Please sign in to comment.