From 16fea7e32cdc76694fe5d31ddf6cde7176e0d8e4 Mon Sep 17 00:00:00 2001 From: Diego Borges Date: Fri, 14 Feb 2025 16:25:29 -0300 Subject: [PATCH 1/3] Handle nodes belonging to detached paths --- packages/arbor-store/src/utilities.ts | 10 +++++++++- .../arbor-store/tests/utilities/isDetached.test.ts | 14 ++++++++++++++ 2 files changed, 23 insertions(+), 1 deletion(-) diff --git a/packages/arbor-store/src/utilities.ts b/packages/arbor-store/src/utilities.ts index 2dd62e98..3c5357b3 100644 --- a/packages/arbor-store/src/utilities.ts +++ b/packages/arbor-store/src/utilities.ts @@ -95,7 +95,15 @@ export function isDetached(node: T): boolean { // but an ancestor has been detached.' if (!isNode(node)) return true - return !node.$tree.getLinkFor(node) && !node.$tree.getNodeFor(node) + const path = node.$tree.getPathFor(node) + + if (!path) { + return true + } + + return path.seeds.some( + (seed) => !node.$tree.getLinkFor(seed) && !node.$tree.getNodeFor(seed) + ) } /** diff --git a/packages/arbor-store/tests/utilities/isDetached.test.ts b/packages/arbor-store/tests/utilities/isDetached.test.ts index 31cf08bd..f0f195f1 100644 --- a/packages/arbor-store/tests/utilities/isDetached.test.ts +++ b/packages/arbor-store/tests/utilities/isDetached.test.ts @@ -25,4 +25,18 @@ describe("isDetached", () => { expect(isDetached(node)).toBe(true) }) + + it("returns true if the node belongs to a detached path", () => { + const store = new Arbor({ + todos: [ + { id: 1, text: "Do the dishes", author: { name: "Alice" } }, + { id: 2, text: "Walk the dogs", author: { name: "Bob" } }, + ], + }) + + const alice = store.state.todos[0].author + delete store.state.todos[0] + + expect(isDetached(alice)).toBe(true) + }) }) From ea581519a9ecc47850b7bc78ffa37766e3950bff Mon Sep 17 00:00:00 2001 From: Diego Borges Date: Fri, 21 Feb 2025 09:37:32 -0300 Subject: [PATCH 2/3] Only set store's state in useArborDeprecated if the store state is not detached This is important to avoid updates in unmounted React components still bound to detached nodes of the store --- packages/arbor-react/src/useArbor.ts | 5 +++- packages/arbor-store/tests/arbor.test.ts | 30 +++++++++++++++++++++--- 2 files changed, 31 insertions(+), 4 deletions(-) diff --git a/packages/arbor-react/src/useArbor.ts b/packages/arbor-react/src/useArbor.ts index f18d71b5..4d59a6da 100644 --- a/packages/arbor-react/src/useArbor.ts +++ b/packages/arbor-react/src/useArbor.ts @@ -1,6 +1,7 @@ import { Arbor, ArborNode, + isDetached, isNode, isProxiable, ScopedStore, @@ -76,7 +77,9 @@ function useArborDeprecated(store: Store): ArborNode { } return store.subscribe(() => { - setState(store.state) + if (!isDetached(store.state)) { + setState(store.state) + } }) }, [state, store]) diff --git a/packages/arbor-store/tests/arbor.test.ts b/packages/arbor-store/tests/arbor.test.ts index 54bed9b3..2f488f18 100644 --- a/packages/arbor-store/tests/arbor.test.ts +++ b/packages/arbor-store/tests/arbor.test.ts @@ -352,9 +352,8 @@ describe("Arbor", () => { delete store.state.todos[0] - // todos[0] should still exist in the previous snapshot - expect(todos[0]).toBeDefined() - expect(unwrap(todos[0])).toBe(state.todos[0]) + expect(todos).toEqual([{ text: "Walk the dogs" }]) + expect(todos[0]).toEqual({ text: "Walk the dogs" }) expect(store.state).toEqual({ todos: [{ text: "Walk the dogs" }] }) expect(store.state.todos).toEqual([{ text: "Walk the dogs" }]) @@ -822,6 +821,31 @@ describe("Arbor", () => { expect(subscriber2).not.toHaveBeenCalled() }) + it("does not trigger notifications when mutations are performed on nodes belonging to detached paths", () => { + const store = new Arbor({ + todos: [ + { id: 1, done: false, author: { name: "Alice", age: 29 } }, + { id: 2, done: false, author: { name: "Bob", age: 30 } }, + ], + }) + + const todo0 = store.state.todos[0] + const alice = store.state.todos[0].author + const subscriber1 = vi.fn() + const subscriber2 = vi.fn() + + delete store.state.todos[0] + + store.subscribe(subscriber1) + store.subscribeTo(alice, subscriber2) + store.subscribeTo(todo0, subscriber2) + + expect(() => (todo0.done = true)).toThrow(DetachedNodeError) + expect(() => alice.age++).toThrow(DetachedNodeError) + expect(subscriber1).not.toHaveBeenCalled() + expect(subscriber2).not.toHaveBeenCalled() + }) + it("ignores assignments when new value is the current value", () => { const alice = { name: "Alice" } const store = new Arbor({ From bfa0dfc79f339d882ddf813a630171121f57340e Mon Sep 17 00:00:00 2001 From: Diego Borges Date: Mon, 24 Feb 2025 11:02:10 -0300 Subject: [PATCH 3/3] Fix typo --- packages/arbor-react/tests/useArbor.test.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/packages/arbor-react/tests/useArbor.test.ts b/packages/arbor-react/tests/useArbor.test.ts index e4ea4684..b14bb1f1 100644 --- a/packages/arbor-react/tests/useArbor.test.ts +++ b/packages/arbor-react/tests/useArbor.test.ts @@ -67,14 +67,14 @@ describe("useArbor", () => { const { result, unmount } = renderHook(() => useArbor(store)) - const initialSstate = result.current + const initialState = result.current act(() => { result.current.count++ }) const nextState = result.current - expect(initialSstate).not.toBe(nextState) + expect(initialState).not.toBe(nextState) unmount()