Skip to content

Commit c6ce71d

Browse files
HexaFieldclaude
andcommitted
feat(schema-renderer): abort in-flight findAll on cleanup and re-run
SchemaRenderer's `createQuerySignal` and `$single` paths both run `ModelClass.findAll` inside a `createEffect`. The effect re-runs on any reactive dependency change (perspective swap, params token update) and the component can unmount mid-query. Without cancellation the stale findAll keeps grinding through SPARQL serialisation, network round-trip, and client-side deserialisation — work that's discarded the moment a fresher result arrives. Wires an `AbortController` scoped to each effect iteration: - `onCleanup` aborts the controller before the effect re-runs or the component unmounts. - The `findAll(p, queryOptions, { signal })` call forwards the signal to ad4m's `Ad4mModel.findAll` (extended in coasys/ad4m#855 to thread the signal through `modelQuery` → `apiClient.call` → the executor's `request.cancel` WebSocket message). - `.then` guards on `controller.signal.aborted` to ignore the stale result if the controller fired before the promise resolved. - `.catch` swallows `DOMException('Aborted', 'AbortError')` so cancellation isn't surfaced to the UI as an error — the new effect run (or unmount) handles state instead. The subscribe path already cleans up via `builder.dispose()` and isn't changed. Caveat: the executor's Oxigraph SPARQL engine can't be interrupted mid-evaluation; what's saved is the JSON serialise + WebSocket reply + client deserialise tax, which dominates for any non-trivial result set. Tests (queryToken.test.tsx): - findAll receives an AbortSignal in `options.signal`. - Unmount aborts the controller. - Re-running the effect (perspective signal change) aborts the prior controller and gives the new run a fresh, un-aborted signal. - AbortError rejection is swallowed without surfacing. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
1 parent deb4c5b commit c6ce71d

2 files changed

Lines changed: 130 additions & 4 deletions

File tree

packages/schema-system/frameworks/solid/src/SchemaRenderer.tsx

Lines changed: 38 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -235,9 +235,29 @@ function createQuerySignal(
235235
});
236236
onCleanup(() => builder.dispose());
237237
} else {
238-
(ModelClass.findAll(p, queryOptions) as Promise<unknown[]>).then((results) => {
239-
setItems(reconcile(normalise(results), { key: 'id', merge: true }));
240-
});
238+
// The effect re-runs when any reactive dep changes — perspective swap,
239+
// resolved params, etc. When that happens (or the component unmounts)
240+
// the previous findAll may still be in flight against a slow query.
241+
// An AbortController scoped to this iteration tells the executor to
242+
// drop the JSON reply for the stale query instead of paying its
243+
// serialise + WebSocket + deserialise cost.
244+
//
245+
// ad4m's `Ad4mModel.findAll` accepts `options?: { signal?: AbortSignal }`
246+
// as the 3rd argument and forwards it through `perspective.modelQuery`
247+
// to the executor's `request.cancel` machinery.
248+
const controller = new AbortController();
249+
onCleanup(() => controller.abort());
250+
(ModelClass.findAll(p, queryOptions, { signal: controller.signal }) as Promise<unknown[]>)
251+
.then((results) => {
252+
if (controller.signal.aborted) return;
253+
setItems(reconcile(normalise(results), { key: 'id', merge: true }));
254+
})
255+
.catch((err) => {
256+
// AbortError = a newer effect run or unmount cancelled this query.
257+
// Silently drop — no UI state to update, the new run handles it.
258+
if (err instanceof DOMException && err.name === 'AbortError') return;
259+
throw err;
260+
});
241261
}
242262
});
243263

@@ -557,7 +577,21 @@ export function RenderSchema({ node, stores, registry, context = {}, children }:
557577
builder.subscribe(handleResults).then(handleResults);
558578
onCleanup(() => builder.dispose());
559579
} else {
560-
(ModelClass.findAll(p, queryOptions) as Promise<unknown[]>).then(handleResults);
580+
// Same abort discipline as the list-path createQuerySignal: the
581+
// effect re-runs on perspective swap / param change, and any
582+
// in-flight findAll from the previous run should be cancelled so
583+
// we don't pay its serialise + transit + deserialise tax.
584+
const controller = new AbortController();
585+
onCleanup(() => controller.abort());
586+
(ModelClass.findAll(p, queryOptions, { signal: controller.signal }) as Promise<unknown[]>)
587+
.then((results) => {
588+
if (controller.signal.aborted) return;
589+
handleResults(results);
590+
})
591+
.catch((err) => {
592+
if (err instanceof DOMException && err.name === 'AbortError') return;
593+
throw err;
594+
});
561595
}
562596
});
563597
}

