feat: Move auth pages to tanstack start - #765
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 761d0f76ba
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| if (!result.ok) return signInFormResult(result) | ||
|
|
||
| await invalidateSession(queryClient) | ||
| await router.navigate(parseInternalDestination(destination())) |
There was a problem hiding this comment.
Route legacy return targets with a document navigation
When returnTo names a non-migrated page such as /settings/security—which postAuthDestination explicitly preserves—this always calls TanStack Router navigation even though the current route tree does not own /settings; the browser therefore remains on the TanStack app and renders its not-found page instead of loading the legacy Next.js route. Use ownership-aware navigation here and perform a full-document navigation to the legacy app for destinations not handled by this router.
Useful? React with 👍 / 👎.
| const { error, formSchema, onSubmit } = useSignInForm({ onSignIn }); | ||
|
|
||
| return ( | ||
| <div className="space-y-4"> |
There was a problem hiding this comment.
Replace forbidden
space-y utilities with gaps
The newly shared auth and error components introduce space-y-* utilities in seven places, beginning with this wrapper, despite the repository convention explicitly requiring gap-* and forbidding space-*. Convert these vertical stacks to flex/grid layouts with the corresponding gap utilities so the migrated UI follows the repository's spacing system consistently.
AGENTS.md reference: AGENTS.md:L51-L51
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: af54964235
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| * link into a broken login page rather than an ordinary one. | ||
| */ | ||
| const loginSearchSchema = z.object({ | ||
| returnTo: z.string().optional(), |
There was a problem hiding this comment.
Let the sanitizer handle non-string return targets
When an attacker-controlled login URL contains a JSON-like scalar such as ?returnTo=true, TanStack's default search parser supplies a boolean, so this validator rejects the navigation before beforeLoad can pass the value to postAuthDestination. The login route consequently renders a search-validation error instead of falling back safely to /; accept an unknown optional value here and let sanitizeReturnTo enforce the redirect policy.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: d14a659ff8
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| export const ensureAuthState = async ( | ||
| queryClient: QueryClient, | ||
| ): Promise<AuthState> => | ||
| authStateFromSession(await queryClient.ensureQueryData(sessionQueryOptions())) |
There was a problem hiding this comment.
Refetch stale sessions before evaluating guards
When a session changes after this query has been cached—most immediately after email sign-in, where invalidateSession marks the cached guest session stale—ensureQueryData still returns the existing value by default rather than awaiting a refetch. Because the session query has no active observer, the preceding invalidation also does not refetch it, so navigating to an owned protected destination such as /account evaluates the old guest session and redirects the newly signed-in user back to /login; expiration or cross-tab session changes can likewise remain invisible until the query is removed. Use a read that awaits stale-query refresh (or explicitly remove/refetch this cache entry) before deriving the guard state.
Useful? React with 👍 / 👎.
Improving Documentation
pnpm lint:fixto fix formatting issues before opening the PR.Description
What?
Why?