Skip to content

Commit

Permalink
fix(core): parent component lookup during pause
Browse files Browse the repository at this point in the history
the parentCtx attribute was optimized to point directly to parents with
$contexts$ defined, but that broke pausing which needs the immediate parent.

Co-authored-by: Jesse Zhang <jesse.zhang@orchard.com>
  • Loading branch information
wmertens and jessezhang91 committed Nov 2, 2023
1 parent 5277f0c commit edb77a3
Show file tree
Hide file tree
Showing 4 changed files with 85 additions and 11 deletions.
5 changes: 4 additions & 1 deletion packages/qwik/src/core/state/context.ts
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,10 @@ export interface QContext {
$vdom$: ProcessedJSXNode | null;
$slots$: ProcessedJSXNode[] | null;
$dynamicSlots$: QContext[] | null;
/** The Qwik Context of a parent component that has a useContextProvider, null if no parent */
/**
* The Qwik Context of the virtual parent component, null if no parent. For an real element, it's
* the owner virtual component, and for a virtual component it's the wrapping virtual component.
*/
$parentCtx$: QContext | null | undefined;
}

Expand Down
18 changes: 8 additions & 10 deletions packages/qwik/src/core/use/use-context.ts
Original file line number Diff line number Diff line change
Expand Up @@ -291,7 +291,7 @@ export const useContext: UseContext = <STATE extends object>(
throw qError(QError_notFoundContext, context.id);
};

/** Find a wrapping Virtual component in the DOM that has contexts */
/** Find a wrapping Virtual component in the DOM */
const findParentCtx = (el: QwikElement | null, containerState: ContainerState) => {
let node = el;
let stack = 1;
Expand Down Expand Up @@ -332,18 +332,16 @@ const findParentCtx = (el: QwikElement | null, containerState: ContainerState) =
};

const getParentProvider = (ctx: QContext, containerState: ContainerState): QContext | null => {
// `null` means there's no parent, `undefined` means we don't know yet.
if (ctx.$parentCtx$ === undefined) {
// Not fully resumed container, find context from DOM
const wrappingCtx = findParentCtx(ctx.$element$, containerState);
ctx.$parentCtx$ =
!wrappingCtx || wrappingCtx.$contexts$
? wrappingCtx
: // Keep trying until we find a provider
getParentProvider(wrappingCtx, containerState);
} else if (ctx.$parentCtx$ && !ctx.$parentCtx$.$contexts$) {
// Fully resumed container, but parent is not a provider: update the reference
ctx.$parentCtx$ = getParentProvider(ctx.$parentCtx$, containerState);
ctx.$parentCtx$ = findParentCtx(ctx.$element$, containerState);
}
/**
* Note, the parentCtx is used during pause to to get the immediate parent, so we can't shortcut
* the search for $contexts$ here. If that turns out to be needed, it needs to be cached in a
* separate property.
*/
return ctx.$parentCtx$;
};

Expand Down
53 changes: 53 additions & 0 deletions starters/apps/e2e/src/components/context/context.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -56,6 +56,7 @@ export const ContextApp = component$(() => {
<Issue1971 />
<Issue2087 />
<Issue2894 />
<Issue5356 />
</div>
);
});
Expand Down Expand Up @@ -257,3 +258,55 @@ export const Issue2894_Consumer = component$(() => {
const ctx = useContext(CTX_2894);
return <div id="issue2894-value">Value: {ctx.foo}</div>;
});

export const Issue5356Context = createContextId<object>("issue-5356");
export const Issue5356 = component$(() => {
useContextProvider(Issue5356Context, {});

return <Issue5356_Parent />;
});

export const Issue5356_Parent = component$(() => {
const signal = useSignal(1);

const children = [1, 2];

return (
<div id="issue-5356">
<button id="issue5356-button-1" onClick$={() => (signal.value = 1)}>
1
</button>
<button id="issue5356-button-2" onClick$={() => (signal.value = 2)}>
2
</button>

<>
{children.map((value) => (
<Issue5356_Child
key={value}
value={value}
active={signal.value === value}
/>
))}
</>
<>
{[
<Issue5356_Child value={3} active={signal.value === 1} />,
<Issue5356_Child value={4} active={signal.value === 2} />,
]}
</>
</div>
);
});

export const Issue5356_Child = component$<{ value: number; active: boolean }>(
(props) => {
useContext(Issue5356Context);

return (
<div id={`issue5356-child-${props.value}`}>
Child {props.value}, active: {props.active ? "true" : "false"}
</div>
);
},
);
20 changes: 20 additions & 0 deletions starters/e2e/e2e.context.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -144,6 +144,26 @@ test.describe("context", () => {
await expect(value).toHaveText("Value: bar");
await expect(value).toBeVisible();
});

test("issue 5356", async ({ page }) => {
const btn1 = page.locator("#issue5356-button-1");
const btn2 = page.locator("#issue5356-button-2");
const child1 = page.locator("#issue5356-child-1");
const child2 = page.locator("#issue5356-child-2");

await expect(child1).toContainText("Child 1, active: true");
await expect(child2).toContainText("Child 2, active: false");

await btn2.click();

await expect(child1).toContainText("Child 1, active: false");
await expect(child2).toContainText("Child 2, active: true");

await btn1.click();

await expect(child1).toContainText("Child 1, active: true");
await expect(child2).toContainText("Child 2, active: false");
});
}

test.beforeEach(async ({ page }) => {
Expand Down

0 comments on commit edb77a3

Please sign in to comment.