packages/schema-system/frameworks/solid/tests/queryToken.test.tsx

Lines changed: 92 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -228,6 +228,98 @@ describe('$query token', () => {
228228
expect(JSON.parse(el?.textContent ?? '[]')).toEqual([{ id: 1 }]);
229229
});
230230

231+
// ---- AbortSignal threading + cleanup ----
232+
233+
it('passes an AbortSignal to findAll and aborts it on unmount', async () => {
234+
let capturedSignal: AbortSignal | undefined;
235+
const MockModel = {
236+
query: vi.fn(),
237+
findAll: vi.fn((_p: unknown, _q: unknown, opts: { signal?: AbortSignal } = {}) => {
238+
capturedSignal = opts.signal;
239+
return Promise.resolve([]);
240+
}),
241+
};
242+
const stores = {
243+
spaceStore: { perspective: () => ({ uuid: 'p1' }) },
244+
$getModel: () => MockModel,
245+
};
246+
247+
const node: SchemaNode = {
248+
type: 'DataDisplay',
249+
props: { data: { $query: { model: 'Post', subscribe: false } } },
250+
};
251+
252+
const { unmount } = render(() => <RenderSchema node={node} stores={stores} registry={registry} />);
253+
await tick();
254+
255+
expect(MockModel.findAll).toHaveBeenCalledOnce();
256+
expect(capturedSignal).toBeInstanceOf(AbortSignal);
257+
expect(capturedSignal!.aborted).toBe(false);
258+
259+
// Unmount should trigger onCleanup, which aborts the controller.
260+
unmount();
261+
expect(capturedSignal!.aborted).toBe(true);
262+
});
263+
264+
it('aborts a stale findAll when the effect re-runs', async () => {
265+
const signals: AbortSignal[] = [];
266+
const MockModel = {
267+
query: vi.fn(),
268+
findAll: vi.fn((_p: unknown, _q: unknown, opts: { signal?: AbortSignal } = {}) => {
269+
if (opts.signal) signals.push(opts.signal);
270+
return new Promise<unknown[]>(() => {
271+
// Never resolve — simulate slow query
272+
});
273+
}),
274+
};
275+
const [perspective, setPerspective] = createSignal<{ uuid: string } | null>({ uuid: 'p1' });
276+
const stores = {
277+
spaceStore: { perspective },
278+
$getModel: () => MockModel,
279+
};
280+
281+
const node: SchemaNode = {
282+
type: 'DataDisplay',
283+
props: { data: { $query: { model: 'Post', subscribe: false } } },
284+
};
285+
286+
render(() => <RenderSchema node={node} stores={stores} registry={registry} />);
287+
await tick();
288+
expect(signals.length).toBe(1);
289+
expect(signals[0].aborted).toBe(false);
290+
291+
// Trigger effect re-run by changing the reactive perspective dep.
292+
setPerspective({ uuid: 'p2' });
293+
await tick();
294+
295+
// Old run's controller aborted; new run got its own fresh signal.
296+
expect(signals[0].aborted).toBe(true);
297+
expect(signals.length).toBe(2);
298+
expect(signals[1].aborted).toBe(false);
299+
});
300+
301+
it('swallows AbortError from findAll without surfacing it', async () => {
302+
const MockModel = {
303+
query: vi.fn(),
304+
findAll: vi.fn(() => Promise.reject(new DOMException('Aborted', 'AbortError'))),
305+
};
306+
const stores = {
307+
spaceStore: { perspective: () => ({ uuid: 'p1' }) },
308+
$getModel: () => MockModel,
309+
};
310+
311+
const node: SchemaNode = {
312+
type: 'DataDisplay',
313+
props: { data: { $query: { model: 'Post', subscribe: false } } },
314+
};
315+
316+
// If the catch arm doesn't swallow AbortError, the unhandled rejection
317+
// would surface as a test failure.
318+
render(() => <RenderSchema node={node} stores={stores} registry={registry} />);
319+
await tick();
320+
expect(MockModel.findAll).toHaveBeenCalledOnce();
321+
});
322+
231323
// ---- Query params forwarding ----
232324

233325
it('forwards where/order/limit params to query builder', async () => {

0 commit comments

Comments
 (0)