Skip to content
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

fix(core): parent component lookup during pause #5390

Merged
merged 1 commit into from
Nov 2, 2023
Merged
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: 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
Loading