Skip to content

Commit 5d16e66

Browse files
authored
Revert "feat: allow async operations in useComputed$ hook" (#7594)
1 parent d7421f1 commit 5d16e66

File tree

10 files changed

+47
-115
lines changed

10 files changed

+47
-115
lines changed

.changeset/good-tables-rush.md

-5
This file was deleted.

packages/docs/src/routes/api/qwik/api.json

+1-15
Original file line numberDiff line numberDiff line change
@@ -324,20 +324,6 @@
324324
"editUrl": "https://github.com/QwikDev/qwik/tree/main/packages/qwik/src/core/use/use-computed.ts",
325325
"mdFile": "core.computedfn.md"
326326
},
327-
{
328-
"name": "ComputedReturnType",
329-
"id": "computedreturntype",
330-
"hierarchy": [
331-
{
332-
"name": "ComputedReturnType",
333-
"id": "computedreturntype"
334-
}
335-
],
336-
"kind": "TypeAlias",
337-
"content": "```typescript\nexport type ComputedReturnType<T> = T extends Promise<infer T> ? ReadonlySignal<T> : ReadonlySignal<T>;\n```\n**References:** [ReadonlySignal](#readonlysignal)",
338-
"editUrl": "https://github.com/QwikDev/qwik/tree/main/packages/qwik/src/core/use/use-computed.ts",
339-
"mdFile": "core.computedreturntype.md"
340-
},
341327
{
342328
"name": "ComputedSignal",
343329
"id": "computedsignal",
@@ -2144,7 +2130,7 @@
21442130
}
21452131
],
21462132
"kind": "Function",
2147-
"content": "Creates a computed signal which is calculated from the given function. A computed signal is a signal which is calculated from other signals. When the signals change, the computed signal is recalculated, and if the result changed, all tasks which are tracking the signal will be re-run and all components that read the signal will be re-rendered.\n\nThe function must be synchronous and must not have any side effects.\n\n\n```typescript\nuseComputed$: <T>(qrl: ComputedFn<T>) => ComputedReturnType<T>\n```\n\n\n<table><thead><tr><th>\n\nParameter\n\n\n</th><th>\n\nType\n\n\n</th><th>\n\nDescription\n\n\n</th></tr></thead>\n<tbody><tr><td>\n\nqrl\n\n\n</td><td>\n\n[ComputedFn](#computedfn)<!-- -->&lt;T&gt;\n\n\n</td><td>\n\n\n</td></tr>\n</tbody></table>\n**Returns:**\n\n[ComputedReturnType](#computedreturntype)<!-- -->&lt;T&gt;",
2133+
"content": "Creates a computed signal which is calculated from the given function. A computed signal is a signal which is calculated from other signals. When the signals change, the computed signal is recalculated, and if the result changed, all tasks which are tracking the signal will be re-run and all components that read the signal will be re-rendered.\n\nThe function must be synchronous and must not have any side effects.\n\n\n```typescript\nuseComputed$: <T>(qrl: ComputedFn<T>) => T extends Promise<any> ? never : ReadonlySignal<T>\n```\n\n\n<table><thead><tr><th>\n\nParameter\n\n\n</th><th>\n\nType\n\n\n</th><th>\n\nDescription\n\n\n</th></tr></thead>\n<tbody><tr><td>\n\nqrl\n\n\n</td><td>\n\n[ComputedFn](#computedfn)<!-- -->&lt;T&gt;\n\n\n</td><td>\n\n\n</td></tr>\n</tbody></table>\n**Returns:**\n\nT extends Promise&lt;any&gt; ? never : [ReadonlySignal](#readonlysignal)<!-- -->&lt;T&gt;",
21482134
"editUrl": "https://github.com/QwikDev/qwik/tree/main/packages/qwik/src/core/use/use-computed.ts",
21492135
"mdFile": "core.usecomputed_.md"
21502136
},

packages/docs/src/routes/api/qwik/index.mdx

+2-13
Original file line numberDiff line numberDiff line change
@@ -353,17 +353,6 @@ export type ComputedFn<T> = () => T;
353353

