From 7fa5af59ff6ab3b67fc7bf2e74334e3746af9068 Mon Sep 17 00:00:00 2001 From: Kam Date: Fri, 9 Oct 2026 23:49:37 +0300 Subject: [PATCH 1/2] fix(overlay): resend signal history after a failed push and sweep element ids The NativeScript overlay kept historyDelta true after a failed signal graph push, so the retry sent a delta against history the server never received. It now resets it like the Angular Native overlay. elementId now sweeps dead and disconnected entries once the id map doubles from its size after the last sweep, so collectors that run without the Components inspector no longer grow it without bound. --- .../devtools/src/__tests__/element-id.test.ts | 34 ++++++ .../overlay-nativescript-signal-push.test.ts | 113 ++++++++++++++++++ packages/devtools/src/element-id.ts | 15 +++ packages/devtools/src/overlay-nativescript.ts | 1 + 4 files changed, 163 insertions(+) create mode 100644 packages/devtools/src/__tests__/element-id.test.ts create mode 100644 packages/devtools/src/__tests__/overlay-nativescript-signal-push.test.ts diff --git a/packages/devtools/src/__tests__/element-id.test.ts b/packages/devtools/src/__tests__/element-id.test.ts new file mode 100644 index 00000000..a26f2cd4 --- /dev/null +++ b/packages/devtools/src/__tests__/element-id.test.ts @@ -0,0 +1,34 @@ +import { describe, expect, it } from 'vitest'; +import { elementById, elementId, elementIdCount, pruneElementIds } from '../element-id.ts'; + +const attached = () => ({ isConnected: true }); +const detached = () => ({ isConnected: false }); + +describe('element ids', () => { + it('keeps the map bounded when elements are detached and nothing prunes it', () => { + const keeper = attached(); + const keeperId = elementId(keeper); + for (let i = 0; i < 5000; i++) elementId(detached()); + expect(elementIdCount()).toBeLessThan(1200); + expect(elementId(keeper)).toBe(keeperId); + expect(elementById(keeperId)).toBe(keeper); + }); + + it('keeps ids of connected elements stable across many sweeps', () => { + const live = Array.from({ length: 300 }, attached); + const first = live.map((el) => elementId(el)); + for (let i = 0; i < 4000; i++) elementId(detached()); + expect(live.map((el) => elementId(el))).toEqual(first); + expect(elementById(first[0])).toBe(live[0]); + }); + + it('sweeps with the predicate the last explicit prune used', () => { + const view = { alive: true }; + const connected = (host: object) => (host as { alive?: boolean }).alive === true; + pruneElementIds(connected); + const id = elementId(view); + for (let i = 0; i < 4000; i++) elementId({ alive: false }); + expect(elementById(id, connected)).toBe(view); + pruneElementIds(); + }); +}); diff --git a/packages/devtools/src/__tests__/overlay-nativescript-signal-push.test.ts b/packages/devtools/src/__tests__/overlay-nativescript-signal-push.test.ts new file mode 100644 index 00000000..640c2265 --- /dev/null +++ b/packages/devtools/src/__tests__/overlay-nativescript-signal-push.test.ts @@ -0,0 +1,113 @@ +import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'; + +type Call = [string, Record]; + +const calls: Call[] = []; +let signalValue = 0; +let failNext = false; + +const host = { + typeName: 'AppHostView', + parent: null, + isLoaded: true, + eachChildView: () => undefined, +}; +class AppComponent {} +Object.assign(AppComponent, { ɵcmp: { selectors: [['ns-app']] } }); +const app = new AppComponent(); + +vi.mock('@nativescript/core', () => ({ + Application: { getRootView: () => host }, + isAndroid: false, +})); + +vi.mock('devframe/client', () => ({ + connectDevframe: vi.fn(async () => ({ + status: 'connected', + close: vi.fn(), + scope: () => ({ + rpc: { + register: vi.fn(), + call: vi.fn(async (name: string, payload: Record) => { + if (name === 'push-signal-graph') { + if (failNext) { + failNext = false; + throw new Error('offline'); + } + calls.push([name, payload]); + return { delta: true }; + } + return undefined; + }), + }, + }), + })), +})); + +vi.mock('../signal-history.ts', async (importOriginal) => ({ + ...(await importOriginal()), + installSignalWriteHook: vi.fn(async () => () => undefined), +})); + +vi.mock('../ngrx-overlay.ts', () => ({ + attachNgrx: () => ({ push: async () => undefined, stop: () => undefined }), +})); + +async function flush() { + for (let i = 0; i < 40; i++) await Promise.resolve(); +} + +describe('NativeScript signal graph pushes', () => { + beforeEach(() => { + vi.useFakeTimers(); + vi.stubGlobal('WebSocket', class {}); + vi.spyOn(console, 'log').mockImplementation(() => undefined); + vi.spyOn(console, 'warn').mockImplementation(() => undefined); + calls.length = 0; + signalValue = 0; + failNext = false; + (globalThis as Record)['ng'] = { + getComponent: (h: unknown) => (h === host ? app : null), + getRootComponents: () => [app], + getHostElement: (c: unknown) => (c === app ? host : null), + getInjector: (h: unknown) => h, + ɵgetSignalGraph: () => ({ + nodes: [ + { id: '1', kind: 'signal', label: 'count', epoch: signalValue, value: signalValue }, + ], + }), + }; + }); + + afterEach(() => { + vi.useRealTimers(); + vi.unstubAllGlobals(); + vi.restoreAllMocks(); + delete (globalThis as Record)['ng']; + }); + + it('sends the full history again after a failed push', async () => { + const { initNativeScriptOverlay } = await import('../overlay-nativescript.ts'); + const dispose = initNativeScriptOverlay({ baseURL: 'http://localhost:9999/', intervalMs: 100 }); + await flush(); + expect(calls).toHaveLength(1); + expect(calls[0][1]).toHaveProperty('history'); + + signalValue = 1; + await vi.advanceTimersByTimeAsync(100); + expect(calls).toHaveLength(2); + expect(calls[1][1]).toHaveProperty('historyDelta'); + + signalValue = 2; + failNext = true; + await vi.advanceTimersByTimeAsync(100); + expect(calls).toHaveLength(2); + + signalValue = 3; + await vi.advanceTimersByTimeAsync(100); + expect(calls).toHaveLength(3); + expect(calls[2][1]).toHaveProperty('history'); + expect(calls[2][1]).not.toHaveProperty('historyDelta'); + dispose(); + }); +}); diff --git a/packages/devtools/src/element-id.ts b/packages/devtools/src/element-id.ts index f129d06f..acec61a0 100644 --- a/packages/devtools/src/element-id.ts +++ b/packages/devtools/src/element-id.ts @@ -2,12 +2,17 @@ const ids = new WeakMap(); const byId = new Map>(); const load = Math.random().toString(36).slice(2, 6).padEnd(4, '0'); let nextId = 0; +const FIRST_SWEEP = 512; +let sweepAt = FIRST_SWEEP; type Connected = (host: object) => boolean; const isConnected: Connected = (host) => (host as { isConnected?: unknown }).isConnected === true; +let lastConnected: Connected = isConnected; + export function elementId(el: object): string { + if (byId.size >= sweepAt) sweep(lastConnected); let id = ids.get(el); if (!id) { id = `c${load}-${++nextId}`; @@ -29,8 +34,18 @@ export function elementById(id: string, connected: Connected = isConnected): obj } export function pruneElementIds(connected: Connected = isConnected) { + lastConnected = connected; + sweep(connected); +} + +export function elementIdCount(): number { + return byId.size; +} + +function sweep(connected: Connected) { for (const [id, ref] of byId) { const el = ref.deref(); if (!el || !connected(el)) byId.delete(id); } + sweepAt = Math.max(FIRST_SWEEP, byId.size * 2); } diff --git a/packages/devtools/src/overlay-nativescript.ts b/packages/devtools/src/overlay-nativescript.ts index d2a1c723..eb0c0c0c 100644 --- a/packages/devtools/src/overlay-nativescript.ts +++ b/packages/devtools/src/overlay-nativescript.ts @@ -207,6 +207,7 @@ async function startSession( } catch (error) { rollback(); lastSignalKey = ''; + historyDelta = false; throw error; } historyDelta = answer?.delta === true; From 68eb5886e4ee722cb174cf683224fde940b36132 Mon Sep 17 00:00:00 2001 From: Kam Date: Sat, 10 Oct 2026 00:09:04 +0300 Subject: [PATCH 2/2] fix(overlay): sweep element ids without a retained predicate Automatic sweeps reused the host-only predicate from the last component tree prune, so connected elements registered by other collectors were dropped. Sweeps now remove only dead references and objects whose isConnected is explicitly false, and keep objects with no isConnected. An explicit pruneElementIds call still uses its own predicate. --- .../devtools/src/__tests__/element-id.test.ts | 69 ++++++++++++++++--- packages/devtools/src/element-id.ts | 6 +- 2 files changed, 61 insertions(+), 14 deletions(-) diff --git a/packages/devtools/src/__tests__/element-id.test.ts b/packages/devtools/src/__tests__/element-id.test.ts index a26f2cd4..95a813a2 100644 --- a/packages/devtools/src/__tests__/element-id.test.ts +++ b/packages/devtools/src/__tests__/element-id.test.ts @@ -1,14 +1,23 @@ -import { describe, expect, it } from 'vitest'; +import { afterEach, describe, expect, it, vi } from 'vitest'; import { elementById, elementId, elementIdCount, pruneElementIds } from '../element-id.ts'; const attached = () => ({ isConnected: true }); const detached = () => ({ isConnected: false }); +function fill(count: number, make: () => object = detached) { + for (let i = 0; i < count; i++) elementId(make()); +} + describe('element ids', () => { + afterEach(() => { + vi.unstubAllGlobals(); + pruneElementIds(); + }); + it('keeps the map bounded when elements are detached and nothing prunes it', () => { const keeper = attached(); const keeperId = elementId(keeper); - for (let i = 0; i < 5000; i++) elementId(detached()); + fill(5000); expect(elementIdCount()).toBeLessThan(1200); expect(elementId(keeper)).toBe(keeperId); expect(elementById(keeperId)).toBe(keeper); @@ -17,18 +26,56 @@ describe('element ids', () => { it('keeps ids of connected elements stable across many sweeps', () => { const live = Array.from({ length: 300 }, attached); const first = live.map((el) => elementId(el)); - for (let i = 0; i < 4000; i++) elementId(detached()); - expect(live.map((el) => elementId(el))).toEqual(first); + fill(4000); expect(elementById(first[0])).toBe(live[0]); + expect(live.map((el) => elementId(el))).toEqual(first); }); - it('sweeps with the predicate the last explicit prune used', () => { - const view = { alive: true }; - const connected = (host: object) => (host as { alive?: boolean }).alive === true; - pruneElementIds(connected); + it('keeps connected non-host elements after a host-only prune and an automatic sweep', () => { + const hosts = new WeakSet(); + const host = attached(); + hosts.add(host); + pruneElementIds( + (el) => hosts.has(el) && (el as { isConnected?: boolean }).isConnected === true, + ); + const other = attached(); + const otherId = elementId(other); + const gone = detached(); + const goneId = elementId(gone); + fill(4000); + expect(elementById(otherId)).toBe(other); + expect(elementById(goneId)).toBeNull(); + expect(elementId(other)).toBe(otherId); + }); + + it('keeps objects that have no isConnected until they are collected', () => { + const view = { typeName: 'Label' }; const id = elementId(view); - for (let i = 0; i < 4000; i++) elementId({ alive: false }); - expect(elementById(id, connected)).toBe(view); - pruneElementIds(); + fill(4000); + expect(elementById(id, () => true)).toBe(view); + expect(elementId(view)).toBe(id); + }); + + it('drops entries whose reference is dead', () => { + const dead = new WeakSet(); + const Real = WeakRef; + vi.stubGlobal( + 'WeakRef', + class { + private readonly ref: WeakRef; + constructor(target: T) { + this.ref = new Real(target); + } + deref() { + const target = this.ref.deref(); + return target && dead.has(target) ? undefined : target; + } + }, + ); + const doomed = attached(); + const doomedId = elementId(doomed); + dead.add(doomed); + fill(4000, attached); + expect(elementById(doomedId)).toBeNull(); }); }); diff --git a/packages/devtools/src/element-id.ts b/packages/devtools/src/element-id.ts index acec61a0..bf98a12b 100644 --- a/packages/devtools/src/element-id.ts +++ b/packages/devtools/src/element-id.ts @@ -9,10 +9,11 @@ type Connected = (host: object) => boolean; const isConnected: Connected = (host) => (host as { isConnected?: unknown }).isConnected === true; -let lastConnected: Connected = isConnected; +const notDisconnected: Connected = (host) => + (host as { isConnected?: unknown }).isConnected !== false; export function elementId(el: object): string { - if (byId.size >= sweepAt) sweep(lastConnected); + if (byId.size >= sweepAt) sweep(notDisconnected); let id = ids.get(el); if (!id) { id = `c${load}-${++nextId}`; @@ -34,7 +35,6 @@ export function elementById(id: string, connected: Connected = isConnected): obj } export function pruneElementIds(connected: Connected = isConnected) { - lastConnected = connected; sweep(connected); }