Skip to content

fix: early schedule computed qrls to resolve #7435

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
wants to merge 1 commit into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions .changeset/calm-hounds-brake.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
'@qwik.dev/core': patch
---

fix: early schedule computed qrls to resolve
31 changes: 30 additions & 1 deletion packages/qwik/src/core/client/dom-container.ts
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,12 @@ import { emitEvent } from '../shared/qrl/qrl-class';
import type { QRL } from '../shared/qrl/qrl.public';
import { ChoreType } from '../shared/util-chore-type';
import { _SharedContainer } from '../shared/shared-container';
import { inflateQRL, parseQRL, wrapDeserializerProxy } from '../shared/shared-serialization';
import {
TypeIds,
inflateQRL,
parseQRL,
wrapDeserializerProxy,
} from '../shared/shared-serialization';
import { QContainerValue, type HostElement, type ObjToProxyMap } from '../shared/types';
import { EMPTY_ARRAY } from '../shared/utils/flyweight';
import {
Expand Down Expand Up @@ -155,11 +160,16 @@ export class DomContainer extends _SharedContainer implements IClientContainer {
element.setAttribute(QContainerAttr, QContainerValue.RESUMED);
element.qContainer = this;

this.$qFuncs$ = getQFuncs(document, this.$instanceHash$) || EMPTY_ARRAY;
this.$setServerData$();
element.setAttribute(QContainerAttr, QContainerValue.RESUMED);
element.qContainer = this;
const qwikStates = element.querySelectorAll('script[type="qwik/state"]');
if (qwikStates.length !== 0) {
const lastState = qwikStates[qwikStates.length - 1];
this.$rawStateData$ = JSON.parse(lastState.textContent!);
this.$stateData$ = wrapDeserializerProxy(this, this.$rawStateData$) as unknown[];
this.$scheduleQRLs$();
}
}

Expand Down Expand Up @@ -371,4 +381,23 @@ export class DomContainer extends _SharedContainer implements IClientContainer {
}
this.$serverData$ = { containerAttributes };
}

/**
* Schedule all computed signals to be inflated. This is done after at the time of DomContainer
* creation to ensure that all computed signals are inflated and QRLs are resolved before any
* signals are used. This is necessary because if a computed QRL is not resolved, it will throw a
* promise and we will have to rerun the entire function, which we want to avoid.
*/
private $scheduleQRLs$(): void {
const deserializeValue = <T>(i: number) => {
return this.$stateData$[i / 2] as T;
};
for (let i = 0; i < this.$rawStateData$.length; i += 2) {
const type = this.$rawStateData$[i];
if (type === TypeIds.ComputedSignal) {
// use deserializer proxy to inflate the computed signal and schedule computed QRL
deserializeValue(i);
}
}
}
}
3 changes: 2 additions & 1 deletion packages/qwik/src/core/shared/utils/markers.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
import { QContainerValue } from '../types';
import { EMPTY_ARRAY } from './flyweight';

/** State factory of the component. */
export const OnRenderProp = 'q:renderFn';
Expand All @@ -23,7 +24,7 @@ export const getQFuncs = (
document: Document,
hash: string
): Array<(...args: unknown[]) => unknown> => {
return (document as any)[QFuncsPrefix + hash] || [];
return (document as any)[QFuncsPrefix + hash] || EMPTY_ARRAY;
};

export const QRenderAttr = 'q:render';
Expand Down
42 changes: 40 additions & 2 deletions starters/apps/e2e/src/components/qrl/qrl.tsx
Original file line number Diff line number Diff line change
@@ -1,13 +1,51 @@
import { $, component$, useComputed$, useSignal } from "@qwik.dev/core";
import {
$,
component$,
useComputed$,
useSignal,
useVisibleTask$,
} from "@qwik.dev/core";

export const QRL = component$(() => {
const render = useSignal(0);
return (
<>
<ShouldResolveInnerComputedQRL />
<button id="rerender" onClick$={() => render.value++}>
rerender {render.value}
</button>
<div key={render.value}>
<ShouldResolveComputedQRL />
<ShouldResolveInnerComputedQRL />
</div>
</>
);
});

export const ShouldResolveComputedQRL = component$(() => {
const computedCounter = useSignal(0);
const text = useComputed$(() => {
computedCounter.value++;
return "";
});

return (
<>
<span id="computed-counter">{computedCounter.value}</span>
<Test text={text} />
</>
);
});

export const Test = component$<any>(({ text }) => {
const visibleCounter = useSignal(0);
useVisibleTask$(async () => {
visibleCounter.value++;
text.value;
});

return <div id="visible-counter">{visibleCounter.value}</div>;
});

export const ShouldResolveInnerComputedQRL = component$(() => {
const test = useComputed$(() => 0);
return <InnerComputedButton test={test} />;
Expand Down
2 changes: 1 addition & 1 deletion starters/apps/e2e/src/root.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -33,9 +33,9 @@ import { TreeshakingApp } from "./components/treeshaking/treeshaking";
import { TwoListeners } from "./components/two-listeners/twolisteners";
import { UseId } from "./components/useid/useid";
import { Watch } from "./components/watch/watch";
import { QRL } from "./components/qrl/qrl";

import "./global.css";
import { QRL } from "./components/qrl/qrl";

const tests: Record<string, FunctionComponent> = {
"/e2e/two-listeners": () => <TwoListeners />,
Expand Down
47 changes: 36 additions & 11 deletions starters/e2e/e2e.qrl.e2e.ts
Original file line number Diff line number Diff line change
@@ -1,14 +1,39 @@
import { expect, test } from "@playwright/test";
test("should update counter without uncaught promises", async ({ page }) => {
await page.goto("/e2e/qrl");
page.on("pageerror", (err) => expect(err).toEqual(undefined));
page.on("console", (msg) => {
if (msg.type() === "error") {
expect(msg.text()).toEqual(undefined);
}
import { test, expect } from "@playwright/test";

test.describe("qrl", () => {
test.beforeEach(async ({ page }) => {
await page.goto("/e2e/qrl");
page.on("pageerror", (err) => expect(err).toEqual(undefined));
page.on("console", (msg) => {
if (msg.type() === "error") {
expect(msg.text()).toEqual(undefined);
}
});
});
const button = page.locator("#inner-computed-button");
await expect(button).toContainText("Click Me 0");

await button.click();
function tests() {
test("should resolve computed QRL", async ({ page }) => {
const computedCounter = page.locator("#computed-counter");
const visibleCounter = page.locator("#visible-counter");
await expect(computedCounter).toHaveText("1");
await expect(visibleCounter).toHaveText("1");
});

test("should resolve inner computed QRL", async ({ page }) => {
const innerButton = page.locator("#inner-computed-button");
await innerButton.click();
await expect(innerButton).toHaveText("Click Me 1");
});
}

tests();

test.describe("client rerender", () => {
test.beforeEach(async ({ page }) => {
const toggleRender = page.locator("#rerender");
await toggleRender.click();
await expect(page.locator("#rerender")).toHaveText("rerender 1");
});
tests();
});
});