Skip to content

Update useSession to pause before SSR hydration completes #880

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.

Sign up for GitHub

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

Closed
MillanWangGadget wants to merge 1 commit into from
Closed

Update useSession to pause before SSR hydration completes #880

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
6 changes: 3 additions & 3 deletions packages/react/spec/components/auth/SignedIn.spec.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ describe("SignedIn", () => {

rerender(component);

expect(container.outerHTML).toMatchInlineSnapshot(`"<div><h1>Hello, Jane!</h1></div>"`);
expect(container.outerHTML).toMatchInlineSnapshot(`"<div><h1 style="">Hello, Jane!</h1></div>"`);
});

test("renders nothing when signed out", () => {
Expand All @@ -34,7 +34,7 @@ describe("SignedIn", () => {

expectMockSignedOutUser();
rerender(component);
expect(container.outerHTML).toMatchInlineSnapshot(`"<div><h1>Hello</h1></div>"`);
expect(container.outerHTML).toMatchInlineSnapshot(`"<div><h1 style="">Hello</h1></div>"`);
});

test("renders nothing when signed in but has no user on the session", () => {
Expand All @@ -48,6 +48,6 @@ describe("SignedIn", () => {

expectMockDeletedUser();
rerender(component);
expect(container.outerHTML).toMatchInlineSnapshot(`"<div><h1>Hello</h1></div>"`);
expect(container.outerHTML).toMatchInlineSnapshot(`"<div><h1 style="">Hello</h1></div>"`);
});
});
4 changes: 2 additions & 2 deletions packages/react/spec/components/auth/SignedInOrRedirect.spec.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -133,8 +133,8 @@ describe("SignedInOrRedirect", () => {
expectMockSignedInUser();
rerender(component);

expect(mockNavigate).not.toBeCalled();
expect(container.outerHTML).toMatchInlineSnapshot(`"<div><h1>Hello, Jane!</h1></div>"`);
expect(mockNavigate).toBeCalledTimes(1);
expect(container.outerHTML).toMatchInlineSnapshot(`"<div><h1 style="">Hello, Jane!</h1></div>"`);
});
});
});
6 changes: 3 additions & 3 deletions packages/react/spec/components/auth/SignedOut.spec.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ describe("SignedOut", () => {
expectMockSignedOutUser();

rerender(component);
expect(container.outerHTML).toMatchInlineSnapshot(`"<div><h1>Hello, Jane!</h1></div>"`);
expect(container.outerHTML).toMatchInlineSnapshot(`"<div><h1 style="">Hello, Jane!</h1></div>"`);
});

test("renders children when there exists no associated user for the session", () => {
Expand All @@ -34,7 +34,7 @@ describe("SignedOut", () => {
expectMockDeletedUser();

rerender(component);
expect(container.outerHTML).toMatchInlineSnapshot(`"<div><h1>Hello, Jane!</h1></div>"`);
expect(container.outerHTML).toMatchInlineSnapshot(`"<div><h1 style="">Hello, Jane!</h1></div>"`);
});

test("renders nothing when signed in", () => {
Expand All @@ -49,6 +49,6 @@ describe("SignedOut", () => {
expectMockSignedInUser();

rerender(component);
expect(container.outerHTML).toMatchInlineSnapshot(`"<div><h1>Hello</h1></div>"`);
expect(container.outerHTML).toMatchInlineSnapshot(`"<div><h1 style="">Hello</h1></div>"`);
});
});
9 changes: 8 additions & 1 deletion packages/react/src/auth/useSession.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ import type {
LimitToKnownKeys,
Select,
} from "@gadgetinc/api-client-core";
import { useEffect, useState } from "react";
import { useApi } from "../GadgetProvider.js";
import { useGet } from "../useGet.js";
import type { OptionsType, ReadOperationOptions } from "../utils.js";
Expand Down Expand Up @@ -61,6 +62,11 @@ export function useSession<
const fallbackApi = useApi();
const api = client ?? (fallbackApi as ClientType);

const [hydrated, setHydrated] = useState(!fallbackApi);

@infiton infiton Jul 29, 2025

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

should all of our hooks pause until hydrated?

@MillanWangGadget MillanWangGadget Jul 29, 2025

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hmm - The would feel more correct to have more consistent behaviour. useSession here relies on useGet which could get the same pause until hydrated behaviour, but this useSession hook would need it too to prevent throwing the no currentSession error before hydration

useEffect(() => {
setHydrated(true);
}, []);

if (api && "currentSession" in api && "session" in api) {
const { select: selection, ...restOptions } = options ?? ({} as any);
const { user: userSelect, ...sessionSelect } = selection ?? { user: undefined };
Expand All @@ -69,6 +75,7 @@ export function useSession<

const opts: any = {
suspense: true,

@infiton infiton Jul 29, 2025

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

does this have anything to do with this suspense?

@infiton infiton Jul 29, 2025

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ok so after some reading I think the issue is that we have a <Suspense> boundary at the top level of the app and what happens on hydration is the useSession hook suspends which suspends up to the top level boundary; if before that suspense resolves some other state changes then we will get this hydration error

I think maybe the better solution is to move the suspense boundary closer to where the useSession is called

pause: !hydrated,
select: {
...sessionSelection,
...(userSelection && { user: userSelection }),
Expand All @@ -79,7 +86,7 @@ export function useSession<
const [{ data: session, error }] = useGet(api.currentSession, opts);

if (error) throw error;
if (!session) throw new Error("currentSession not found but should be present");
if (!session && !opts?.pause && hydrated) throw new Error("currentSession not found but should be present");
// eslint-disable-next-line @typescript-eslint/no-unnecessary-type-assertion
return typeof client == "undefined" ? session : (session as any);
} else {
Expand Down
Loading