From e74d2470c5220c19fb17ce910d1562ce4c116812 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Mime=20=C4=8Cuvalo?= <mimecuvalo@gmail.com>
Date: Tue, 4 Jun 2024 11:55:22 +0100
Subject: [PATCH] timeouts: rework effectschedule timeout tracking (#3870)
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

talked to @ds300 and decided that we needed to rework this latest bit to
cleanup outside of `EffectScheduler`

### Change Type

<!-- ❗ Please select a 'Scope' label ❗️ -->

- [x] `sdk` — Changes the tldraw SDK
- [ ] `dotcom` — Changes the tldraw.com web app
- [ ] `docs` — Changes to the documentation, examples, or templates.
- [ ] `vs code` — Changes to the vscode plugin
- [ ] `internal` — Does not affect user-facing stuff

<!-- ❗ Please select a 'Type' label ❗️ -->

- [x] `bugfix` — Bug fix
- [ ] `feature` — New feature
- [ ] `improvement` — Improving existing features
- [ ] `chore` — Updating dependencies, other boring stuff
- [ ] `galaxy brain` — Architectural changes
- [ ] `tests` — Changes to any test code
- [ ] `tools` — Changes to infrastructure, CI, internal scripts,
debugging tools, etc.
- [ ] `dunno` — I don't know
---
 packages/state/src/lib/core/EffectScheduler.ts |  9 +++------
 packages/state/src/lib/react/useReactor.ts     | 13 +++++++++++--
 2 files changed, 14 insertions(+), 8 deletions(-)

diff --git a/packages/state/src/lib/core/EffectScheduler.ts b/packages/state/src/lib/core/EffectScheduler.ts
index fb13c90df..9d4767d89 100644
--- a/packages/state/src/lib/core/EffectScheduler.ts
+++ b/packages/state/src/lib/core/EffectScheduler.ts
@@ -35,7 +35,7 @@ interface EffectSchedulerOptions {
 	 * @param execute - A function that will execute the effect.
 	 * @returns
 	 */
-	scheduleEffect?: (execute: () => void) => number | void | (() => void)
+	scheduleEffect?: (execute: () => void) => void
 }
 
 class __EffectScheduler__<Result> {
@@ -63,15 +63,13 @@ class __EffectScheduler__<Result> {
 		return this._scheduleCount
 	}
 
-	/** @internal */
-	private maybeRaf: number | void | (() => void) = -1
 	/** @internal */
 	readonly parentSet = new ArraySet<Signal<any, any>>()
 	/** @internal */
 	readonly parentEpochs: number[] = []
 	/** @internal */
 	readonly parents: Signal<any, any>[] = []
-	private readonly _scheduleEffect?: (execute: () => void) => number | void | (() => void)
+	private readonly _scheduleEffect?: (execute: () => void) => void
 	constructor(
 		public readonly name: string,
 		private readonly runEffect: (lastReactedEpoch: number) => Result,
@@ -101,7 +99,7 @@ class __EffectScheduler__<Result> {
 		this._scheduleCount++
 		if (this._scheduleEffect) {
 			// if the effect should be deferred (e.g. until a react render), do so
-			this.maybeRaf = this._scheduleEffect(this.maybeExecute)
+			this._scheduleEffect(this.maybeExecute)
 		} else {
 			// otherwise execute right now!
 			this.execute()
@@ -137,7 +135,6 @@ class __EffectScheduler__<Result> {
 		for (let i = 0, n = this.parents.length; i < n; i++) {
 			detach(this.parents[i], this)
 		}
-		typeof this.maybeRaf === 'number' && cancelAnimationFrame(this.maybeRaf)
 	}
 
 	/**
diff --git a/packages/state/src/lib/react/useReactor.ts b/packages/state/src/lib/react/useReactor.ts
index 11dbd03a8..f3a8fd05e 100644
--- a/packages/state/src/lib/react/useReactor.ts
+++ b/packages/state/src/lib/react/useReactor.ts
@@ -1,10 +1,18 @@
-import { useEffect, useMemo } from 'react'
+import { useEffect, useMemo, useRef } from 'react'
 import { EffectScheduler } from '../core'
 
 /** @public */
 export function useReactor(name: string, reactFn: () => void, deps: undefined | any[] = []) {
+	const raf = useRef(-1)
 	const scheduler = useMemo(
-		() => new EffectScheduler(name, reactFn, { scheduleEffect: (cb) => requestAnimationFrame(cb) }),
+		() =>
+			new EffectScheduler(name, reactFn, {
+				scheduleEffect: (cb) => {
+					const rafId = requestAnimationFrame(cb)
+					raf.current = rafId
+					return rafId
+				},
+			}),
 		// eslint-disable-next-line react-hooks/exhaustive-deps
 		deps
 	)
@@ -14,6 +22,7 @@ export function useReactor(name: string, reactFn: () => void, deps: undefined |
 		scheduler.execute()
 		return () => {
 			scheduler.detach()
+			cancelAnimationFrame(raf.current)
 		}
 	}, [scheduler])
 }