Skip to content

Commit

Permalink
change cleanup vnode chores
Browse files Browse the repository at this point in the history
  • Loading branch information
Varixo committed Aug 27, 2024
1 parent 13f8719 commit f67de35
Show file tree
Hide file tree
Showing 2 changed files with 104 additions and 6 deletions.
30 changes: 24 additions & 6 deletions packages/qwik/src/core/v2/client/vnode-diff.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1213,6 +1213,7 @@ export function cleanup(container: ClientContainer, vNode: VNode) {
if (type & VNodeFlags.Virtual) {
// Only virtual nodes have subscriptions
clearVNodeDependencies(vCursor);
cleanupVNodeChores(vNode, vParent, vCursor, container);
const seq = container.getHostProp<Array<any>>(vCursor as VirtualVNode, ELEMENT_SEQ);
if (seq) {
for (let i = 0; i < seq.length; i++) {
Expand Down Expand Up @@ -1266,12 +1267,6 @@ export function cleanup(container: ClientContainer, vNode: VNode) {
const vFirstChild = vnode_getFirstChild(vCursor);
if (vFirstChild) {
vCursor = vFirstChild;
/**
* Cleanup stale chores for current vCursor, but only if it is not a projection. We need
* to do this to prevent stale chores from running after the vnode is removed. (for
* example signal subscriptions)
*/
container.$scheduler$.cleanupStaleChores(vCursor as VirtualVNode);
continue;
}
} else if (vCursor === vNode) {
Expand Down Expand Up @@ -1336,6 +1331,29 @@ function cleanupStaleUnclaimedProjection(journal: VNodeJournal, projection: VNod
}
}

function cleanupVNodeChores(
vNode: VNode,
vParent: VNode | null,
vCursor: VNode,
container: ClientContainer
) {
/**
* Cleanup stale chores for current vCursor, but only if it is not a projection. We need to do
* this to prevent stale chores from running after the vnode is removed. (for example signal
* subscriptions)
*/
if (vNode !== vCursor) {
container.$scheduler$.cleanupStaleChores(vCursor as VirtualVNode);
} else {
const currentVParent = vParent || vnode_getParent(vNode);
const isParentProjection =
currentVParent && vnode_getProp(currentVParent, QSlot, null) !== null;
if (!isParentProjection) {
container.$scheduler$.cleanupStaleChores(vCursor as VirtualVNode);
}
}
}

/**
* This marks the property as immutable. It is needed for the QRLs so that QwikLoader can get a hold
* of them. This character must be `:` so that the `vnode_getAttr` can ignore them.
Expand Down
80 changes: 80 additions & 0 deletions packages/qwik/src/core/v2/tests/use-visible-task.spec.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,10 @@ import {
useSignal,
useStore,
useVisibleTask$,
useContext,
useComputed$,
useContextProvider,
createContextId,
} from '@builder.io/qwik';
import { trigger, domRender, ssrRenderToDom } from '@builder.io/qwik/testing';
import { ErrorProvider } from '../../../testing/rendering.unit-util';
Expand Down Expand Up @@ -626,5 +630,81 @@ describe.each([
</Component>
);
});

it('#4432 - should cleanup child visible task with correct value', async () => {
const ContextIssue4432 = createContextId<{ url: URL; logs: string }>('issue-4432');

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`;
});
});

return <p>Child A</p>;
});

const Issue4432 = component$(() => {
const loc = useStore({
url: new URL('http://localhost:3000/'),
logs: '',
});
useContextProvider(ContextIssue4432, loc);

return (
<>
<button onClick$={() => (loc.url = new URL('http://localhost:3000/other'))}>
Change
</button>
<pre>{loc.logs}</pre>
{loc.url.pathname === '/' && <Issue4432Child />}
</>
);
});

const { vNode, document } = await render(<Issue4432 />, { debug });

if (render === ssrRenderToDom) {
await trigger(document.body, 'p', 'qvisible');
}

expect(vNode).toMatchVDOM(
<Component>
<Fragment>
<button>Change</button>
<pre>
<Signal>{'VisibleTask ChildA /\n'}</Signal>
</pre>
<Component>
<p>Child A</p>
</Component>
</Fragment>
</Component>
);

await trigger(document.body, 'button', 'click');

expect(vNode).toMatchVDOM(
<Component>
<Fragment>
<button>Change</button>
<pre>
<Signal>{'VisibleTask ChildA /\nCleanup ChildA /other\n'}</Signal>
</pre>
{''}
</Fragment>
</Component>
);
});
});
});

0 comments on commit f67de35

Please sign in to comment.