raveclassic / frp-ts

Functional reactive values-over-time
MIT License
79 stars 8 forks source link

fix(core): combine lost notification of middle node #61

Closed Fyzu closed 2 years ago

Fyzu commented 2 years ago

We lose change notifications if we combine a and its derivative b into c.

const a = newAtom(1)
const b = combine(a, (a) => [a])

const c = combine(a, b, (v1, v2) => [v1, v2])

And then read c value immediately in next, we warm up the cache of the b.

const cb1 = jest.fn()
// subscribe and read value instantly
c.subscribe({
    next: () => c.get(),
})

b.subscribe({
    next: cb1,
})

And then emit a new value in a, the subscriber b will not receive a notification.

a.set(2)
nx-cloud[bot] commented 2 years ago

☁️ Nx Cloud Report

CI is running/has finished running commands for commit 1567210075708bddcf314c6c3ebeb05f98e587cd. As they complete they will appear below. Click to see the status, the terminal output, and the build insights.

📂 See all runs for this branch


✅ Successfully ran 3 targets - [`nx affected --target=test --parallel --max-parallel=2`](https://nx.app/runs/3pV7rGT0zHZ) - [`nx affected --target=lint --parallel --max-parallel=3`](https://nx.app/runs/iBFzOJiG5dV) - [`nx affected --target=build --parallel --max-parallel=3`](https://nx.app/runs/W0gG99CIKzB)

Sent with 💌 from NxCloud.

mankdev commented 2 years ago

here more simple test that affeced by this issue:

const a = newAtom(1)
const b = combine(a, (a) => [a])

const cb1 = jest.fn()

b.subscribe({
   next: cb1,
})

action(() => {
   a.set(2)
   b.get()
})
expect(cb1).toBeCalledTimes(1)
raveclassic commented 2 years ago

Thanks folks!