feat(core): standardize API and client options and return types#143
Merged
halvaradop merged 7 commits intomasterfrom Apr 17, 2026
Merged
feat(core): standardize API and client options and return types#143halvaradop merged 7 commits intomasterfrom
halvaradop merged 7 commits intomasterfrom
Conversation
Contributor
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
Warning Rate limit exceeded
Your organization is not enrolled in usage-based pricing. Contact your admin to enable usage-based pricing to continue reviews beyond the rate limit, or try again in 1 minutes and 9 seconds. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: ⛔ Files ignored due to path filters (3)
📒 Files selected for processing (22)
📝 WalkthroughWalkthroughRefactors auth API and types to return standardized objects with Changes
Sequence Diagram(s)sequenceDiagram
participant Client
participant API as Auth API
participant Strategy as SessionStrategy
participant Headers as HeadersStore
Client->>API: call getSession(ctx/options)
API->>Strategy: getSession(request.headers)
Strategy-->>API: { session, headers }
API->>Headers: toUnionHeaders(headers, secureApiHeaders)
API-->>Client: { success: true|false, session|null, headers, toResponse() }
Client->>Client: call toResponse() or apply cookies/redirect based on result
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Suggested labels
Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (2)
packages/core/src/@types/session.ts (2)
238-265: Inconsistent success/failure indicator:AuthActionReturnusesokwhileSessionReturnusessuccess.
AuthActionReturn(lines 238-249) usesok: true/false, butSessionReturn(lines 263-265) andSignInReturn(line 281) usesuccess: true/false. For API consistency, consider aligning on a single property name across all return types.♻️ Proposed alignment
export type AuthActionReturn = | { - ok: true + success: true headers: Headers redirectURL?: string toResponse: () => Response } | { - ok: false + success: false headers: Headers toResponse: () => Response }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/core/src/`@types/session.ts around lines 238 - 265, The return types are inconsistent: AuthActionReturn uses ok while SessionReturn and SignInReturn use success; pick one property name (recommend changing AuthActionReturn to success) and update all related types and any callers to use that single boolean key consistently. Specifically, change AuthActionReturn's discriminant from ok:boolean to success:boolean (and update any code referencing AuthActionReturn.ok), or alternatively rename SessionReturn/SignInReturn to ok if you prefer that convention—ensure AuthActionReturn, SessionReturn, SignInReturn (and all uses of those types/functions) are updated together so the discriminant name is uniform across the codebase.
279-284:SignInReturn.signInURLtype may not accurately reflect all cases.The interface declares
signInURL: string, but whenredirect === true, the implementation insignIn.tsline 75 setssignInURL: authorization(a string) on the object whiletoResponse()returnssignInURL: nullin the JSON body. Consider whether the type should reflect this nuance, or if the current typing is sufficient since it describes the object property (not the JSON response).🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/core/src/`@types/session.ts around lines 279 - 284, The SignInReturn interface's signInURL currently typed as string doesn't reflect that toResponse() may return null for signInURL when redirect is true; update the type to capture this nuance by making signInURL conditional on the generic Redirect (e.g. change signInURL to a conditional type such as Redirect extends true ? string : string | null or, if simpler, string | null) and ensure the symbols mentioned (SignInReturn, signInURL, toResponse, and the signIn.ts implementation) remain consistent with that updated typing so callers and the JSON response types align.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@packages/core/src/api/getSession.ts`:
- Around line 15-20: The unauthorized SessionReturn currently sets headers: new
Headers() while its toResponse() uses the existing headers variable that
contains the expired cookie values, causing inconsistency; update the
unauthorized object so its headers property uses the same headers instance (the
headers variable used in toResponse) instead of new Headers(), or construct a
newHeaders variable for both success and unauthorized and reuse it so
unauthorized.headers and unauthorized.toResponse() are consistent (refer to the
unauthorized object, the headers variable, toResponse(), and the newHeaders
usage in the success path).
In `@packages/core/src/api/signIn.ts`:
- Around line 41-55: When redirect === false the toResponse closure references
headersList which is declared only in the redirect path, causing a
ReferenceError; update the non-redirect branch (the block that calls
createSignInURL and returns the object with toResponse) to define or reuse the
same headersList used in the redirect path before building the response (or
construct headers inline), ensuring the returned toResponse() uses a defined
headersList; modify the signIn flow around createSignInURL/toResponse so both
branches share a single headersList variable or factory and keep the
Response.json call consistent.
In `@packages/next/src/lib/api.ts`:
- Line 34: Remove the debug console.log that prints the full session payload in
the server path: locate the console.log("getSession - Retrieved session:",
session) in the getSession function inside packages/next/src/lib/api.ts and
delete it (or replace it with a non-sensitive log such as a single boolean or
masked userId) so session details are not written to server logs; if needed, use
a proper logger at debug level gated by NODE_ENV or logger.debug with redaction
instead of console.log.
---
Nitpick comments:
In `@packages/core/src/`@types/session.ts:
- Around line 238-265: The return types are inconsistent: AuthActionReturn uses
ok while SessionReturn and SignInReturn use success; pick one property name
(recommend changing AuthActionReturn to success) and update all related types
and any callers to use that single boolean key consistently. Specifically,
change AuthActionReturn's discriminant from ok:boolean to success:boolean (and
update any code referencing AuthActionReturn.ok), or alternatively rename
SessionReturn/SignInReturn to ok if you prefer that convention—ensure
AuthActionReturn, SessionReturn, SignInReturn (and all uses of those
types/functions) are updated together so the discriminant name is uniform across
the codebase.
- Around line 279-284: The SignInReturn interface's signInURL currently typed as
string doesn't reflect that toResponse() may return null for signInURL when
redirect is true; update the type to capture this nuance by making signInURL
conditional on the generic Redirect (e.g. change signInURL to a conditional type
such as Redirect extends true ? string : string | null or, if simpler, string |
null) and ensure the symbols mentioned (SignInReturn, signInURL, toResponse, and
the signIn.ts implementation) remain consistent with that updated typing so
callers and the JSON response types align.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: a48a23c9-c31d-4eea-82dd-f80abcc0eb3d
📒 Files selected for processing (16)
packages/core/src/@types/session.tspackages/core/src/@types/utility.tspackages/core/src/actions/session/session.tspackages/core/src/actions/signIn/signIn.tspackages/core/src/api/createApi.tspackages/core/src/api/getSession.tspackages/core/src/api/signIn.tspackages/core/src/session/stateless.tspackages/core/test/actions/session/session.test.tspackages/core/test/api/getSession.test.tspackages/core/test/api/signIn.test.tspackages/core/test/session/stateless.test.tspackages/core/test/types.test-d.tspackages/elysia/src/lib/with-auth.tspackages/next/src/lib/api.tspackages/react-router/src/lib/api.ts
There was a problem hiding this comment.
Actionable comments posted: 6
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
packages/core/test/actions/signIn/signIn.test.ts (1)
20-28:⚠️ Potential issue | 🟠 MajorFix test expectations to match the API contract:
signInURLshould contain the authorization URL, notnull.The API implementation at
packages/core/src/api/signIn.ts:72-74returnssignInURL: authorizationin the 302 response body. The type definition atpackages/core/src/@types/session.ts:299-300explicitly documents this: "Whenredirectis true, the JSON body still carries the authorization URL for consistency." The test expectations at bothpackages/core/test/actions/signIn/signIn.test.ts:26andpackages/core/test/api/signIn.test.ts:103incorrectly expectsignInURL: null. Either the tests need to be updated to expect the authorization string, or the implementation must be changed to match the test expectations—but the current state violates the documented API contract.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/core/test/actions/signIn/signIn.test.ts` around lines 20 - 28, The tests are asserting signInURL is null but the API (packages/core/src/api/signIn.ts) returns signInURL: authorization when redirect=true; update the expectations in the tests (packages/core/test/actions/signIn/signIn.test.ts and packages/core/test/api/signIn.test.ts) to assert that response.json().signInURL is the authorization URL string (non-null) instead of null, e.g., verify signInURL is a non-empty string or equals the expected authorization value returned by the signIn handler.packages/core/src/client/client.ts (1)
79-161:⚠️ Potential issue | 🟡 MinorCasting
response.json()to*APIReturnmisrepresents the wire body.The server's
toResponse()serializes only the body fields ({ success, redirect?, signInURL | redirectURL | session }), not the full*APIReturn(which also containsheaders: HeadersandtoResponse: () => AuthResponse<...>). Casting the client-sideresponse.json()result toSignInAPIReturn/SignOutAPIReturn/SignInCredentialsAPIReturn/UpdateSessionAPIReturnimplies these fields exist on the parsed JSON — they never will, and any code doingjson.toResponse()orjson.headerswould fail at runtime.Define "body" types in
@/@types/session.ts(the generic parameters you already pass intoAuthActionAPIReturn) and use those on the client:🛠️ Suggested approach
// In session.ts export type SignInAPIBody = | { success: true; redirect: true; signInURL: string } | { success: true; redirect: false; signInURL: string } export type SignOutAPIBody = | { success: true; redirectURL: string } | { success: false; redirectURL: null } // ...etc, then: // client.ts const json = (await response.json()) as SignInAPIBodyApplies to lines 79, 108, 138, and 157.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/core/src/client/client.ts` around lines 79 - 161, The client is casting response.json() to the full *APIReturn types which include headers/toResponse that the wire JSON never contains; instead define distinct "body" types (e.g. SignInAPIBody, SignOutAPIBody, SignInCredentialsAPIBody, UpdateSessionAPIBody) in `@/`@types/session.ts matching the serialized shape and update the casts inside signIn, signOut, updateSession, and signInCredentials to use those body types (and adjust any callers expecting full APIReturns accordingly) so you only treat the actual JSON body fields returned by the server.
🧹 Nitpick comments (6)
packages/core/src/shared/utils.ts (1)
121-128:toUnionHeaderswill drop additionalSet-Cookievalues.
Headers#forEachcombines multiple same-named headers into a single comma-joined string for most names, butSet-Cookieis special — iterating/getting it can collapse or split cookies depending on the runtime, andinit.set(key, value)unconditionally replaces any prior value. Ifheadersever contains multipleSet-Cookieentries andinithas none, only one merged value will be written, silently dropping cookies.Current callers (
getSession,updateSession) passsecureApiHeaders(security/cache headers), so today this is safe, but it's an easy footgun if reused. Consider either documenting the “no multi-valued headers” contract, or usinginit.append(key, value)forset-cookiespecifically:Suggested hardening
export const toUnionHeaders = (init: Headers, headers: HeadersInit): Headers => { new Headers(headers).forEach((value, key) => { if (!init.has(key)) { - init.set(key, value) + if (key.toLowerCase() === "set-cookie") { + init.append(key, value) + } else { + init.set(key, value) + } } }) return init }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/core/src/shared/utils.ts` around lines 121 - 128, toUnionHeaders can drop multiple Set-Cookie values because Headers.forEach may merge them and init.set(...) replaces prior values; update the function so that when the header name lowercased equals "set-cookie" you call init.append(key, value) (not init.set) to preserve multiple cookies, while keeping the existing behavior for other headers (only set if !init.has(key)). Modify the toUnionHeaders implementation accordingly, referencing the toUnionHeaders function and the Headers.forEach iteration.packages/core/src/api/updateSession.ts (1)
13-20: Optional: drop theascast by branching onsession.The
as UpdateSessionAPIReturn<DefaultUser>suggests TypeScript can't prove the returned object satisfies the union. IfUpdateSessionAPIReturnis a{success: true; session: NonNull} | {success: false; session: null}union, splitting the return resolves this safely and keeps the type inference honest.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/core/src/api/updateSession.ts` around lines 13 - 20, The return currently uses a type cast because TypeScript can't prove the union shape; instead branch on session and return the appropriate typed variant so no `as` is needed: if `session` is truthy return an object matching the success-true arm (success: true, session: the non-null session value, headers: newHeaders, toResponse returning status 200), otherwise return the success-false arm (success: false, session: null, headers: newHeaders, toResponse returning status 401). Update the code around the existing `session`, `newHeaders`, and `toResponse` logic so each branch returns the correctly shaped `UpdateSessionAPIReturn<DefaultUser>` without casting.packages/core/src/@types/utility.ts (1)
42-44: Minor:DeepPartialalso descends into arrays/functions.
T[P] extends objectis true for arrays ([]) and functions, soDeepPartialwill recurse into them and produce an array type with optional indices / a function-shape with optional own-keys. If this utility is only used for plain config objects it's fine, but a more conventional form excludes arrays/functions:♻️ Safer shape
-export type DeepPartial<T> = { - [P in keyof T]?: T[P] extends object ? DeepPartial<T[P]> : T[P] -} +export type DeepPartial<T> = T extends (...args: any[]) => any + ? T + : T extends Array<infer U> + ? Array<DeepPartial<U>> + : T extends object + ? { [P in keyof T]?: DeepPartial<T[P]> } + : T🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/core/src/`@types/utility.ts around lines 42 - 44, DeepPartial currently recurses into arrays and functions because it uses "T[P] extends object"; update the DeepPartial conditional so arrays and function types are handled explicitly: when T[P] is an array, map to an array of DeepPartial of the element type; when T[P] is a function, leave it as-is (do not recurse); otherwise, recurse into plain objects. Locate the DeepPartial type and adjust the conditional checks to first test for array (infer element type) and for Function before applying the object recursion.packages/core/src/client/client.ts (1)
146-149:signInCredentialsis typed with OAuthSignInOptions.
SignInOptionsis the OAuth client options type. While the shape (redirect?,redirectTo?) coincidentally works for credentials today, coupling credentials sign-in to an OAuth-named option type makes future divergence painful (e.g. if credentials grows arememberMeor OAuth growsscopeat the client layer). Consider introducing a dedicatedSignInCredentialsOptionsand referencing it here.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/core/src/client/client.ts` around lines 146 - 149, The signInCredentials function is using the OAuth-specific SignInOptions type; create a dedicated type (e.g., SignInCredentialsOptions) that models only the options needed by credentials sign-in (e.g., redirect?, redirectTo?, rememberMe? if applicable), replace SignInOptions with SignInCredentialsOptions in the signInCredentials declaration and any internal/consumer references, and export the new type where appropriate so future divergence between OAuth and credentials options is decoupled (update related overloads/uses that currently assume SignInOptions).packages/react-router/src/@types/index.ts (1)
34-47: Optional: narrow the JSON variants to theredirect: falseunion member.
SignInAPIReturnis itself a union ofredirect: true | falsevariants. WhenOptions extends { redirect: false }, only theredirect: falsevariant can be produced at runtime, so consumers currently have to discriminate aredirectthey already pinned. Narrowing viaExtractyields better inference:♻️ Suggested change
export type ReactRouterSignInReturn<Options extends ReactRouterSignInAPIOptions> = Options extends { redirect: false } - ? SignInAPIReturn + ? Extract<SignInAPIReturn, { redirect: false }> : Response export type ReactRouterSignInCredentialsReturn<Options extends ReactRouterSignInCredentialsAPIOptions> = Options extends { redirect: false } - ? SignInCredentialsAPIReturn + ? SignInCredentialsAPIReturn // already discriminated by success; keep as-is : Response🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/react-router/src/`@types/index.ts around lines 34 - 47, ReactRouterSignInReturn and ReactRouterSignInCredentialsReturn currently return SignInAPIReturn / SignInCredentialsAPIReturn even when Options extends { redirect: false }, which keeps the broader union; change their return types to extract only the redirect:false variant (use Extract<SignInAPIReturn, { redirect: false }> and Extract<SignInCredentialsAPIReturn, { redirect: false }>) so consumers get the narrowed JSON shape when Options pins redirect: false.packages/core/src/@types/session.ts (1)
286-343: Align client return shapes:SignInReturn/SignOutReturnshould be discriminated likeSignInCredentialsReturn.
SignInCredentialsReturnis already a proper discriminated union:{ success: true; redirectURL: string } | { success: false; redirectURL: null }but
SignInReturnandSignOutReturnusesuccess: boolean, which blocks narrowing. Consumers who writeif (result.success)can't narrowsignInURL/redirectURLto non-null in the success branch. The client also populates these differently on error (e.g.client.tsline 86 returns{ success: false, ... signInURL: "/" }), which a discriminated type would have caught.♻️ Suggested change
export type SignInReturn<Redirect extends boolean = boolean> = Redirect extends true ? void - : { success: boolean; redirect: false; signInURL: string | null } + : { success: true; redirect: false; signInURL: string } | { success: false; redirect: false; signInURL: null } export type SignOutReturn<Redirect extends boolean = boolean> = Redirect extends true ? void - : { success: boolean; redirect: false; redirectURL: string } + : { success: true; redirect: false; redirectURL: string } | { success: false; redirect: false; redirectURL: null }This also cleans up the odd error-path return of
signInURL: "/"inclient.ts(line 86) — the type forces it to benull.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/core/src/`@types/session.ts around lines 286 - 343, Update the client return types SignInReturn and SignOutReturn to be discriminated unions like SignInCredentialsReturn (i.e. { success: true; signInURL: string } | { success: false; signInURL: null } for SignInReturn, and { success: true; redirectURL: string } | { success: false; redirectURL: null } for SignOutReturn) so consumers can narrow on success; then fix the client sign-in/sign-out implementation (the client code that returns the failure shape) to return null for signInURL/redirectURL on failure instead of string placeholders. Ensure you change the generic Redirect handling consistently with the existing Redirect extends true -> void behavior and update any call sites to match the new discriminated shapes.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@apps/react-router/app/lib/auth.ts`:
- Line 12: Replace the hardcoded baseURL value with an environment-driven value:
read from process.env.BASE_URL (with a sensible localhost fallback for
development) and use that when building the auth client/origin instead of the
literal "http://localhost:5174"; update the object/property named baseURL in the
auth setup (the baseURL property in apps/react-router/app/lib/auth.ts) to source
from the env var so builds work across dev/staging/prod.
In `@apps/react-router/app/routes/index.tsx`:
- Around line 18-29: The action handler currently returns the raw results `out`
and `signIn` from `api.signOut` and `api.signIn`, which drops response
headers/cookies; instead call and return their `toResponse()` so the generated
Response (with Set-Cookie headers) is sent to the client. Locate the branches
using `action === "signOut"` and `action === "signIn"` and replace returning
`out` / `signIn` with `out.toResponse()` / `signIn.toResponse()` respectively.
In `@packages/core/src/api/createApi.ts`:
- Around line 31-36: The current call sites in createApi (the signOut and
updateSession wrappers) allow callers to override skipCSRFCheck because they
spread ...options after setting skipCSRFCheck: true; change the argument order
so the forced value cannot be overridden (e.g., pass { ctx, ...options,
skipCSRFCheck: true } for signOut and updateSession<DefaultUser>) to guarantee
skipCSRFCheck is always true, or alternatively remove skipCSRFCheck from the
public SignOutAPIOptions/UpdateSessionAPIOptions types if it should not be
caller-controlled.
In `@packages/core/src/api/credentials.ts`:
- Around line 50-55: The returned invalidCredentials object creates a headers
instance (invalidCredentials.headers) from new Headers(secureApiHeaders) but
toResponse() uses the shared secureApiHeaders, causing inconsistency; update
toResponse inside the SignInCredentialsAPIReturn construction so it reuses the
same Headers instance (invalidCredentials.headers) when calling Response.json
and setting the response headers, ensuring both the headers property and
toResponse() return the identical Headers object (reference the
invalidCredentials object, its headers property, the toResponse() function, and
the secureApiHeaders source to locate and change the code).
In `@packages/core/src/api/signOut.ts`:
- Around line 22-32: The current code always sets a Location header and includes
redirectURL even when the caller didn't pass redirectTo, but then returns status
202; change this so the Location header and redirectURL are only present when
redirectTo is truthy: compute redirectToURL via createRedirectTo as before, but
only call HeadersBuilder(...).setHeader("Location", redirectToURL) when
redirectTo is provided (leave headers unchanged otherwise), and in toResponse
return status 302 and include Location/redirectURL when redirectTo is truthy,
otherwise return a 200 (or 204) response without a Location header and without
redirectURL in the JSON body; update the references around headersList,
redirectToURL, redirectTo, and toResponse accordingly.
In `@packages/next/src/`@types/api.ts:
- Around line 24-26: The generic constraint on NextSignInCredentials is wrong:
it currently limits Options to SignInAPIOptions (OAuth) but the
signInCredentials helper supplies SignInCredentialsAPIOptions; update the type
parameter so NextSignInCredentials<Options extends SignInCredentialsAPIOptions>
uses SignInCredentialsAPIOptions instead of SignInAPIOptions to match usage in
signInCredentials (refer to NextSignInCredentials and signInCredentials symbols)
and ensure the conditional type remains unchanged.
---
Outside diff comments:
In `@packages/core/src/client/client.ts`:
- Around line 79-161: The client is casting response.json() to the full
*APIReturn types which include headers/toResponse that the wire JSON never
contains; instead define distinct "body" types (e.g. SignInAPIBody,
SignOutAPIBody, SignInCredentialsAPIBody, UpdateSessionAPIBody) in
`@/`@types/session.ts matching the serialized shape and update the casts inside
signIn, signOut, updateSession, and signInCredentials to use those body types
(and adjust any callers expecting full APIReturns accordingly) so you only treat
the actual JSON body fields returned by the server.
In `@packages/core/test/actions/signIn/signIn.test.ts`:
- Around line 20-28: The tests are asserting signInURL is null but the API
(packages/core/src/api/signIn.ts) returns signInURL: authorization when
redirect=true; update the expectations in the tests
(packages/core/test/actions/signIn/signIn.test.ts and
packages/core/test/api/signIn.test.ts) to assert that response.json().signInURL
is the authorization URL string (non-null) instead of null, e.g., verify
signInURL is a non-empty string or equals the expected authorization value
returned by the signIn handler.
---
Nitpick comments:
In `@packages/core/src/`@types/session.ts:
- Around line 286-343: Update the client return types SignInReturn and
SignOutReturn to be discriminated unions like SignInCredentialsReturn (i.e. {
success: true; signInURL: string } | { success: false; signInURL: null } for
SignInReturn, and { success: true; redirectURL: string } | { success: false;
redirectURL: null } for SignOutReturn) so consumers can narrow on success; then
fix the client sign-in/sign-out implementation (the client code that returns the
failure shape) to return null for signInURL/redirectURL on failure instead of
string placeholders. Ensure you change the generic Redirect handling
consistently with the existing Redirect extends true -> void behavior and update
any call sites to match the new discriminated shapes.
In `@packages/core/src/`@types/utility.ts:
- Around line 42-44: DeepPartial currently recurses into arrays and functions
because it uses "T[P] extends object"; update the DeepPartial conditional so
arrays and function types are handled explicitly: when T[P] is an array, map to
an array of DeepPartial of the element type; when T[P] is a function, leave it
as-is (do not recurse); otherwise, recurse into plain objects. Locate the
DeepPartial type and adjust the conditional checks to first test for array
(infer element type) and for Function before applying the object recursion.
In `@packages/core/src/api/updateSession.ts`:
- Around line 13-20: The return currently uses a type cast because TypeScript
can't prove the union shape; instead branch on session and return the
appropriate typed variant so no `as` is needed: if `session` is truthy return an
object matching the success-true arm (success: true, session: the non-null
session value, headers: newHeaders, toResponse returning status 200), otherwise
return the success-false arm (success: false, session: null, headers:
newHeaders, toResponse returning status 401). Update the code around the
existing `session`, `newHeaders`, and `toResponse` logic so each branch returns
the correctly shaped `UpdateSessionAPIReturn<DefaultUser>` without casting.
In `@packages/core/src/client/client.ts`:
- Around line 146-149: The signInCredentials function is using the
OAuth-specific SignInOptions type; create a dedicated type (e.g.,
SignInCredentialsOptions) that models only the options needed by credentials
sign-in (e.g., redirect?, redirectTo?, rememberMe? if applicable), replace
SignInOptions with SignInCredentialsOptions in the signInCredentials declaration
and any internal/consumer references, and export the new type where appropriate
so future divergence between OAuth and credentials options is decoupled (update
related overloads/uses that currently assume SignInOptions).
In `@packages/core/src/shared/utils.ts`:
- Around line 121-128: toUnionHeaders can drop multiple Set-Cookie values
because Headers.forEach may merge them and init.set(...) replaces prior values;
update the function so that when the header name lowercased equals "set-cookie"
you call init.append(key, value) (not init.set) to preserve multiple cookies,
while keeping the existing behavior for other headers (only set if
!init.has(key)). Modify the toUnionHeaders implementation accordingly,
referencing the toUnionHeaders function and the Headers.forEach iteration.
In `@packages/react-router/src/`@types/index.ts:
- Around line 34-47: ReactRouterSignInReturn and
ReactRouterSignInCredentialsReturn currently return SignInAPIReturn /
SignInCredentialsAPIReturn even when Options extends { redirect: false }, which
keeps the broader union; change their return types to extract only the
redirect:false variant (use Extract<SignInAPIReturn, { redirect: false }> and
Extract<SignInCredentialsAPIReturn, { redirect: false }>) so consumers get the
narrowed JSON shape when Options pins redirect: false.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 8a61f252-6da6-44b9-810f-f0fe279394f3
📒 Files selected for processing (38)
apps/react-router/app/lib/auth.tsapps/react-router/app/routes/index.tsxpackages/core/src/@types/config.tspackages/core/src/@types/errors.tspackages/core/src/@types/index.tspackages/core/src/@types/oauth.tspackages/core/src/@types/router.d.tspackages/core/src/@types/session.tspackages/core/src/@types/utility.tspackages/core/src/actions/signIn/signIn.tspackages/core/src/actions/signInCredentials/signInCredentials.tspackages/core/src/actions/signOut/signOut.tspackages/core/src/actions/updateSession/updateSession.tspackages/core/src/api/createApi.tspackages/core/src/api/credentials.tspackages/core/src/api/getSession.tspackages/core/src/api/signIn.tspackages/core/src/api/signOut.tspackages/core/src/api/updateSession.tspackages/core/src/client/client.tspackages/core/src/oauth/index.tspackages/core/src/shared/utils.tspackages/core/test/actions/signIn/signIn.test.tspackages/core/test/actions/signOut/signOut.test.tspackages/core/test/actions/updateSession/updateSession.test.tspackages/core/test/api/updateSession.test.tspackages/core/test/types.test-d.tspackages/core/vitest.config.tspackages/next/src/@types/api.tspackages/next/src/@types/core.tspackages/next/src/@types/index.tspackages/next/src/lib/api.tspackages/react-router/src/@types/core.tspackages/react-router/src/@types/index.tspackages/react-router/src/lib/api.tspackages/react/src/@types/core.tspackages/react/src/@types/index.tspackages/react/src/@types/types.ts
✅ Files skipped from review due to trivial changes (12)
- packages/react/src/@types/index.ts
- packages/core/src/@types/errors.ts
- packages/core/src/@types/router.d.ts
- packages/react-router/src/@types/core.ts
- packages/next/src/@types/core.ts
- packages/react/src/@types/core.ts
- packages/core/src/@types/index.ts
- packages/next/src/@types/index.ts
- packages/core/src/oauth/index.ts
- packages/react/src/@types/types.ts
- packages/core/test/types.test-d.ts
- packages/core/src/@types/config.ts
🚧 Files skipped from review as they are similar to previous changes (3)
- packages/core/src/api/getSession.ts
- packages/next/src/lib/api.ts
- packages/react-router/src/lib/api.ts
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
This pull request standardizes the API and client types across all packages, improving type safety and consistency when consuming authentication flows. The changes primarily target the
apiobject returned bycreateAuthand the client functions fromcreateAuthClient.The update introduces dedicated types that override default core types, ensuring more precise inference and better developer experience across integrations (e.g., Next.js, React Router).
Additionally, API responses are now strongly typed and include helper utilities to simplify response handling.
Key Changes
successboolean field to all API responses to indicate operation resulttoResponsehelper to transform API results intoResponseobjectsauthenticatedandupdatedflags in favor of unifiedsuccessfield@aura-stack/next,@aura-stack/react-router)Usage
Server-Side Rendering (SSR)
Client-Side Rendering (CSR)