fix(core): fix type inference across auth actions#190
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub. |
|
Warning Review limit reached
More reviews will be available in 35 minutes and 52 seconds. Learn how PR review limits work. Your organization has used up its prepaid credits, and credit purchases are no longer available. Enable the review add-on in the billing tab to keep reviews running — you're only billed for reviews past your plan's rate limits ($0.25/file). ⌛ How to resolve this issue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based credits. 🚦 How do rate limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan refill rate. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, the refill rate gradually slows as usage increases. The highest same-day bursts are limited more strictly. Please see our Fair Usage Limits Policy for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (4)
📝 WalkthroughWalkthroughIntroduces four new type utilities ( ChangesType inference for signUp and updateSession
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Suggested labels
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 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: 4
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
packages/core/src/validator/registry.ts (1)
123-157:⚠️ Potential issue | 🟠 MajorAdd
.optional()touserin allgetFullSchemaruntime builders to match return type.
ReturnUpdateSessionShapedeclaresuser?as optional for all four schema types (Zod, Valibot, ArkType, TypeBox), but each runtime builder currently constructsuser: schemaas required. This allows optional-user payloads to type-check while failing validation at runtime.Proposed fix
if (isValibotSchema(schema)) { return valibot.object({ - user: schema, + user: valibot.optional(schema as valibot.BaseSchema<any, any, any>), expires: valibot.optional( valibot.pipe( valibot.string(), valibot.transform((input) => new Date(input)), valibot.date() ) ), }) } if (isArkType(schema)) { return type({ - user: schema, + user: schema.optional(), expires: type("string") .pipe((input) => new Date(input)) .optional(), }) } if (IsObject(schema)) { return Typebox.Object({ - user: schema, + user: Typebox.Optional(schema), expires: Typebox.Optional(Typebox.String()), }) as unknown as ReturnUpdateSessionShape<Schema> } if (isZodSchema(schema)) { return z.object({ - user: schema, + user: schema.optional(), expires: z.coerce.date().optional(), }) as unknown as ReturnUpdateSessionShape<Schema> }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/core/src/validator/registry.ts` around lines 123 - 157, The user field in all four runtime schema builders (isValibotSchema, isArkType, IsObject for TypeBox, and isZodSchema) is currently defined as required, but the return type ReturnUpdateSessionShape declares user as optional. Add the appropriate optional wrapper to the user field in each builder to match the return type: wrap the user property with valibot.optional() in the Valibot builder, .optional() in the ArkType builder, Typebox.Optional() in the TypeBox builder, and z.optional() in the Zod builder.
🧹 Nitpick comments (3)
packages/core/src/actions/signUp/signUp.ts (1)
31-32: ⚡ Quick winReplace bare
@ts-ignorewith a scoped@ts-expect-error(or remove it).This suppression is too broad and can hide unrelated typing regressions in this block. Prefer
@ts-expect-errorwith a short reason so it fails closed when the underlying typing issue is fixed.Suggested change
- // `@ts-ignore` - Type excessively expensive to compute. + // `@ts-expect-error` - Deep generic inference with router body type is currently too expensive. const payload = ctx.body as any🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/core/src/actions/signUp/signUp.ts` around lines 31 - 32, Replace the bare `@ts-ignore` comment on the payload variable declaration with `@ts-expect-error` and include a specific reason for the suppression (in this case, explaining that the typing calculation is too expensive). The `@ts-expect-error` directive is more scoped than `@ts-ignore` and will fail to compile if the underlying typing issue is resolved, preventing accidental regressions. Update the comment from `@ts-ignore` - Type excessively expensive to compute to `@ts-expect-error` Type excessively expensive to compute or similar format that clearly documents why the type assertion is needed.packages/core/src/client/client.ts (1)
174-176: ⚡ Quick winUse
@ts-expect-errorinstead of unscoped@ts-ignoreinsignUp.This is currently unbounded suppression. Switching to
@ts-expect-error(with reason) preserves intent and prevents silently hiding future typing regressions.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/core/src/client/client.ts` around lines 174 - 176, Replace the `// `@ts-ignore`` comment on the line before the `client.post("/signUp", ...)` call with `// `@ts-expect-error`` followed by a brief reason explaining why the TypeScript error is expected at that location. This change from unbounded suppression to scoped error expectation prevents masking future typing issues and maintains clarity about intentional type mismatches.packages/core/src/validator/registry.ts (1)
120-122: ⚡ Quick winConstrain
SchematoSchemaTypesto preserve the API contract.The current signature allows
Schemato be inferred as non-schema types; those calls only fail at runtime. ConstrainingSchemakeeps this safeguard at compile time.Proposed refactor
-export const getFullSchema = <Identity extends Identities, Schema = EditableToSchema<Identity>>( +export const getFullSchema = <Identity extends Identities, Schema extends SchemaTypes = EditableToSchema<Identity>>( schema: Schema ): ReturnUpdateSessionShape<Schema> => {🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/core/src/validator/registry.ts` around lines 120 - 122, The `getFullSchema` generic function has an unconstrained `Schema` type parameter that allows non-schema types to be passed, causing type errors only at runtime. Add a constraint to the `Schema` generic parameter in the `getFullSchema` function signature by extending it to `SchemaTypes`. This constraint should be applied to the `Schema` generic parameter (the one with default value `EditableToSchema<Identity>`) to ensure type safety at compile time and preserve the API contract.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@packages/core/CHANGELOG.md`:
- Around line 21-25: Update the PR references in the CHANGELOG.md Unreleased
section from `#189` to `#190` in both the Fixed and Changed entries. The two entries
about type inference for authentication actions with createAuth() and
createAuthClient(), and error handling standardization with AuraAuthError,
currently link to the wrong PR number. Change all occurrences of [`#189`] to
[`#190`] in these two bullet points to ensure accurate changelog traceability.
In `@packages/core/src/`@types/utility.ts:
- Around line 95-98: The ReturnUpdateSessionShape type has a type-runtime
mismatch for the expires field. Currently, the expires field is typed as
ZodOptional<ZodString>, but the runtime builder (getFullSchema) actually creates
z.coerce.date().optional(), which coerces the string to a Date. To fix this,
change the expires field declaration from ZodOptional<ZodString> to
ZodOptional<ZodTypeAny> to accurately reflect the actual schema produced at
runtime and match the behavior of other schema validators like Valibot, ArkType,
and Typebox.
In `@packages/core/src/actions/updateSession/updateSession.ts`:
- Around line 8-11: Make the `updateSessionAction` function itself generic by
adding an `Identity extends Identities` type parameter to match the pattern
already successfully implemented in `signUpAction`. This will allow the concrete
`Identity` type to be preserved and inferred at the call site in
`createAuthInstance`, enabling proper identity-driven type inference for
update-session payloads. Update the function signature of `updateSessionAction`
to accept the generic `Identity` parameter and pass it through to the `config`
function so the type information flows correctly through the entire call chain.
In `@packages/core/test/client/client.test-d.ts`:
- Around line 328-333: The test name on line 328 contains "arktype" but the test
is actually using a TypeBox schema as shown by the typebox.Type usage. Change
the test description string from "with custom arktype signUp schema" to "with
custom typebox signUp schema" to accurately reflect what the test is validating.
---
Outside diff comments:
In `@packages/core/src/validator/registry.ts`:
- Around line 123-157: The user field in all four runtime schema builders
(isValibotSchema, isArkType, IsObject for TypeBox, and isZodSchema) is currently
defined as required, but the return type ReturnUpdateSessionShape declares user
as optional. Add the appropriate optional wrapper to the user field in each
builder to match the return type: wrap the user property with valibot.optional()
in the Valibot builder, .optional() in the ArkType builder, Typebox.Optional()
in the TypeBox builder, and z.optional() in the Zod builder.
---
Nitpick comments:
In `@packages/core/src/actions/signUp/signUp.ts`:
- Around line 31-32: Replace the bare `@ts-ignore` comment on the payload variable
declaration with `@ts-expect-error` and include a specific reason for the
suppression (in this case, explaining that the typing calculation is too
expensive). The `@ts-expect-error` directive is more scoped than `@ts-ignore` and
will fail to compile if the underlying typing issue is resolved, preventing
accidental regressions. Update the comment from `@ts-ignore` - Type excessively
expensive to compute to `@ts-expect-error` Type excessively expensive to compute
or similar format that clearly documents why the type assertion is needed.
In `@packages/core/src/client/client.ts`:
- Around line 174-176: Replace the `// `@ts-ignore`` comment on the line before
the `client.post("/signUp", ...)` call with `// `@ts-expect-error`` followed by a
brief reason explaining why the TypeScript error is expected at that location.
This change from unbounded suppression to scoped error expectation prevents
masking future typing issues and maintains clarity about intentional type
mismatches.
In `@packages/core/src/validator/registry.ts`:
- Around line 120-122: The `getFullSchema` generic function has an unconstrained
`Schema` type parameter that allows non-schema types to be passed, causing type
errors only at runtime. Add a constraint to the `Schema` generic parameter in
the `getFullSchema` function signature by extending it to `SchemaTypes`. This
constraint should be applied to the `Schema` generic parameter (the one with
default value `EditableToSchema<Identity>`) to ensure type safety at compile
time and preserve the API contract.
🪄 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: 034f6a8c-af5b-4fc6-8383-b7fdd68802c0
📒 Files selected for processing (9)
packages/core/CHANGELOG.mdpackages/core/src/@types/utility.tspackages/core/src/actions/signUp/signUp.tspackages/core/src/actions/updateSession/updateSession.tspackages/core/src/api/createApi.tspackages/core/src/client/client.tspackages/core/src/createAuth.tspackages/core/src/validator/registry.tspackages/core/test/client/client.test-d.ts
1 participant
Description
This pull request fixes type inference issues affecting authentication actions such as
updateSessionandsignUp.The issue caused incorrect type inference in both
createAuth()andcreateAuthClient(), resulting in improperly typed payloads and action parameters in certain configurations.The root cause was incorrect type inference from the
identity.schemaandsignUp.schemaconfiguration options. These schemas are used to validate incoming payloads and should also drive the inferred types exposed by the authentication APIs. However, the inferred types were not being propagated correctly, leading to inconsistencies between the configured schemas and the generated API types.