Skip to content

Commit

Permalink
Merge pull request #6983 from QwikDev/v2-fix-computed-throw-promise
Browse files Browse the repository at this point in the history
fix(signals): should early resolve computed qrl
  • Loading branch information
wmertens authored Oct 16, 2024
2 parents 3b5d6d9 + 4a77491 commit d271212
Show file tree
Hide file tree
Showing 3 changed files with 51 additions and 0 deletions.
10 changes: 10 additions & 0 deletions packages/qwik/src/core/shared/shared-serialization.ts
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,7 @@ import {
type SyncQRLInternal,
} from './qrl/qrl-class';
import type { QRL } from './qrl/qrl.public';
import { ChoreType } from './scheduler';
import type { DeserializeContainer, HostElement, ObjToProxyMap } from './types';
import { _CONST_PROPS, _VAR_PROPS } from './utils/constants';
import { isElement, isNode } from './utils/element';
Expand Down Expand Up @@ -282,6 +283,15 @@ const inflate = (container: DeserializeContainer, target: any, typeId: TypeIds,
computed.$untrackedValue$ = d[1];
computed.$invalid$ = d[2];
computed.$effects$ = d.slice(3);
/**
* If we try to compute value and the qrl is not resolved, then system throws an error with
* qrl promise. To prevent that we should early resolve computed qrl while computed
* deserialization. This also prevents anything from firing while computed qrls load, because
* of scheduler
*/
// try to download qrl in this tick
computed.$computeQrl$.resolve();
(container as DomContainer).$scheduler$?.(ChoreType.QRL_RESOLVE, null, computed.$computeQrl$);
break;
}
case TypeIds.Error: {
Expand Down
30 changes: 30 additions & 0 deletions starters/apps/e2e/src/components/computed/computed.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ export const ComputedRoot = component$(() => {
<Issue3482 />
<Issue3488 />
<Issue5738 />
<ShouldResolveComputedQrlEarly />
</div>
);
});
Expand Down Expand Up @@ -107,3 +108,32 @@ export const Issue5738 = component$(() => {
});
return <div id="issue-5738-result">Calc: {comp.value}</div>;
});

export const ShouldResolveComputedQrlEarly = component$(() => {
const isToggled = useSignal<boolean>(false);

const demo = useComputed$(() => 3);

// change attribute and read computed
const repro = useComputed$(() => {
if (!isToggled.value) {
return;
}

// happens when we read another computed value
return demo.value + 2;
});

return (
<>
<button
id="early-computed-qrl"
// also when tied to an attribute
data-test={repro.value}
onClick$={() => (isToggled.value = !isToggled.value)}
>
Click me! {repro.value}
</button>
</>
);
});
11 changes: 11 additions & 0 deletions starters/e2e/e2e.computed.e2e.ts
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,17 @@ test.describe("computed", () => {
]);
});

test("should early resolve computed qrl", async ({ page }) => {
const button = page.locator("#early-computed-qrl");
await expect(button).not.toHaveAttribute("data-test");
await expect(button).toContainText("Click me!");

await button.click();

await expect(button).toHaveAttribute("data-test", "5");
await expect(button).toContainText("Click me! 5");
});

test("issue 3482", async ({ page }) => {
const button = page.locator("#issue-3482-button");
const div = page.locator("#issue-3482-div");
Expand Down

0 comments on commit d271212

Please sign in to comment.