From 7787a1c1f31fdb07525ad8d935c6a3bbf0f171e5 Mon Sep 17 00:00:00 2001 From: Chris Lorenzo Date: Wed, 12 Aug 2026 09:36:18 -0400 Subject: [PATCH 1/2] clear savedFocusedElement ptr to free up memory --- src/primitives/KeepAlive.tsx | 49 +++++++++++++++------- tests/keepAlive.test.tsx | 81 ++++++++++++++++++++++++++++++++++++ 2 files changed, 114 insertions(+), 16 deletions(-) create mode 100644 tests/keepAlive.test.tsx diff --git a/src/primitives/KeepAlive.tsx b/src/primitives/KeepAlive.tsx index 2f2d4a2..67811f8 100644 --- a/src/primitives/KeepAlive.tsx +++ b/src/primitives/KeepAlive.tsx @@ -14,6 +14,7 @@ export interface KeepAliveElement { isAlive?: s.Accessor; setIsAlive?: (v: boolean) => void; dispose?: () => void; + savedFocusedElement?: ElementNode | undefined; } const keepAliveElements = new Map(); @@ -37,34 +38,48 @@ export const storeKeepAlive = (element: KeepAliveElement) => export const storeKeepAliveRoute = (element: KeepAliveElement) => _storeKeepAlive(keepAliveRouteElements, element); +const _cleanElement = (element: KeepAliveElement): void => { + element.savedFocusedElement = undefined; + (element.children as unknown as ElementNode | undefined)?.destroy(); + element.dispose?.(); + element.children = undefined; + element.owner = undefined; + element.routeSignal = undefined; + element.isAlive = undefined; + element.setIsAlive = undefined; + element.dispose = undefined; +}; + const _removeKeepAlive = ( map: Map, id: string, ): void => { const element = map.get(id); if (element) { - (element.children as unknown as ElementNode | undefined)?.destroy(); - element.dispose?.(); + _cleanElement(element); map.delete(id); } }; export const removeKeepAlive = (id: string): void => _removeKeepAlive(keepAliveElements, id); -export const removeKeepAliveRoute = (id: string): void => +export const removeKeepAliveRoute = (id: string): void => { + keepAliveRouteCache.delete(id); _removeKeepAlive(keepAliveRouteElements, id); +}; const _clearKeepAlive = (map: Map): void => { map.forEach((element) => { - (element.children as unknown as ElementNode | undefined)?.destroy(); - element.dispose?.(); + _cleanElement(element); }); map.clear(); }; export const clearKeepAlive = (): void => _clearKeepAlive(keepAliveElements); -export const clearKeepAliveRoute = (): void => +export const clearKeepAliveRoute = (): void => { + keepAliveRouteCache.clear(); _clearKeepAlive(keepAliveRouteElements); +}; interface KeepAliveProps { id: string; @@ -118,8 +133,7 @@ const createKeepAliveComponent = ( existing && (props.shouldDispose?.(props.id) || existingChild?.destroyed) ) { - existingChild?.destroy(); - existing.dispose?.(); + _cleanElement(existing); map.delete(props.id); existing = undefined; } @@ -199,8 +213,6 @@ export const KeepAliveRoute = ( return cached; } - let savedFocusedElement: ElementNode | undefined; - const getExisting = (): KeepAliveElement => { let existing = keepAliveRouteElements.get(key); if (!existing) { @@ -212,13 +224,16 @@ export const KeepAliveRoute = ( }; const onRemove = chainFunctions(props.onRemove, (elm: ElementNode) => { - savedFocusedElement = activeElement(); + const existing = getExisting(); + existing.savedFocusedElement = activeElement(); elm.alpha = 0; }); const onRender = chainFunctions(props.onRender, (elm: ElementNode) => { + const existing = keepAliveRouteElements.get(key); + const savedFocused = existing?.savedFocusedElement; let isChild = false; - let current = savedFocusedElement; + let current = savedFocused; while (current) { if (current === elm) { isChild = true; @@ -227,11 +242,14 @@ export const KeepAliveRoute = ( current = current.parent; } - if (isChild && savedFocusedElement) { - savedFocusedElement.setFocus(); + if (isChild && savedFocused) { + savedFocused.setFocus(); } else { elm.setFocus(); } + if (existing) { + existing.savedFocusedElement = undefined; + } elm.alpha = 1; }); @@ -246,8 +264,7 @@ export const KeepAliveRoute = ( existingChild && (props.shouldDispose?.(key) || existingChild.destroyed) ) { - existingChild.destroy(); - existing.dispose?.(); + _cleanElement(existing); keepAliveRouteElements.delete(key); existing = getExisting(); } diff --git a/tests/keepAlive.test.tsx b/tests/keepAlive.test.tsx new file mode 100644 index 0000000..29ec800 --- /dev/null +++ b/tests/keepAlive.test.tsx @@ -0,0 +1,81 @@ +import * as v from 'vitest'; +import * as s from 'solid-js'; +import * as lng from '@solidtv/solid'; + +v.vi.mock('@solidjs/router', () => ({ + Route: (props: any) => props, +})); + +import { renderer } from './setup.js'; + +v.describe('KeepAlive memory & pointer management', () => { + let KeepAliveModule: typeof import('../src/primitives/KeepAlive.js'); + + v.beforeAll(async () => { + KeepAliveModule = await import('../src/primitives/KeepAlive.js'); + }); + + v.afterEach(() => { + KeepAliveModule.clearKeepAlive(); + KeepAliveModule.clearKeepAliveRoute(); + }); + + v.test('clearKeepAliveRoute clears savedFocusedElement and all pointers on KeepAliveElement', () => { + const dummyNode = new lng.ElementNode(renderer.stage); + const mockDispose = v.vi.fn(); + const mockElement: import('../src/primitives/KeepAlive.js').KeepAliveElement = { + id: 'test-route', + children: dummyNode as any, + owner: s.getOwner(), + savedFocusedElement: dummyNode, + dispose: mockDispose, + isAlive: (() => true) as any, + setIsAlive: v.vi.fn(), + }; + + KeepAliveModule.storeKeepAliveRoute(mockElement); + v.expect(KeepAliveModule.keepAliveRouteElements.get('test-route')?.savedFocusedElement).toBe(dummyNode); + + KeepAliveModule.clearKeepAliveRoute(); + + v.expect(KeepAliveModule.keepAliveRouteElements.size).toBe(0); + v.expect(mockDispose).toHaveBeenCalled(); + v.expect(mockElement.savedFocusedElement).toBeUndefined(); + v.expect(mockElement.children).toBeUndefined(); + v.expect(mockElement.owner).toBeUndefined(); + v.expect(mockElement.dispose).toBeUndefined(); + v.expect(mockElement.isAlive).toBeUndefined(); + v.expect(mockElement.setIsAlive).toBeUndefined(); + }); + + v.test('removeKeepAliveRoute clears savedFocusedElement and all pointers for a specific route', () => { + const dummyNode1 = new lng.ElementNode(renderer.stage); + const dummyNode2 = new lng.ElementNode(renderer.stage); + + const elem1: import('../src/primitives/KeepAlive.js').KeepAliveElement = { + id: 'route-1', + children: dummyNode1 as any, + savedFocusedElement: dummyNode1, + dispose: v.vi.fn(), + }; + const elem2: import('../src/primitives/KeepAlive.js').KeepAliveElement = { + id: 'route-2', + children: dummyNode2 as any, + savedFocusedElement: dummyNode2, + dispose: v.vi.fn(), + }; + + KeepAliveModule.storeKeepAliveRoute(elem1); + KeepAliveModule.storeKeepAliveRoute(elem2); + + KeepAliveModule.removeKeepAliveRoute('route-1'); + + v.expect(KeepAliveModule.keepAliveRouteElements.has('route-1')).toBe(false); + v.expect(elem1.savedFocusedElement).toBeUndefined(); + v.expect(elem1.children).toBeUndefined(); + v.expect(elem1.dispose).toBeUndefined(); + + v.expect(KeepAliveModule.keepAliveRouteElements.has('route-2')).true; + v.expect(elem2.savedFocusedElement).toBe(dummyNode2); + }); +}); From f86b459bd4a77a0ba246886fb1d9ea13053b07ec Mon Sep 17 00:00:00 2001 From: Chris Lorenzo Date: Wed, 12 Aug 2026 09:46:03 -0400 Subject: [PATCH 2/2] fix(KeepAlive): release savedFocusedElement with the route entry savedFocusedElement lived in a KeepAliveRoute closure that is retained forever by keepAliveRouteCache, so a node captured on route exit stayed reachable even after the route was disposed or cleared. Move the pointer onto the KeepAliveElement record: dropping the entry from keepAliveRouteElements now drops the saved node with it. Also clear it once focus has been restored. Co-Authored-By: Claude Opus 5 --- src/primitives/KeepAlive.tsx | 58 ++++++------- tests/keepAlive.test.tsx | 148 +++++++++++++++++++--------------- tests/stubs/solidjs-router.ts | 4 + vitest.config.ts | 9 +++ 4 files changed, 120 insertions(+), 99 deletions(-) create mode 100644 tests/stubs/solidjs-router.ts diff --git a/src/primitives/KeepAlive.tsx b/src/primitives/KeepAlive.tsx index 67811f8..71a03e7 100644 --- a/src/primitives/KeepAlive.tsx +++ b/src/primitives/KeepAlive.tsx @@ -14,7 +14,10 @@ export interface KeepAliveElement { isAlive?: s.Accessor; setIsAlive?: (v: boolean) => void; dispose?: () => void; - savedFocusedElement?: ElementNode | undefined; + // Focused node captured on route exit so it can be refocused on re-entry. + // Stored here (rather than in a KeepAliveRoute closure) so it becomes + // garbage as soon as the entry is dropped from the map. + savedFocusedElement?: ElementNode; } const keepAliveElements = new Map(); @@ -38,48 +41,34 @@ export const storeKeepAlive = (element: KeepAliveElement) => export const storeKeepAliveRoute = (element: KeepAliveElement) => _storeKeepAlive(keepAliveRouteElements, element); -const _cleanElement = (element: KeepAliveElement): void => { - element.savedFocusedElement = undefined; - (element.children as unknown as ElementNode | undefined)?.destroy(); - element.dispose?.(); - element.children = undefined; - element.owner = undefined; - element.routeSignal = undefined; - element.isAlive = undefined; - element.setIsAlive = undefined; - element.dispose = undefined; -}; - const _removeKeepAlive = ( map: Map, id: string, ): void => { const element = map.get(id); if (element) { - _cleanElement(element); + (element.children as unknown as ElementNode | undefined)?.destroy(); + element.dispose?.(); map.delete(id); } }; export const removeKeepAlive = (id: string): void => _removeKeepAlive(keepAliveElements, id); -export const removeKeepAliveRoute = (id: string): void => { - keepAliveRouteCache.delete(id); +export const removeKeepAliveRoute = (id: string): void => _removeKeepAlive(keepAliveRouteElements, id); -}; const _clearKeepAlive = (map: Map): void => { map.forEach((element) => { - _cleanElement(element); + (element.children as unknown as ElementNode | undefined)?.destroy(); + element.dispose?.(); }); map.clear(); }; export const clearKeepAlive = (): void => _clearKeepAlive(keepAliveElements); -export const clearKeepAliveRoute = (): void => { - keepAliveRouteCache.clear(); +export const clearKeepAliveRoute = (): void => _clearKeepAlive(keepAliveRouteElements); -}; interface KeepAliveProps { id: string; @@ -133,7 +122,8 @@ const createKeepAliveComponent = ( existing && (props.shouldDispose?.(props.id) || existingChild?.destroyed) ) { - _cleanElement(existing); + existingChild?.destroy(); + existing.dispose?.(); map.delete(props.id); existing = undefined; } @@ -224,16 +214,22 @@ export const KeepAliveRoute = ( }; const onRemove = chainFunctions(props.onRemove, (elm: ElementNode) => { - const existing = getExisting(); - existing.savedFocusedElement = activeElement(); + const existing = keepAliveRouteElements.get(key); + if (existing) { + existing.savedFocusedElement = activeElement(); + } elm.alpha = 0; }); const onRender = chainFunctions(props.onRender, (elm: ElementNode) => { const existing = keepAliveRouteElements.get(key); - const savedFocused = existing?.savedFocusedElement; + const savedFocusedElement = existing?.savedFocusedElement; + if (existing) { + existing.savedFocusedElement = undefined; + } + let isChild = false; - let current = savedFocused; + let current = savedFocusedElement; while (current) { if (current === elm) { isChild = true; @@ -242,14 +238,11 @@ export const KeepAliveRoute = ( current = current.parent; } - if (isChild && savedFocused) { - savedFocused.setFocus(); + if (isChild && savedFocusedElement) { + savedFocusedElement.setFocus(); } else { elm.setFocus(); } - if (existing) { - existing.savedFocusedElement = undefined; - } elm.alpha = 1; }); @@ -264,7 +257,8 @@ export const KeepAliveRoute = ( existingChild && (props.shouldDispose?.(key) || existingChild.destroyed) ) { - _cleanElement(existing); + existingChild.destroy(); + existing.dispose?.(); keepAliveRouteElements.delete(key); existing = getExisting(); } diff --git a/tests/keepAlive.test.tsx b/tests/keepAlive.test.tsx index 29ec800..6aa3244 100644 --- a/tests/keepAlive.test.tsx +++ b/tests/keepAlive.test.tsx @@ -1,81 +1,95 @@ import * as v from 'vitest'; -import * as s from 'solid-js'; import * as lng from '@solidtv/solid'; -v.vi.mock('@solidjs/router', () => ({ - Route: (props: any) => props, -})); - +// @solidjs/router is aliased to tests/stubs/solidjs-router.ts, whose +// hands its props straight back — which is all this test needs. +import { + KeepAliveRoute, + keepAliveRouteElements, + removeKeepAliveRoute, + clearKeepAliveRoute, + clearKeepAliveRouteCache, +} from '../src/primitives/KeepAlive.jsx'; import { renderer } from './setup.js'; -v.describe('KeepAlive memory & pointer management', () => { - let KeepAliveModule: typeof import('../src/primitives/KeepAlive.js'); +const wait = (ms = 10) => new Promise((r) => setTimeout(r, ms)); - v.beforeAll(async () => { - KeepAliveModule = await import('../src/primitives/KeepAlive.js'); - }); +// Renders the route's component wrapper and returns the KeepAlive , +// which carries the chained onRemove/onRender we want to exercise. +const renderRoute = (path: string) => { + const routeProps = KeepAliveRoute({ + path, + component: () => ( + + + + ), + }) as any; + + let outer!: lng.ElementNode; + const dispose = renderer.render(() => ( + + {routeProps.component({})} + + )); + + return { keepAliveView: outer.children[0] as lng.ElementNode, dispose }; +}; +v.describe('KeepAliveRoute saved focus', () => { v.afterEach(() => { - KeepAliveModule.clearKeepAlive(); - KeepAliveModule.clearKeepAliveRoute(); + clearKeepAliveRoute(); + clearKeepAliveRouteCache(); }); - v.test('clearKeepAliveRoute clears savedFocusedElement and all pointers on KeepAliveElement', () => { - const dummyNode = new lng.ElementNode(renderer.stage); - const mockDispose = v.vi.fn(); - const mockElement: import('../src/primitives/KeepAlive.js').KeepAliveElement = { - id: 'test-route', - children: dummyNode as any, - owner: s.getOwner(), - savedFocusedElement: dummyNode, - dispose: mockDispose, - isAlive: (() => true) as any, - setIsAlive: v.vi.fn(), - }; - - KeepAliveModule.storeKeepAliveRoute(mockElement); - v.expect(KeepAliveModule.keepAliveRouteElements.get('test-route')?.savedFocusedElement).toBe(dummyNode); - - KeepAliveModule.clearKeepAliveRoute(); - - v.expect(KeepAliveModule.keepAliveRouteElements.size).toBe(0); - v.expect(mockDispose).toHaveBeenCalled(); - v.expect(mockElement.savedFocusedElement).toBeUndefined(); - v.expect(mockElement.children).toBeUndefined(); - v.expect(mockElement.owner).toBeUndefined(); - v.expect(mockElement.dispose).toBeUndefined(); - v.expect(mockElement.isAlive).toBeUndefined(); - v.expect(mockElement.setIsAlive).toBeUndefined(); + v.test('stores the focused element on the map entry, then releases it on re-entry', async () => { + const { keepAliveView, dispose } = renderRoute('/stores'); + await wait(); + + const focused = keepAliveView.children[0]!.children[0] as lng.ElementNode; + focused.setFocus(); + await wait(); + v.expect(lng.activeElement()).toBe(focused); + + keepAliveView.onRemove!(keepAliveView); + v.expect(keepAliveRouteElements.get('/stores')!.savedFocusedElement).toBe( + focused, + ); + + keepAliveView.onRender!(keepAliveView); + await wait(); + v.expect(lng.activeElement()).toBe(focused); + // Pointer dropped once it has been used — nothing left to retain. + v.expect( + keepAliveRouteElements.get('/stores')!.savedFocusedElement, + ).toBeUndefined(); + + dispose(); }); - v.test('removeKeepAliveRoute clears savedFocusedElement and all pointers for a specific route', () => { - const dummyNode1 = new lng.ElementNode(renderer.stage); - const dummyNode2 = new lng.ElementNode(renderer.stage); - - const elem1: import('../src/primitives/KeepAlive.js').KeepAliveElement = { - id: 'route-1', - children: dummyNode1 as any, - savedFocusedElement: dummyNode1, - dispose: v.vi.fn(), - }; - const elem2: import('../src/primitives/KeepAlive.js').KeepAliveElement = { - id: 'route-2', - children: dummyNode2 as any, - savedFocusedElement: dummyNode2, - dispose: v.vi.fn(), - }; - - KeepAliveModule.storeKeepAliveRoute(elem1); - KeepAliveModule.storeKeepAliveRoute(elem2); - - KeepAliveModule.removeKeepAliveRoute('route-1'); - - v.expect(KeepAliveModule.keepAliveRouteElements.has('route-1')).toBe(false); - v.expect(elem1.savedFocusedElement).toBeUndefined(); - v.expect(elem1.children).toBeUndefined(); - v.expect(elem1.dispose).toBeUndefined(); - - v.expect(KeepAliveModule.keepAliveRouteElements.has('route-2')).true; - v.expect(elem2.savedFocusedElement).toBe(dummyNode2); + v.test('dropping the map entry drops the saved element with it', async () => { + const { keepAliveView, dispose } = renderRoute('/dropped'); + await wait(); + + const focused = keepAliveView.children[0]!.children[0] as lng.ElementNode; + focused.setFocus(); + await wait(); + + keepAliveView.onRemove!(keepAliveView); + v.expect(keepAliveRouteElements.get('/dropped')!.savedFocusedElement).toBe( + focused, + ); + + // No closure holds the pointer, so removing the entry is enough to make + // the saved element collectable. Re-entering falls back to the route + // element instead of refocusing a node from the torn-down subtree. + removeKeepAliveRoute('/dropped'); + v.expect(keepAliveRouteElements.has('/dropped')).toBe(false); + + keepAliveView.onRender!(keepAliveView); + await wait(); + v.expect(lng.activeElement()).not.toBe(focused); + + dispose(); }); }); diff --git a/tests/stubs/solidjs-router.ts b/tests/stubs/solidjs-router.ts new file mode 100644 index 0000000..8303c0b --- /dev/null +++ b/tests/stubs/solidjs-router.ts @@ -0,0 +1,4 @@ +// The real @solidjs/router ships untranspiled .jsx compiled against a +// different solid moduleName, which vitest can't load. Nothing under test +// needs router behaviour — only the props KeepAliveRoute hands to . +export const Route = (props: unknown) => props; diff --git a/vitest.config.ts b/vitest.config.ts index 4d12e2d..18fe28b 100644 --- a/vitest.config.ts +++ b/vitest.config.ts @@ -24,5 +24,14 @@ export default defineConfig(({ mode }) => ({ }, resolve: { conditions: ['@solidtv/source', 'browser', 'development'], + alias: { + // @solidjs/router resolves to untranspiled .jsx under the + // `@solidtv/source` condition, which vitest can't load. Tests only + // need to exist. + '@solidjs/router': new URL( + './tests/stubs/solidjs-router.ts', + import.meta.url, + ).pathname, + }, }, }));