From dce6f1ec1eeb09958932eb28542e638770f542c2 Mon Sep 17 00:00:00 2001 From: sashkashishka Date: Sun, 16 Nov 2025 11:06:46 +0100 Subject: [PATCH 1/8] test: e2e add slowmo mode --- starters/e2e/e2e.attributes.spec.ts | 8 +++++--- starters/e2e/e2e.context.spec.ts | 11 ++++++++++- starters/e2e/e2e.style.spec.ts | 6 +++++- starters/e2e/qwikcity/nav.spec.ts | 2 -- starters/playwright.config.ts | 8 ++++++-- 5 files changed, 26 insertions(+), 9 deletions(-) diff --git a/starters/e2e/e2e.attributes.spec.ts b/starters/e2e/e2e.attributes.spec.ts index 51e7c64d19f..60fde7f3717 100644 --- a/starters/e2e/e2e.attributes.spec.ts +++ b/starters/e2e/e2e.attributes.spec.ts @@ -302,11 +302,13 @@ test.describe("attributes", () => { test.describe("client rerender", () => { test.beforeEach(async ({ page }) => { const toggleRender = page.locator("#force-rerender"); + const renderCount = page.locator("#renderCount"); const v = Number(await toggleRender.getAttribute("data-v")); + + expect(v).toBe(0); + await expect(renderCount).toHaveText(`Render ${v}`); await toggleRender.click(); - await expect(page.locator("#renderCount")).toHaveText(`Render ${v}`); - // Without this the tests still fail? - await page.waitForTimeout(50); + await expect(renderCount).toHaveText(`Render ${v + 1}`); }); tests(); }); diff --git a/starters/e2e/e2e.context.spec.ts b/starters/e2e/e2e.context.spec.ts index 09ca1b2f33e..6fdb801877e 100644 --- a/starters/e2e/e2e.context.spec.ts +++ b/starters/e2e/e2e.context.spec.ts @@ -64,6 +64,16 @@ test.describe("context", () => { const level3State3 = page.locator(".level3-state3"); const level3Slot = page.locator(".level3-slot"); + // const a = await level3State1.all() + // console.log(a) + // await page.waitForTimeout(50); + // const b = await level3State1.all() + // console.log(b) + // await expect(level3State1).toBeVisible(); + // await expect(level3State2).toBeVisible(); + // await expect(level3State3).toBeVisible(); + // await expect(level3Slot).toBeVisible(); + await expect(level3State1.first()).toHaveText("Level2 / state1 = 0"); expect(await level2State1.allTextContents()).toEqual([ "ROOT / state1 = 1", @@ -187,7 +197,6 @@ test.describe("context", () => { test.beforeEach(async ({ page }) => { const rerender = page.locator("#btn-rerender"); await rerender.click(); - await page.waitForTimeout(100); }); tests(); }); diff --git a/starters/e2e/e2e.style.spec.ts b/starters/e2e/e2e.style.spec.ts index 107827745ce..115c19d955a 100644 --- a/starters/e2e/e2e.style.spec.ts +++ b/starters/e2e/e2e.style.spec.ts @@ -17,9 +17,13 @@ test.describe("styles", () => { test.describe("client side", () => { test.beforeEach(async ({ page }) => { const reload = page.locator("#reload"); + const renderCount = page.locator("#renderCount"); const v = Number(await reload.getAttribute("v")); + + expect(v).toBe(0); + await expect(renderCount).toHaveText(`Render ${v}`); await reload.click(); - await expect(page.locator("#renderCount")).toHaveText(`Render ${v}`); + await expect(renderCount).toHaveText(`Render ${v + 1}`); }); runTests(); }); diff --git a/starters/e2e/qwikcity/nav.spec.ts b/starters/e2e/qwikcity/nav.spec.ts index ef469ebabec..83da0d5f7cd 100644 --- a/starters/e2e/qwikcity/nav.spec.ts +++ b/starters/e2e/qwikcity/nav.spec.ts @@ -59,8 +59,6 @@ test.describe("actions", () => { const link = page.locator("#hash-1"); await link.click(); - // Without this, sometimes the URL is #hash-1 - await page.waitForTimeout(100); await expect(page).toHaveURL( "/qwikcity-test/scroll-restoration/hash/#hash-2", diff --git a/starters/playwright.config.ts b/starters/playwright.config.ts index 5ff55370008..fa60102285b 100644 --- a/starters/playwright.config.ts +++ b/starters/playwright.config.ts @@ -19,6 +19,9 @@ expect.extend({ const config: PlaywrightTestConfig = { use: { + launchOptions: { + slowMo: 100, + }, viewport: { width: 520, height: 600, @@ -27,8 +30,9 @@ const config: PlaywrightTestConfig = { /* Fail the build on CI if you accidentally left test.only in the source code. */ forbidOnly: !!process.env.CI, testIgnore: /.*example.spec.tsx?$/, - retries: inGithubCI ? 0 : 1, - expect: { timeout: inGithubCI ? 120000 : 10000 }, + retries: 0, + // retries: inGithubCI ? 0 : 1, + expect: { timeout: inGithubCI ? 120000 : 3000 }, webServer: { command: "pnpm node --require ./scripts/runBefore.ts starters/dev-server.ts 3301", From 6b35fd14bba73a31b0c1f14deae71b4986bb921c Mon Sep 17 00:00:00 2001 From: sashkashishka Date: Sun, 16 Nov 2025 11:18:05 +0100 Subject: [PATCH 2/8] fix: while unmounting qwik app rendered in csr call cleanup for the whole tree --- packages/docs/scripts/showcase.ts | 1 + .../qwik/src/core/render/dom/render.unit.tsx | 71 +++++++++++++++++-- packages/qwik/src/core/render/dom/visitor.ts | 7 +- 3 files changed, 72 insertions(+), 7 deletions(-) diff --git a/packages/docs/scripts/showcase.ts b/packages/docs/scripts/showcase.ts index bce12e597f2..ae58799529b 100644 --- a/packages/docs/scripts/showcase.ts +++ b/packages/docs/scripts/showcase.ts @@ -1,6 +1,7 @@ import { readFileSync, writeFileSync, existsSync, mkdirSync } from 'fs'; import { chromium, type Page } from 'playwright'; import { fetch } from 'undici'; +// @ts-ignore import pages from './pages.json' with { type: 'json' }; type InputPageData = { diff --git a/packages/qwik/src/core/render/dom/render.unit.tsx b/packages/qwik/src/core/render/dom/render.unit.tsx index bf89c3dd977..60fc11825a1 100644 --- a/packages/qwik/src/core/render/dom/render.unit.tsx +++ b/packages/qwik/src/core/render/dom/render.unit.tsx @@ -778,14 +778,37 @@ test('should clean up subscriptions after calling the returned cleanup function' const fixture = new ElementFixture(); const spies = { - cleanupSpy: false, + cleanup: false, }; const { cleanup } = await render(fixture.host, ); cleanup(); - assert.equal(spies.cleanupSpy, true); + assert.equal(spies.cleanup, true); +}); + +test('should clean up nested subscriptions after calling the returned cleanup function', async () => { + const fixture = new ElementFixture(); + + const spies = { + parentCleanup: false, + cleanup: false, + slottedCleanup: false, + }; + + const { cleanup } = await render( + fixture.host, + + + + ); + + cleanup(); + + assert.equal(spies.parentCleanup, true); + assert.equal(spies.cleanup, true); + assert.equal(spies.slottedCleanup, true); }); async function expectRendered(fixture: ElementFixture, expected: string) { @@ -1045,16 +1068,56 @@ export const Hooks = component$(() => { ////////////////////////////////////////////////////////////////////////////////////////// -export const CleanupComponent = component$((props: { spies: { cleanupSpy: boolean } }) => { +interface CleanupProps { + spies: { + parentCleanup?: boolean; + cleanup?: boolean; + slottedCleanup?: boolean; + }; +} + +export const ParentCleanupComponent = component$((props: CleanupProps) => { useTask$(({ cleanup }) => { cleanup(() => { - props.spies.cleanupSpy = true; + props.spies.parentCleanup = true; + }); + }); + + return ( +
+
true
+ + + +
+ ); +}); + +export const CleanupComponent = component$((props: CleanupProps) => { + useTask$(({ cleanup }) => { + cleanup(() => { + props.spies.cleanup = true; }); }); return (
true
+ +
+ ); +}); + +export const SlottedCleanupComponent = component$((props: CleanupProps) => { + useTask$(({ cleanup }) => { + cleanup(() => { + props.spies.slottedCleanup = true; + }); + }); + + return ( +
+
true
); }); diff --git a/packages/qwik/src/core/render/dom/visitor.ts b/packages/qwik/src/core/render/dom/visitor.ts index 401749980ba..988b4206474 100644 --- a/packages/qwik/src/core/render/dom/visitor.ts +++ b/packages/qwik/src/core/render/dom/visitor.ts @@ -1135,11 +1135,12 @@ export const cleanupTree = ( elm: Node | VirtualElement, staticCtx: RenderStaticContext, subsManager: SubscriptionManager, - stopSlots: boolean + stopSlots: boolean, + unmount = false ) => { subsManager.$clearSub$(elm); if (isQwikElement(elm)) { - if (stopSlots && elm.hasAttribute(QSlotS)) { + if (!unmount && stopSlots && elm.hasAttribute(QSlotS)) { staticCtx.$rmSlots$.push(elm); return; } @@ -1150,7 +1151,7 @@ export const cleanupTree = ( const end = isVirtualElement(elm) ? elm.close : null; let node: Node | null | VirtualElement = elm.firstChild; while ((node = processVirtualNodes(node))) { - cleanupTree(node!, staticCtx, subsManager, true); + cleanupTree(node!, staticCtx, subsManager, true, unmount); node = node.nextSibling; if (node === end) { break; From a47fe9d27748b866689011339c0460f01ed4820e Mon Sep 17 00:00:00 2001 From: sashkashishka Date: Sun, 16 Nov 2025 11:19:41 +0100 Subject: [PATCH 3/8] fix: add missing files --- packages/qwik/src/core/render/dom/render.public.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/qwik/src/core/render/dom/render.public.ts b/packages/qwik/src/core/render/dom/render.public.ts index 27ecd32ba09..c4d875cf254 100644 --- a/packages/qwik/src/core/render/dom/render.public.ts +++ b/packages/qwik/src/core/render/dom/render.public.ts @@ -124,7 +124,7 @@ export const injectQContainer = (containerEl: Element) => { function cleanupContainer(renderCtx: RenderContext, container: Element) { const subsManager = renderCtx.$static$.$containerState$.$subsManager$; - cleanupTree(container, renderCtx.$static$, subsManager, true); + cleanupTree(container, renderCtx.$static$, subsManager, true, true); removeContainerState(container); From c1bbb2d535abac5ff84b0de0e0df7e6c28958ecc Mon Sep 17 00:00:00 2001 From: sashkashishka Date: Mon, 17 Nov 2025 16:52:19 +0100 Subject: [PATCH 4/8] refactor: use dispose flag name instead of cleanup --- packages/qwik/src/core/render/dom/visitor.ts | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/packages/qwik/src/core/render/dom/visitor.ts b/packages/qwik/src/core/render/dom/visitor.ts index 988b4206474..e7ff49f4bf4 100644 --- a/packages/qwik/src/core/render/dom/visitor.ts +++ b/packages/qwik/src/core/render/dom/visitor.ts @@ -1136,11 +1136,11 @@ export const cleanupTree = ( staticCtx: RenderStaticContext, subsManager: SubscriptionManager, stopSlots: boolean, - unmount = false + dispose = false ) => { subsManager.$clearSub$(elm); if (isQwikElement(elm)) { - if (!unmount && stopSlots && elm.hasAttribute(QSlotS)) { + if (!dispose && stopSlots && elm.hasAttribute(QSlotS)) { staticCtx.$rmSlots$.push(elm); return; } @@ -1151,7 +1151,7 @@ export const cleanupTree = ( const end = isVirtualElement(elm) ? elm.close : null; let node: Node | null | VirtualElement = elm.firstChild; while ((node = processVirtualNodes(node))) { - cleanupTree(node!, staticCtx, subsManager, true, unmount); + cleanupTree(node!, staticCtx, subsManager, true, dispose); node = node.nextSibling; if (node === end) { break; From 518a819383acd7041427301e5ffdc8f519dd321a Mon Sep 17 00:00:00 2001 From: sashkashishka Date: Mon, 17 Nov 2025 16:52:38 +0100 Subject: [PATCH 5/8] feat: speed up partytown test --- starters/apps/starter-partytown-test/src/root.tsx | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/starters/apps/starter-partytown-test/src/root.tsx b/starters/apps/starter-partytown-test/src/root.tsx index 72ce5c7d1e9..801a3fcfb96 100644 --- a/starters/apps/starter-partytown-test/src/root.tsx +++ b/starters/apps/starter-partytown-test/src/root.tsx @@ -9,7 +9,12 @@ export default () => { Qwik + Partytown Blank App -