Skip to content

Commit 9a00f77

Browse files
committed
feat: run cleanup function for async computed before computation
1 parent 5084b42 commit 9a00f77

File tree

4 files changed

+52
-20
lines changed

4 files changed

+52
-20
lines changed

packages/qwik/src/core/client/vnode-diff.ts

Lines changed: 22 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,11 @@ import { clearAllEffects, clearEffectSubscription } from '../reactive-primitives
44
import { WrappedSignalImpl } from '../reactive-primitives/impl/wrapped-signal-impl';
55
import type { Signal } from '../reactive-primitives/signal.public';
66
import { SubscriptionData } from '../reactive-primitives/subscription-data';
7-
import { EffectProperty, EffectSubscriptionProp } from '../reactive-primitives/types';
7+
import {
8+
EffectProperty,
9+
EffectSubscriptionProp,
10+
type Consumer,
11+
} from '../reactive-primitives/types';
812
import { isSignal } from '../reactive-primitives/utils';
913
import { executeComponent } from '../shared/component-execution';
1014
import { SERIALIZABLE_STATE, type OnRenderFn } from '../shared/component.public';
@@ -48,7 +52,7 @@ import { isPromise, retryOnPromise } from '../shared/utils/promises';
4852
import { isSlotProp } from '../shared/utils/prop';
4953
import { hasClassAttr } from '../shared/utils/scoped-styles';
5054
import { serializeAttribute } from '../shared/utils/styles';
51-
import { isArray, type ValueOrPromise } from '../shared/utils/types';
55+
import { isArray, isObject, type ValueOrPromise } from '../shared/utils/types';
5256
import { trackSignalAndAssignHost } from '../use/use-core';
5357
import { TaskFlags, isTask } from '../use/use-task';
5458
import type { DomContainer } from './dom-container';
@@ -81,9 +85,10 @@ import {
8185
} from './vnode';
8286
import type { ElementVNode, TextVNode, VNode, VirtualVNode } from './vnode-impl';
8387
import { getAttributeNamespace, getNewElementNamespaceData } from './vnode-namespace';
84-
import { cleanupDestroyable, isDestroyable } from '../use/utils/destroyable';
88+
import { cleanupDestroyable } from '../use/utils/destroyable';
8589
import { SignalImpl } from '../reactive-primitives/impl/signal-impl';
8690
import { isStore } from '../reactive-primitives/impl/store';
91+
import { AsyncComputedSignalImpl } from '../reactive-primitives/impl/async-computed-signal-impl';
8792

8893
export const vnode_diff = (
8994
container: ClientContainer,
@@ -1540,19 +1545,22 @@ export function cleanup(container: ClientContainer, vNode: VNode) {
15401545
if (seq) {
15411546
for (let i = 0; i < seq.length; i++) {
15421547
const obj = seq[i];
1543-
if (isTask(obj)) {
1544-
clearAllEffects(container, obj);
1545-
if (obj.$flags$ & TaskFlags.VISIBLE_TASK) {
1546-
container.$scheduler$(ChoreType.CLEANUP_VISIBLE, obj);
1547-
// don't call cleanupDestroyable yet, do it by the scheduler
1548-
continue;
1548+
if (isObject(obj)) {
1549+
const objIsTask = isTask(obj);
1550+
if (objIsTask) {
1551+
clearAllEffects(container, obj);
1552+
if (obj.$flags$ & TaskFlags.VISIBLE_TASK) {
1553+
container.$scheduler$(ChoreType.CLEANUP_VISIBLE, obj);
1554+
// don't call cleanupDestroyable yet, do it by the scheduler
1555+
continue;
1556+
}
1557+
} else if (obj instanceof SignalImpl || isStore(obj)) {
1558+
clearAllEffects(container, obj as Consumer);
15491559
}
1550-
} else if (obj instanceof SignalImpl || isStore(obj)) {
1551-
clearAllEffects(container, obj);
1552-
}
15531560

1554-
if (isDestroyable(obj)) {
1555-
cleanupDestroyable(obj);
1561+
if (objIsTask || obj instanceof AsyncComputedSignalImpl) {
1562+
cleanupDestroyable(obj);
1563+
}
15561564
}
15571565
}
15581566
}

packages/qwik/src/core/reactive-primitives/impl/async-computed-signal-impl.ts

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@ import type { NoSerialize } from '../../shared/serdes/verify';
33
import type { Container } from '../../shared/types';
44
import { ChoreType } from '../../shared/util-chore-type';
55
import { isPromise, retryOnPromise } from '../../shared/utils/promises';
6+
import { cleanupDestroyable } from '../../use/utils/destroyable';
67
import { cleanupFn, trackFn } from '../../use/utils/tracker';
78
import type { BackRef } from '../cleanup';
89
import {
@@ -135,6 +136,11 @@ export class AsyncComputedSignalImpl<T>
135136
this.untrackedLoading = true;
136137
this.untrackedError = null;
137138

139+
if (this.$promiseValue$ !== NEEDS_COMPUTATION) {
140+
// skip cleanup after resuming
141+
cleanupDestroyable(this);
142+
}
143+
138144
const promise = untrackedValue
139145
.then((promiseValue) => {
140146
DEBUG && log('Promise resolved', promiseValue);

packages/qwik/src/core/tests/use-async-computed.spec.tsx

Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -347,5 +347,29 @@ describe.each([
347347
// on server and client cleanup called again
348348
expect((globalThis as any).log).toEqual(['cleanup', 'cleanup']);
349349
});
350+
351+
it('should run cleanup on re-compute', async () => {
352+
(globalThis as any).log = [];
353+
354+
const Counter = component$(() => {
355+
const count = useSignal(1);
356+
const asyncValue = useAsyncComputed$(({ track, cleanup }) => {
357+
const current = track(count);
358+
cleanup(() => {
359+
(globalThis as any).log.push('cleanup');
360+
});
361+
return Promise.resolve(current * 2);
362+
});
363+
return <button onClick$={() => count.value++}>{asyncValue.value}</button>;
364+
});
365+
const { container } = await render(<Counter />, { debug });
366+
expect((globalThis as any).log).toEqual(render === ssrRenderToDom ? ['cleanup'] : []);
367+
368+
await trigger(container.element, 'button', 'click');
369+
expect((globalThis as any).log).toEqual(['cleanup']);
370+
371+
await trigger(container.element, 'button', 'click');
372+
expect((globalThis as any).log).toEqual(['cleanup', 'cleanup']);
373+
});
350374
});
351375
});

packages/qwik/src/core/use/utils/destroyable.ts

Lines changed: 0 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,5 @@
1-
import { AsyncComputedSignalImpl } from '../../reactive-primitives/impl/async-computed-signal-impl';
21
import type { NoSerialize } from '../../shared/serdes/verify';
32
import { logError } from '../../shared/utils/log';
4-
import { isTask } from '../use-task';
53

64
export type Destroyable = { $destroy$: NoSerialize<() => void> | null };
75

@@ -16,7 +14,3 @@ export const cleanupDestroyable = (destroyable: Destroyable) => {
1614
}
1715
}
1816
};
19-
20-
export const isDestroyable = (obj: unknown): obj is Destroyable => {
21-
return isTask(obj) || obj instanceof AsyncComputedSignalImpl;
22-
};

0 commit comments

Comments
 (0)