Skip to content

Commit 2a7b165

Browse files
authored
fix(overlay): resend signal history after a failed push and sweep element ids (#237)
* 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. * 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.
1 parent 8581fd2 commit 2a7b165

4 files changed

Lines changed: 210 additions & 0 deletions

File tree

Lines changed: 81 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,81 @@
1+
import { afterEach, describe, expect, it, vi } from 'vitest';
2+
import { elementById, elementId, elementIdCount, pruneElementIds } from '../element-id.ts';
3+
4+
const attached = () => ({ isConnected: true });
5+
const detached = () => ({ isConnected: false });
6+
7+
function fill(count: number, make: () => object = detached) {
8+
for (let i = 0; i < count; i++) elementId(make());
9+
}
10+
11+
describe('element ids', () => {
12+
afterEach(() => {
13+
vi.unstubAllGlobals();
14+
pruneElementIds();
15+
});
16+
17+
it('keeps the map bounded when elements are detached and nothing prunes it', () => {
18+
const keeper = attached();
19+
const keeperId = elementId(keeper);
20+
fill(5000);
21+
expect(elementIdCount()).toBeLessThan(1200);
22+
expect(elementId(keeper)).toBe(keeperId);
23+
expect(elementById(keeperId)).toBe(keeper);
24+
});
25+
26+
it('keeps ids of connected elements stable across many sweeps', () => {
27+
const live = Array.from({ length: 300 }, attached);
28+
const first = live.map((el) => elementId(el));
29+
fill(4000);
30+
expect(elementById(first[0])).toBe(live[0]);
31+
expect(live.map((el) => elementId(el))).toEqual(first);
32+
});
33+
34+
it('keeps connected non-host elements after a host-only prune and an automatic sweep', () => {
35+
const hosts = new WeakSet<object>();
36+
const host = attached();
37+
hosts.add(host);
38+
pruneElementIds(
39+
(el) => hosts.has(el) && (el as { isConnected?: boolean }).isConnected === true,
40+
);
41+
const other = attached();
42+
const otherId = elementId(other);
43+
const gone = detached();
44+
const goneId = elementId(gone);
45+
fill(4000);
46+
expect(elementById(otherId)).toBe(other);
47+
expect(elementById(goneId)).toBeNull();
48+
expect(elementId(other)).toBe(otherId);
49+
});
50+
51+
it('keeps objects that have no isConnected until they are collected', () => {
52+
const view = { typeName: 'Label' };
53+
const id = elementId(view);
54+
fill(4000);
55+
expect(elementById(id, () => true)).toBe(view);
56+
expect(elementId(view)).toBe(id);
57+
});
58+
59+
it('drops entries whose reference is dead', () => {
60+
const dead = new WeakSet<object>();
61+
const Real = WeakRef;
62+
vi.stubGlobal(
63+
'WeakRef',
64+
class<T extends object> {
65+
private readonly ref: WeakRef<T>;
66+
constructor(target: T) {
67+
this.ref = new Real(target);
68+
}
69+
deref() {
70+
const target = this.ref.deref();
71+
return target && dead.has(target) ? undefined : target;
72+
}
73+
},
74+
);
75+
const doomed = attached();
76+
const doomedId = elementId(doomed);
77+
dead.add(doomed);
78+
fill(4000, attached);
79+
expect(elementById(doomedId)).toBeNull();
80+
});
81+
});
Lines changed: 113 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,113 @@
1+
import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest';
2+
3+
type Call = [string, Record<string, unknown>];
4+
5+
const calls: Call[] = [];
6+
let signalValue = 0;
7+
let failNext = false;
8+
9+
const host = {
10+
typeName: 'AppHostView',
11+
parent: null,
12+
isLoaded: true,
13+
eachChildView: () => undefined,
14+
};
15+
class AppComponent {}
16+
Object.assign(AppComponent, { ɵcmp: { selectors: [['ns-app']] } });
17+
const app = new AppComponent();
18+
19+
vi.mock('@nativescript/core', () => ({
20+
Application: { getRootView: () => host },
21+
isAndroid: false,
22+
}));
23+
24+
vi.mock('devframe/client', () => ({
25+
connectDevframe: vi.fn(async () => ({
26+
status: 'connected',
27+
close: vi.fn(),
28+
scope: () => ({
29+
rpc: {
30+
register: vi.fn(),
31+
call: vi.fn(async (name: string, payload: Record<string, unknown>) => {
32+
if (name === 'push-signal-graph') {
33+
if (failNext) {
34+
failNext = false;
35+
throw new Error('offline');
36+
}
37+
calls.push([name, payload]);
38+
return { delta: true };
39+
}
40+
return undefined;
41+
}),
42+
},
43+
}),
44+
})),
45+
}));
46+
47+
vi.mock('../signal-history.ts', async (importOriginal) => ({
48+
...(await importOriginal<typeof import('../signal-history.ts')>()),
49+
installSignalWriteHook: vi.fn(async () => () => undefined),
50+
}));
51+
52+
vi.mock('../ngrx-overlay.ts', () => ({
53+
attachNgrx: () => ({ push: async () => undefined, stop: () => undefined }),
54+
}));
55+
56+
async function flush() {
57+
for (let i = 0; i < 40; i++) await Promise.resolve();
58+
}
59+
60+
describe('NativeScript signal graph pushes', () => {
61+
beforeEach(() => {
62+
vi.useFakeTimers();
63+
vi.stubGlobal('WebSocket', class {});
64+
vi.spyOn(console, 'log').mockImplementation(() => undefined);
65+
vi.spyOn(console, 'warn').mockImplementation(() => undefined);
66+
calls.length = 0;
67+
signalValue = 0;
68+
failNext = false;
69+
(globalThis as Record<string, unknown>)['ng'] = {
70+
getComponent: (h: unknown) => (h === host ? app : null),
71+
getRootComponents: () => [app],
72+
getHostElement: (c: unknown) => (c === app ? host : null),
73+
getInjector: (h: unknown) => h,
74+
ɵgetSignalGraph: () => ({
75+
nodes: [
76+
{ id: '1', kind: 'signal', label: 'count', epoch: signalValue, value: signalValue },
77+
],
78+
}),
79+
};
80+
});
81+
82+
afterEach(() => {
83+
vi.useRealTimers();
84+
vi.unstubAllGlobals();
85+
vi.restoreAllMocks();
86+
delete (globalThis as Record<string, unknown>)['ng'];
87+
});
88+
89+
it('sends the full history again after a failed push', async () => {
90+
const { initNativeScriptOverlay } = await import('../overlay-nativescript.ts');
91+
const dispose = initNativeScriptOverlay({ baseURL: 'http://localhost:9999/', intervalMs: 100 });
92+
await flush();
93+
expect(calls).toHaveLength(1);
94+
expect(calls[0][1]).toHaveProperty('history');
95+
96+
signalValue = 1;
97+
await vi.advanceTimersByTimeAsync(100);
98+
expect(calls).toHaveLength(2);
99+
expect(calls[1][1]).toHaveProperty('historyDelta');
100+
101+
signalValue = 2;
102+
failNext = true;
103+
await vi.advanceTimersByTimeAsync(100);
104+
expect(calls).toHaveLength(2);
105+
106+
signalValue = 3;
107+
await vi.advanceTimersByTimeAsync(100);
108+
expect(calls).toHaveLength(3);
109+
expect(calls[2][1]).toHaveProperty('history');
110+
expect(calls[2][1]).not.toHaveProperty('historyDelta');
111+
dispose();
112+
});
113+
});

‎packages/devtools/src/element-id.ts‎

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2,12 +2,18 @@ const ids = new WeakMap<object, string>();
22
const byId = new Map<string, WeakRef<object>>();
33
const load = Math.random().toString(36).slice(2, 6).padEnd(4, '0');
44
let nextId = 0;
5+
const FIRST_SWEEP = 512;
6+
let sweepAt = FIRST_SWEEP;
57

68
type Connected = (host: object) => boolean;
79

810
const isConnected: Connected = (host) => (host as { isConnected?: unknown }).isConnected === true;
911

12+
const notDisconnected: Connected = (host) =>
13+
(host as { isConnected?: unknown }).isConnected !== false;
14+
1015
export function elementId(el: object): string {
16+
if (byId.size >= sweepAt) sweep(notDisconnected);
1117
let id = ids.get(el);
1218
if (!id) {
1319
id = `c${load}-${++nextId}`;
@@ -29,8 +35,17 @@ export function elementById(id: string, connected: Connected = isConnected): obj
2935
}
3036

3137
export function pruneElementIds(connected: Connected = isConnected) {
38+
sweep(connected);
39+
}
40+
41+
export function elementIdCount(): number {
42+
return byId.size;
43+
}
44+
45+
function sweep(connected: Connected) {
3246
for (const [id, ref] of byId) {
3347
const el = ref.deref();
3448
if (!el || !connected(el)) byId.delete(id);
3549
}
50+
sweepAt = Math.max(FIRST_SWEEP, byId.size * 2);
3651
}

‎packages/devtools/src/overlay-nativescript.ts‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -217,6 +217,7 @@ async function startSession(
217217
} catch (error) {
218218
rollback();
219219
lastSignalKey = '';
220+
historyDelta = false;
220221
throw error;
221222
}
222223
historyDelta = answer?.delta === true;

0 commit comments

Comments
 (0)