From 0bc8c5ffd572ec285a5a34daad1fed9576ccd24b Mon Sep 17 00:00:00 2001 From: James Forbes Date: Sun, 1 Jan 2023 15:47:41 +1100 Subject: [PATCH] Fixes #31 --- README.md | 9 ++++ lib/index.ts | 126 ++++++++++++++++++++++++++++++++++++--------------- test/test.ts | 34 ++++++++++++++ 3 files changed, 133 insertions(+), 36 deletions(-) diff --git a/README.md b/README.md index f8443c4..5cefa05 100644 --- a/README.md +++ b/README.md @@ -197,6 +197,15 @@ S.id(a); // kjhjkgasd This id is not globally unique, or cryptographically random, its just a debugging thing. +You can also overwrite the generated id with your own to make debugging easier: + +```js +S.id(a, 'a') +S.id() // 'a' +``` + +> 🤔 In future we may just expose the id map instead of having this getter setter + ### `S.sample` Read the value of a signal within a computation without treating it as a dependency of that computation. diff --git a/lib/index.ts b/lib/index.ts index 7eab087..19a8f07 100644 --- a/lib/index.ts +++ b/lib/index.ts @@ -6,6 +6,7 @@ type SyncComputationInternal = { value?: T; next?: T; compute: () => void; + id: string; }; type GeneratorComputationInternal = { @@ -14,13 +15,16 @@ type GeneratorComputationInternal = { value?: T; next?: T; compute: () => void; + id: string; }; type ComputationInternal = | SyncComputationInternal | GeneratorComputationInternal; -type DataInternal = { type: "Stream"; tag: "Data"; value?: T; next?: T }; +type DataInternal = { + type: "Stream"; tag: "Data"; value?: T; next?: T, id: string +}; type StreamInternal = DataInternal | ComputationInternal; @@ -70,7 +74,21 @@ const rootOfStream = new WeakMap, Root>(); /** * The computations that will need to rerun next tick */ -export let toRun = new Set>(); +export let runningNextTick = new Set>(); + +/** + * The computations that are scheduled to run in the current propagation + */ +export let runningThisTick = new Set>(); + +/** + * If a computation is nested within another computation + * and the parent is scheduled to run in this tick along with the child + * we only need to run the parent, as the child will automatically + * run when the parent compute runs, so we need to skip + * those child nodes to prevent double updates, that's what this set is for + */ +export let allChildren = new Set>(); /** * The direct dependencies of a stream, used in computeDependents @@ -224,7 +242,7 @@ function computeDependents(stream: StreamInternal) { } } - toRun.add(x); + runningNextTick.add(x); } } @@ -245,6 +263,9 @@ export function data( tag: "Data", next: value, value, + get id(){ + return ids.get( accessor )! + } }; let accessor = (...args: T[] | [(x?: T) => T]) => { @@ -325,6 +346,9 @@ export function computation( streamsToResolve.add(stream); } }, + get id(){ + return ids.get(accessor)! + } }; // whatever active was when this was defined @@ -365,7 +389,11 @@ export function computation( stream.compute(); } - return record(stream, () => { + const accessor = () => { + if ( state === 'propagating' && runningThisTick.has(stream) ) { + // compute now + tickOne(stream) + } if (active[0]) { xet(dependents, stream, () => new Set()).add(active[0]); @@ -373,7 +401,9 @@ export function computation( } else { return stream.value!; } - }); + } + + return record(stream, accessor); } // only defining these types as I couldn't @@ -448,6 +478,9 @@ export function generator( } }); }, + get id(){ + return ids.get(accessor)! + } }; async function iterate(it: StreamIterator) { @@ -489,14 +522,19 @@ export function generator( stream.compute(); - return record(stream, () => { + const accessor = () => { + if ( state === 'propagating' && runningThisTick.has(stream) ) { + // compute now + tickOne(stream) + } if (active[0]) { xet(dependents, stream, () => new Set()).add(active[0]); return stream.next; } return stream.value; - }); + } + return record(stream, accessor); } export function freeze(f: VoidFunction): void { @@ -548,7 +586,7 @@ export function root(f: (dispose: VoidFunction) => T) { cleanup(); } dependents.delete(x); - toRun.delete(x); + runningNextTick.delete(x); cleanups.delete(x); parents.delete(x); } @@ -559,6 +597,37 @@ export function root(f: (dispose: VoidFunction) => T) { return out; } +function tickOne( + stream: ComputationInternal +){ + if ( !runningThisTick.has(stream) ) { + // evalauted already by another compute + return; + } + for (let cleanupFn of xet(cleanups, stream, () => new Set())) { + cleanupFn(); + cleanups.get(stream)!.delete(cleanupFn); + } + + // if is a child, we only need to clean up + if (allChildren.has(stream)) { + return; + } else { + let oldActive = active; + active = [...(parents.get(stream) ?? [])]; + + let oldActiveRoot = activeRoot; + activeRoot = rootOfStream.get(stream) ?? null; + + stream.compute(); + + active = oldActive; + activeRoot = oldActiveRoot; + stats.computations.evaluated++; + } + runningThisTick.delete(stream) +} + export function tick() { if (state === "propagating" || state === "frozen") return; @@ -566,18 +635,19 @@ export function tick() { state = "propagating"; - let oldToRun = new Set(toRun); + runningThisTick = new Set(runningNextTick); + + runningNextTick.clear(); - toRun.clear(); + allChildren.clear() - let allChildren = new Set>(); - for (let x of oldToRun) { + for (let x of runningThisTick) { for (let child of children.get(x) ?? []) { // record the child exists allChildren.add(child); // add to the tick so we can run // the clean up fns (at most once) - oldToRun.add(child); + runningThisTick.add(child); parents.delete(child); streamsToResolve.delete(child); // should we delete the dependents too? @@ -588,28 +658,8 @@ export function tick() { children.delete(x); } - for (let f of oldToRun) { - for (let cleanupFn of xet(cleanups, f, () => new Set())) { - cleanupFn(); - cleanups.get(f)!.delete(cleanupFn); - } - - // if is a child, we only need to clean up - if (allChildren.has(f)) { - continue; - } else { - let oldActive = active; - active = [...(parents.get(f) ?? [])]; - - let oldActiveRoot = activeRoot; - activeRoot = rootOfStream.get(f) ?? null; - - f.compute(); - - active = oldActive; - activeRoot = oldActiveRoot; - stats.computations.evaluated++; - } + for (let stream of [...runningThisTick]) { + tickOne(stream) } for (let s of streamsToResolve) { @@ -635,6 +685,7 @@ export function tick() { nextTicks.length = 0; for (let next of xs) { if (runawayTicks > MAX_TICKS) { + state = 'idle' throw new RunawayTicks(); } next(); @@ -659,6 +710,9 @@ export function sample(signal: Signal) { return value; } -export function id(s: Signal) { +export function id(s: Signal, newId?: string) { + if (newId != null ) { + ids.set(s, newId) + } return ids.get(s); } diff --git a/test/test.ts b/test/test.ts index e1ef71b..b5c7359 100644 --- a/test/test.ts +++ b/test/test.ts @@ -1080,5 +1080,39 @@ test('Max ticks', t => { }, S.RunawayTicks, 'Runaway ticks caught') t.equals(S.stats.ticks, S.MAX_TICKS+1, 'Caught at max ticks') + t.end() +}) + +test('https://github.com/JAForbes/S/issues/31', t => { + + const a = S.data(1) + + const { b,c,d } = S.root(() => { + + const b = S.computation(() => { + return a() * 1 + }) + + const c = S.computation(() => { + return b()! * 2 + }) + + const d = S.computation(() => { + return a() + c()! + }) + + return { b, c, d } + }) + + Object.entries({ a,b, c,d }).map( ([k,v]) => S.id(v,k) ) + + t.equals(c()!, 2, 'c propagated') + t.equals(d()!, 3, 'd propagated') + + a(2) + + t.equals(c()!, 4, 'c propagated') + t.equals(d()!, 6, 'd propagated') + t.end() }) \ No newline at end of file