354354
[Edit this section](https://github.com/QwikDev/qwik/tree/main/packages/qwik/src/core/use/use-computed.ts)
355355

356-
## ComputedReturnType
357-
358-
```typescript
359-
export type ComputedReturnType<T> =
360-
T extends Promise<infer T> ? ReadonlySignal<T> : ReadonlySignal<T>;
361-
```
362-
363-
**References:** [ReadonlySignal](#readonlysignal)
364-
365-
[Edit this section](https://github.com/QwikDev/qwik/tree/main/packages/qwik/src/core/use/use-computed.ts)
366-
367356
## ComputedSignal
368357

369358
A computed signal is a signal which is calculated from other signals. When the signals change, the computed signal is recalculated, and if the result changed, all tasks which are tracking the signal will be re-run and all components that read the signal will be re-rendered.
@@ -8451,7 +8440,7 @@ Creates a computed signal which is calculated from the given function. A compute
84518440
The function must be synchronous and must not have any side effects.
84528441
84538442
```typescript
8454-
useComputed$: <T>(qrl: ComputedFn<T>) => ComputedReturnType<T>;
8443+
useComputed$: <T>(qrl: ComputedFn<T>) => T extends Promise<any> ? never : ReadonlySignal<T>
84558444
```
84568445
84578446
<table><thead><tr><th>
@@ -8481,7 +8470,7 @@ qrl
84818470
</tbody></table>
84828471
**Returns:**
84838472
8484-
[ComputedReturnType](#computedreturntype)&lt;T&gt;
8473+
T extends Promise&lt;any&gt; ? never : [ReadonlySignal](#readonlysignal)&lt;T&gt;
84858474
84868475
[Edit this section](https://github.com/QwikDev/qwik/tree/main/packages/qwik/src/core/use/use-computed.ts)
84878476

packages/qwik/src/core/index.ts

+1-1
Original file line numberDiff line numberDiff line change
@@ -111,7 +111,7 @@ export type { UseStylesScoped } from './use/use-styles';
111111
export type { UseSignal } from './use/use-signal';
112112
export type { ContextId } from './use/use-context';
113113
export type { UseStoreOptions } from './use/use-store.public';
114-
export type { ComputedFn, ComputedReturnType } from './use/use-computed';
114+
export type { ComputedFn } from './use/use-computed';
115115
export { useComputedQrl } from './use/use-computed';
116116
export { useSerializerQrl, useSerializer$ } from './use/use-serializer';
117117
export type { OnVisibleTaskOptions, VisibleTaskStrategy } from './use/use-visible-task';

packages/qwik/src/core/qwik.core.api.md

+2-5
Original file line numberDiff line numberDiff line change
@@ -74,9 +74,6 @@ export const componentQrl: <PROPS extends Record<any, any>>(componentQrl: QRL<On
7474
// @public (undocumented)
7575
export type ComputedFn<T> = () => T;
7676

77-
// @public (undocumented)
78-
export type ComputedReturnType<T> = T extends Promise<infer T> ? ReadonlySignal<T> : ReadonlySignal<T>;
79-
8077
// @public
8178
export interface ComputedSignal<T> extends ReadonlySignal<T> {
8279
force(): void;
@@ -1624,12 +1621,12 @@ export const untrack: <T>(fn: () => T) => T;
16241621
export const unwrapStore: <T>(value: T) => T;
16251622

16261623
// @public
1627-
export const useComputed$: <T>(qrl: ComputedFn<T>) => ComputedReturnType<T>;
1624+
export const useComputed$: <T>(qrl: ComputedFn<T>) => T extends Promise<any> ? never : ReadonlySignal<T>;
16281625

16291626
// Warning: (ae-internal-missing-underscore) The name "useComputedQrl" should be prefixed with an underscore because the declaration is marked as @internal
16301627
//
16311628
// @internal (undocumented)
1632-
export const useComputedQrl: <T>(qrl: QRL<ComputedFn<T>>) => ComputedReturnType<T>;
1629+
export const useComputedQrl: <T>(qrl: QRL<ComputedFn<T>>) => T extends Promise<any> ? never : ReadonlySignal<T>;
16331630

16341631
// @public
16351632
export const useConstant: <T>(value: (() => T) | T) => T;

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

+5-7
Original file line numberDiff line numberDiff line change
@@ -31,8 +31,6 @@ export class ComputedSignalImpl<T> extends SignalImpl<T> implements BackRef {
3131
$computeQrl$: ComputeQRL<T>;
3232
$flags$: SignalFlags;
3333
$forceRunEffects$: boolean = false;
34-
private $resolvedPromiseValue$: T | null = null;
35-
3634
[_EFFECT_BACK_REF]: Map<EffectProperty | string, EffectSubscription> | null = null;
3735

3836
constructor(
@@ -84,13 +82,13 @@ export class ComputedSignalImpl<T> extends SignalImpl<T> implements BackRef {
8482
const previousEffectSubscription = ctx?.$effectSubscriber$;
8583
ctx && (ctx.$effectSubscriber$ = getSubscriber(this, EffectProperty.VNODE));
8684
try {
87-
const untrackedValue = this.$resolvedPromiseValue$ || (computeQrl.getFn(ctx)() as T);
85+
const untrackedValue = computeQrl.getFn(ctx)() as T;
8886
if (isPromise(untrackedValue)) {
89-
throw untrackedValue.then((promiseValue) => {
90-
this.$resolvedPromiseValue$ = promiseValue;
91-
});
87+
throw qError(QError.computedNotSync, [
88+
computeQrl.dev ? computeQrl.dev.file : '',
89+
computeQrl.$hash$,
90+
]);
9291
}
93-
this.$resolvedPromiseValue$ = null;
9492
DEBUG && log('Signal.$compute$', untrackedValue);
9593

9694
this.$flags$ &= ~SignalFlags.INVALID;

packages/qwik/src/core/shared/error/error.ts

+2-2
Original file line numberDiff line numberDiff line change
@@ -51,7 +51,7 @@ export const codeToText = (code: number, ...parts: any[]): string => {
5151
'Unknown vnode type {{0}}.', // 43
5252
'Materialize error: missing element: {{0}} {{1}} {{2}}', // 44
5353
'Cannot coerce a Signal, use `.value` instead', // 45
54-
'', // 46 unused
54+
'useComputed$ QRL {{0}} {{1}} cannot return a Promise', // 46
5555
'ComputedSignal is read-only', // 47
5656
'WrappedSignal is read-only', // 48
5757
'Attribute value is unsafe for SSR', // 49
@@ -121,7 +121,7 @@ export const enum QError {
121121
invalidVNodeType = 43,
122122
materializeVNodeDataError = 44,
123123
cannotCoerceSignal = 45,
124-
UNUSED_46 = 46,
124+
computedNotSync = 46,
125125
computedReadOnly = 47,
126126
wrappedReadOnly = 48,
127127
unsafeAttr = 49,

packages/qwik/src/core/ssr/ssr-render-jsx.ts

+4-6
Original file line numberDiff line numberDiff line change
@@ -28,7 +28,7 @@ import {
2828
QSlotParent,
2929
qwikInspectorAttr,
3030
} from '../shared/utils/markers';
31-
import { isPromise, retryOnPromise } from '../shared/utils/promises';
31+
import { isPromise } from '../shared/utils/promises';
3232
import { qInspector } from '../shared/utils/qdev';
3333
import { addComponentStylePrefix, isClassAttr } from '../shared/utils/scoped-styles';
3434
import { serializeAttribute } from '../shared/utils/styles';
@@ -78,7 +78,7 @@ export async function _walkJSX(
7878
await (value as StackFn).apply(ssr);
7979
continue;
8080
}
81-
await processJSXNode(ssr, enqueue, value as JSXOutput, {
81+
processJSXNode(ssr, enqueue, value as JSXOutput, {
8282
styleScoped: options.currentStyleScoped,
8383
parentComponentFrame: options.parentComponentFrame,
8484
});
@@ -87,7 +87,7 @@ export async function _walkJSX(
8787
await drain();
8888
}
8989

90-
async function processJSXNode(
90+
function processJSXNode(
9191
ssr: SSRContainer,
9292
enqueue: (value: StackValue) => void,
9393
value: JSXOutput,
@@ -114,9 +114,7 @@ async function processJSXNode(
114114
ssr.openFragment(isDev ? [DEBUG_TYPE, VirtualType.WrappedSignal] : EMPTY_ARRAY);
115115
const signalNode = ssr.getLastNode();
116116
enqueue(ssr.closeFragment);
117-
await retryOnPromise(() => {
118-
enqueue(trackSignalAndAssignHost(value, signalNode, EffectProperty.VNODE, ssr));
119-
});
117+
enqueue(trackSignalAndAssignHost(value, signalNode, EffectProperty.VNODE, ssr));
120118
} else if (isPromise(value)) {
121119
ssr.openFragment(isDev ? [DEBUG_TYPE, VirtualType.Awaited] : EMPTY_ARRAY);
122120
enqueue(ssr.closeFragment);

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

+26-56
Original file line numberDiff line numberDiff line change
@@ -15,7 +15,10 @@ import {
1515
useTask$,
1616
} from '@qwik.dev/core';
1717
import { domRender, ssrRenderToDom, trigger } from '@qwik.dev/core/testing';
18-
import { describe, expect, it } from 'vitest';
18+
import { describe, expect, it, vi } from 'vitest';
19+
import { ErrorProvider } from '../../testing/rendering.unit-util';
20+
import * as qError from '../shared/error/error';
21+
import { QError } from '../shared/error/error';
1922

2023
const debug = false; //true;
2124
Error.stackTraceLimit = 100;
@@ -218,64 +221,31 @@ describe.each([
218221
);
219222
});
220223

221-
describe('async computed', () => {
222-
it('should resolve promise in computed result', async () => {
223-
const Counter = component$(() => {
224-
const count = useSignal(1);
225-
const doubleCount = useComputed$(() => Promise.resolve(count.value * 2));
226-
return <button onClick$={() => count.value++}>{doubleCount.value}</button>;
227-
});
228-
const { vNode, container } = await render(<Counter />, { debug });
229-
expect(vNode).toMatchVDOM(
230-
<>
231-
<button>
232-
<Signal ssr-required>{'2'}</Signal>
233-
</button>
234-
</>
235-
);
236-
await trigger(container.element, 'button', 'click');
237-
expect(vNode).toMatchVDOM(
238-
<>
239-
<button>
240-
<Signal ssr-required>{'4'}</Signal>
241-
</button>
242-
</>
224+
it('should disallow Promise in computed result', async () => {
225+
const qErrorSpy = vi.spyOn(qError, 'qError');
226+
const Counter = component$(() => {
227+
const count = useSignal(1);
228+
const doubleCount = useComputed$(() => Promise.resolve(count.value * 2));
229+
return (
230+
<button onClick$={() => count.value++}>
231+
{
232+
// @ts-expect-error
233+
doubleCount.value
234+
}
235+
</button>
243236
);
244237
});
245-
246-
it('should resolve delayed promise in computed result', async () => {
247-
const Counter = component$(() => {
248-
const count = useSignal(1);
249-
const doubleCount = useComputed$(
250-
() =>
251-
new Promise<number>((resolve) => {
252-
// TODO: hack: for some reason inside set timeout invoke context is undefined
253-
const value = count.value * 2;
254-
setTimeout(() => {
255-
resolve(value);
256-
});
257-
})
258-
);
259-
return <button onClick$={() => count.value++}>{doubleCount.value}</button>;
260-
});
261-
const { vNode, container } = await render(<Counter />, { debug });
262-
263-
expect(vNode).toMatchVDOM(
264-
<>
265-
<button>
266-
<Signal ssr-required>{'2'}</Signal>
267-
</button>
268-
</>
238+
try {
239+
await render(
240+
<ErrorProvider>
241+
<Counter />
242+
</ErrorProvider>,
243+
{ debug }
269244
);
270-
await trigger(container.element, 'button', 'click');
271-
expect(vNode).toMatchVDOM(
272-
<>
273-
<button>
274-
<Signal ssr-required>{'4'}</Signal>
275-
</button>
276-
</>
277-
);
278-
});
245+
} catch (e) {
246+
expect((e as Error).message).toBeDefined();
247+
expect(qErrorSpy).toHaveBeenCalledWith(QError.computedNotSync, expect.any(Array));
248+
}
279249
});
280250

281251
describe('createComputed$', () => {

packages/qwik/src/core/use/use-computed.ts

+4-5
Original file line numberDiff line numberDiff line change
@@ -8,14 +8,11 @@ import { useSequentialScope } from './use-sequential-scope';
88

99
/** @public */
1010
export type ComputedFn<T> = () => T;
11-
/** @public */
12-
export type ComputedReturnType<T> =
13-
T extends Promise<infer T> ? ReadonlySignal<T> : ReadonlySignal<T>;
1411

1512
export const useComputedCommon = <T>(
1613
qrl: QRL<ComputedFn<T>>,
1714
Class: typeof ComputedSignalImpl
18-
): ComputedReturnType<T> => {
15+
): T extends Promise<any> ? never : ReadonlySignal<T> => {
1916
const { val, set } = useSequentialScope<Signal<T>>();
2017
if (val) {
2118
return val as any;
@@ -32,7 +29,9 @@ export const useComputedCommon = <T>(
3229
};
3330

3431
/** @internal */
35-
export const useComputedQrl = <T>(qrl: QRL<ComputedFn<T>>): ComputedReturnType<T> => {
32+
export const useComputedQrl = <T>(
33+
qrl: QRL<ComputedFn<T>>
34+
): T extends Promise<any> ? never : ReadonlySignal<T> => {
3635
return useComputedCommon(qrl, ComputedSignalImpl);
3736
};
3837

0 commit comments

Comments
 (0)