Skip to content

Commit 5eb95fb

Browse files
wmertenswmertens
authored
Merge pull request #7700 from QwikDev/v2-subscribe-recomputation
fix: subscribe to signals on computed signal recomputation
2 parents af205e2 + 24d600c commit 5eb95fb

File tree

5 files changed

+109
-26
lines changed

5 files changed

+109
-26
lines changed

‎.changeset/old-guests-stare.md

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
---
2+
'@qwik.dev/core': patch
3+
---
4+
5+
fix: subscribe to signals on computed signal recomputation

‎packages/qwik/src/core/reactive-primitives/impl/signal.unit.tsx

Lines changed: 33 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
import { $, isBrowser, type ValueOrPromise } from '@qwik.dev/core';
1+
import { $, isBrowser } from '@qwik.dev/core';
22
import { createDocument, getTestPlatform } from '@qwik.dev/core/testing';
33
import { afterEach, beforeEach, describe, expect, expectTypeOf, it } from 'vitest';
44
import { getDomContainer } from '../../client/dom-container';
@@ -8,7 +8,7 @@ import { type QRLInternal } from '../../shared/qrl/qrl-class';
88
import { type QRL } from '../../shared/qrl/qrl.public';
99
import { ChoreType } from '../../shared/util-chore-type';
1010
import type { Container, HostElement } from '../../shared/types';
11-
import { isPromise } from '../../shared/utils/promises';
11+
import { retryOnPromise } from '../../shared/utils/promises';
1212
import { invoke, newInvokeContext } from '../../use/use-core';
1313
import { Task } from '../../use/use-task';
1414
import {
@@ -157,7 +157,7 @@ describe('signal', () => {
157157
await withContainer(async () => {
158158
const a = createSignal(2) as InternalSignal<number>;
159159
const b = createSignal(10) as InternalSignal<number>;
160-
await retry(() => {
160+
await retryOnPromise(() => {
161161
let signal!: InternalReadonlySignal<number>;
162162
effect$(() => {
163163
signal =
@@ -184,10 +184,39 @@ describe('signal', () => {
184184
expect(log).toEqual([12, 23]);
185185
});
186186
});
187+
188+
it('should track when recomputing computed signal', async () => {
189+
await withContainer(async () => {
190+
const a = createSignal(true) as InternalSignal<boolean>;
191+
const b = createSignal(true) as InternalSignal<boolean>;
192+
await retryOnPromise(async () => {
193+
let signal!: InternalReadonlySignal<boolean>;
194+
effect$(() => {
195+
signal =
196+
signal ||
197+
createComputedQrl(
198+
delayQrl(
199+
$(() => {
200+
return a.value || b.value;
201+
})
202+
)
203+
);
204+
log.push(signal.value); // causes subscription
205+
});
206+
expect(log).toEqual([true]);
207+
a.value = !a.untrackedValue;
208+
await flushSignals();
209+
b.value = !b.untrackedValue;
210+
});
211+
await flushSignals();
212+
expect(log).toEqual([true, false]);
213+
});
214+
});
215+
187216
it('force', () =>
188217
withContainer(async () => {
189218
const obj = { count: 0 };
190-
const computed = await retry(() => {
219+
const computed = await retryOnPromise(() => {
191220
return createComputedQrl(
192221
delayQrl(
193222
$(() => {
@@ -259,15 +288,4 @@ describe('signal', () => {
259288
}
260289

261290
const effect$ = /*#__PURE__*/ implicit$FirstArg(effectQrl);
262-
263-
function retry<T>(fn: () => T): ValueOrPromise<T> {
264-
try {
265-
return fn();
266-
} catch (e) {
267-
if (isPromise(e)) {
268-
return e.then(retry.bind(null, fn)) as ValueOrPromise<T>;
269-
}
270-
throw e;
271-
}
272-
}
273291
});

‎packages/qwik/src/core/reactive-primitives/impl/wrapped-signal-impl.ts

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -44,8 +44,6 @@ export class WrappedSignalImpl<T> extends SignalImpl<T> implements BackRef {
4444
$invalidate$() {
4545
this.$flags$ |= SignalFlags.INVALID;
4646
this.$forceRunEffects$ = false;
47-
// We should only call subscribers if the calculation actually changed.
48-
// Therefore, we need to calculate the value now.
4947
this.$container$?.$scheduler$(
5048
ChoreType.RECOMPUTE_AND_SCHEDULE_EFFECTS,
5149
this.$hostElement$,

‎packages/qwik/src/core/shared/scheduler.ts

Lines changed: 14 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -120,7 +120,7 @@ import { ssrNodeDocumentPosition, vnode_documentPosition } from './scheduler-doc
120120
import type { Container, HostElement } from './types';
121121
import { logWarn } from './utils/log';
122122
import { QScopedStyle } from './utils/markers';
123-
import { isPromise, retryOnPromise, safeCall } from './utils/promises';
123+
import { isPromise, maybeThen, retryOnPromise, safeCall } from './utils/promises';
124124
import { addComponentStylePrefix } from './utils/scoped-styles';
125125
import { serializeAttribute } from './utils/styles';
126126
import type { ValueOrPromise } from './utils/types';
@@ -130,6 +130,7 @@ import { WrappedSignalImpl } from '../reactive-primitives/impl/wrapped-signal-im
130130
import type { StoreHandler } from '../reactive-primitives/impl/store';
131131
import { SignalImpl } from '../reactive-primitives/impl/signal-impl';
132132
import { isQrl } from './qrl/qrl-utils';
133+
import { invoke, newInvokeContext } from '../use/use-core';
133134

134135
// Turn this on to get debug output of what the scheduler is doing.
135136
const DEBUG: boolean = false;
@@ -486,21 +487,25 @@ export const createScheduler = (
486487

487488
const effects = chore.$payload$ as Set<EffectSubscription>;
488489

490+
const ctx = newInvokeContext();
491+
ctx.$container$ = container;
489492
if (target instanceof ComputedSignalImpl || target instanceof WrappedSignalImpl) {
490493
const forceRunEffects = target.$forceRunEffects$;
491494
target.$forceRunEffects$ = false;
492-
if (!target.$effects$?.size) {
495+
if (!effects?.size) {
493496
break;
494497
}
495-
returnValue = retryOnPromise(() => {
496-
if (target.$computeIfNeeded$() || forceRunEffects) {
497-
triggerEffects(container, target, effects);
498+
// needed for computed signals and throwing QRLs
499+
returnValue = maybeThen(
500+
retryOnPromise(() => invoke.call(target, ctx, target.$computeIfNeeded$)),
501+
(didChange) => {
502+
if (didChange || forceRunEffects) {
503+
return retryOnPromise(() => triggerEffects(container, target, effects));
504+
}
498505
}
499-
});
506+
);
500507
} else {
501-
returnValue = retryOnPromise(() => {
502-
triggerEffects(container, target, effects);
503-
});
508+
returnValue = retryOnPromise(() => triggerEffects(container, target, effects));
504509
}
505510
}
506511
break;

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

Lines changed: 57 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
import {
22
Fragment,
33
Fragment as Signal,
4+
Fragment as Component,
45
component$,
56
createComputed$,
67
createSignal,
@@ -248,6 +249,62 @@ describe.each([
248249
}
249250
});
250251

252+
it('should track when recomputing computed signal', async () => {
253+
const Cmp = component$(() => {
254+
const boolean1 = useSignal(true);
255+
const boolean2 = useSignal(true);
256+
const computed = useComputed$(() => {
257+
return boolean1.value || boolean2.value;
258+
});
259+
260+
return (
261+
<div>
262+
{`${boolean1.value}`}
263+
{`${boolean2.value}`}
264+
{`${computed.value}`}
265+
<button id="toggle-boolean1" onClick$={() => (boolean1.value = !boolean1.value)}></button>
266+
<button id="toggle-boolean2" onClick$={() => (boolean2.value = !boolean2.value)}></button>
267+
</div>
268+
);
269+
});
270+
const { vNode, container } = await render(<Cmp />, { debug });
271+
expect(vNode).toMatchVDOM(
272+
<Component>
273+
<div>
274+
<Signal ssr-required>{'true'}</Signal>
275+
<Signal ssr-required>{'true'}</Signal>
276+
<Signal ssr-required>{'true'}</Signal>
277+
<button id="toggle-boolean1"></button>
278+
<button id="toggle-boolean2"></button>
279+
</div>
280+
</Component>
281+
);
282+
await trigger(container.element, 'button[id="toggle-boolean1"]', 'click');
283+
expect(vNode).toMatchVDOM(
284+
<Component>
285+
<div>
286+
<Signal ssr-required>{'false'}</Signal>
287+
<Signal ssr-required>{'true'}</Signal>
288+
<Signal ssr-required>{'true'}</Signal>
289+
<button id="toggle-boolean1"></button>
290+
<button id="toggle-boolean2"></button>
291+
</div>
292+
</Component>
293+
);
294+
await trigger(container.element, 'button[id="toggle-boolean2"]', 'click');
295+
expect(vNode).toMatchVDOM(
296+
<Component>
297+
<div>
298+
<Signal ssr-required>{'false'}</Signal>
299+
<Signal ssr-required>{'false'}</Signal>
300+
<Signal ssr-required>{'false'}</Signal>
301+
<button id="toggle-boolean1"></button>
302+
<button id="toggle-boolean2"></button>
303+
</div>
304+
</Component>
305+
);
306+
});
307+
251308
describe('createComputed$', () => {
252309
it('can be created anywhere', async () => {
253310
const count = createSignal(1);

0 commit comments

Comments
 (0)