fix: visible task execution after removal (QwikDev#4459)

fixes QwikDev#4432
CiBuildOrg · Jun 11, 2023 · 7369ead · 7369ead
1 parent 2ebbbb8
commit 7369ead
Show file tree

Hide file tree

Showing 3 changed files with 76 additions and 5 deletions.
diff --git a/packages/qwik/src/core/render/dom/notify-render.ts b/packages/qwik/src/core/render/dom/notify-render.ts
@@ -306,17 +306,21 @@ const executeWatchesAfter = async (
 
   containerState.$watchNext$.forEach((watch) => {
     if (watchPred(watch, false)) {
-      watchPromises.push(then(watch.$qrl$.$resolveLazy$(containerEl), () => watch));
+      if (watch.$el$.isConnected) {
+        watchPromises.push(then(watch.$qrl$.$resolveLazy$(containerEl), () => watch));
+      }
       containerState.$watchNext$.delete(watch);
     }
   });
   do {
     // Run staging effected
     containerState.$watchStaging$.forEach((watch) => {
-      if (watchPred(watch, true)) {
-        watchPromises.push(then(watch.$qrl$.$resolveLazy$(containerEl), () => watch));
-      } else {
-        containerState.$watchNext$.add(watch);
+      if (watch.$el$.isConnected) {
+        if (watchPred(watch, true)) {
+          watchPromises.push(then(watch.$qrl$.$resolveLazy$(containerEl), () => watch));
+        } else {
+          containerState.$watchNext$.add(watch);
+        }
       }
     });
     containerState.$watchStaging$.clear();

diff --git a/starters/apps/e2e/src/components/effect-client/effect-client.tsx b/starters/apps/e2e/src/components/effect-client/effect-client.tsx
@@ -8,6 +8,10 @@ import {
   useSignal,
   useTask$,
   type Signal,
+  useComputed$,
+  useContext,
+  createContextId,
+  useContextProvider,
 } from '@builder.io/qwik';
 import { delay } from '../streaming/streaming';
 
@@ -39,6 +43,7 @@ export const EffectClient = component$(() => {
 
       <Timer />
       <Eager></Eager>
+      <Issue4432 />
     </div>
   );
 });
@@ -266,3 +271,55 @@ export const CleanupEffectsChild = component$((props: { nuCleanups: Signal<numbe
   });
   return <div>Hello</div>;
 });
+
+const ContextIssue4432 = createContextId<{ url: URL; logs: string }>('issue-4432');
+
+export const Issue4432 = component$(() => {
+  const loc = useStore({
+    url: new URL('http://localhost:3000/'),
+    logs: '',
+  });
+  useContextProvider(ContextIssue4432, loc);
+
+  return (
+    <>
+      <button
+        id="issue-4432-button"
+        onClick$={() => (loc.url = new URL('http://localhost:3000/other'))}
+      >
+        Change
+      </button>
+      <pre id="issue-4432-logs">{loc.logs}</pre>
+      {loc.url.pathname === '/' && <Issue4432Child />}
+    </>
+  );
+});
+
+export const Issue4432Child = component$(() => {
+  const state = useContext(ContextIssue4432);
+
+  const pathname = useComputed$(() => state.url.pathname);
+
+  useVisibleTask$(
+    ({ track, cleanup }) => {
+      track(() => pathname.value);
+
+      // This should only run on page load for path '/'
+      state.logs += `VisibleTask ChildA ${pathname.value}\n`;
+
+      // This should only run when leaving the page
+      cleanup(() => {
+        state.logs += `Cleanup ChildA ${pathname.value}\n`;
+      });
+    },
+    {
+      strategy: 'document-ready',
+    }
+  );
+
+  return (
+    <>
+      <p>Child A</p>
+    </>
+  );
+});
diff --git a/starters/e2e/e2e.effect-client.spec.ts b/starters/e2e/e2e.effect-client.spec.ts
@@ -72,4 +72,14 @@ test.describe('effect-client', () => {
     await counter.click();
     await expect(nuCleanups).toHaveText('2');
   });
+
+  test('issue 4432', async ({ page }) => {
+    const button = page.locator('#issue-4432-button');
+    const logs = page.locator('#issue-4432-logs');
+    await page.waitForTimeout(500);
+    await expect(logs).toHaveText('VisibleTask ChildA /\n');
+    await button.click();
+    await page.waitForTimeout(500);
+    await expect(logs).toHaveText('VisibleTask ChildA /\nCleanup ChildA /other\n');
+  });
 });