feat: better-auth adapter #416
Conversation
|
Note Other AI code review bot(s) detectedCodeRabbit has detected other AI code review bot(s) in this pull request and will avoid duplicating their findings in the review comments. This may lead to a less comprehensive review. WalkthroughAdds a new Better-Auth adapter package for ZenStack (implementation, schema generator, build/config), introduces ESLint/tsconfig/tsup/vitest and package.json exports, exports several ORM client types, and adjusts CLI schema search order and codegen reference lookups. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ 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 |
|
Claude finished @ymc9's task —— View job Code Review: Better Auth AdapterReviewing the new better-auth adapter implementation. Here's my analysis checklist: Review Tasks:
📋 Code Review Summary✅ Strengths
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 7
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
packages/cli/src/actions/action-utils.ts (1)
33-41: Verify and apply the error message and consider release notes for breaking behavior change.The review comment is accurate. Verification confirms:
Error message mismatch confirmed: The code checks
./schema.zmodelfirst (lines 33-34), then./zenstack/schema.zmodel(lines 35-36), but the error message at line 39 still lists them in reverse order.Breaking change confirmed: The help text in
packages/cli/src/index.tsline 52 documents the default as"zenstack/schema.zmodel", but the new search order will now prefer./schema.zmodelfor projects with both files—a behavioral reversal.Apply the suggested diff to correct the error message ordering, and ensure this breaking change is documented in release notes.
🧹 Nitpick comments (4)
packages/auth-adapters/better-auth/src/schema-generator.ts (2)
465-465: Simple pluralization strategy may not handle irregular plurals.The to-many relation names are generated using
${lowerCaseFirst(relatedModel)}s, which won't correctly handle irregular plurals (e.g., "person" → "persons" instead of "people", "child" → "childs" instead of "children").Consider using a pluralization library like
pluralizefor more robust plural handling, or document this limitation for users to manually adjust relation names when needed.
122-133: Multipleas anytype assertions reduce type safety.Throughout the file (e.g., lines 124, 159, 165, 198, 224, and many others), there are numerous
as anycasts when constructing AST nodes. While this may be necessary due to circular dependencies in AST node construction, it significantly reduces type safety.Consider whether typed factory functions or builder patterns could reduce the need for these assertions while maintaining correct AST structure.
packages/auth-adapters/better-auth/src/adapter.ts (2)
70-113: Where clause conversion logic could be simplified.The
convertWhereClausefunction handles single and multiple where clauses separately with some duplication. Lines 88-96 and 98-106 contain nearly identical mapping logic.Consider extracting the common mapping logic into a helper function:
const mapWhereCondition = (w: Where) => ({ [getFieldName({ model, field: w.field })]: w.operator === 'eq' || !w.operator ? w.value : { [operatorToORMOperator(w.operator)]: w.value, }, }); const convertWhereClause = (model: string, where?: Where[]): any => { if (!where || !where.length) return {}; if (where.length === 1) { const w = where[0]!; if (!w) { throw new BetterAuthError('Invalid where clause'); } return mapWhereCondition(w); } const and = where.filter((w) => w.connector === 'AND' || !w.connector); const or = where.filter((w) => w.connector === 'OR'); const andClause = and.map(mapWhereCondition); const orClause = or.map(mapWhereCondition); return { ...(andClause.length ? { AND: andClause } : {}), ...(orClause.length ? { OR: orClause } : {}), }; };
148-148: Consider making default limit configurable.The default limit of 100 for
findManyoperations is hardcoded. Depending on the use case, this might be too high or too low.Consider making this configurable through
AdapterConfigto allow users to adjust based on their performance requirements.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (11)
packages/auth-adapters/better-auth/eslint.config.js(1 hunks)packages/auth-adapters/better-auth/package.json(1 hunks)packages/auth-adapters/better-auth/src/adapter.ts(1 hunks)packages/auth-adapters/better-auth/src/index.ts(1 hunks)packages/auth-adapters/better-auth/src/schema-generator.ts(1 hunks)packages/auth-adapters/better-auth/tsconfig.json(1 hunks)packages/auth-adapters/better-auth/tsup.config.ts(1 hunks)packages/auth-adapters/better-auth/vitest.config.ts(1 hunks)packages/cli/src/actions/action-utils.ts(1 hunks)packages/language/src/zmodel-code-generator.ts(4 hunks)packages/orm/src/client/crud-types.ts(2 hunks)
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2025-10-21T16:09:31.218Z
Learnt from: ymc9
Repo: zenstackhq/zenstack-v3 PR: 319
File: packages/runtime/src/client/executor/zenstack-query-executor.ts:63-72
Timestamp: 2025-10-21T16:09:31.218Z
Learning: In ZenStack, TypeDefs can be inherited by models. When a TypeDef contains fields with `map` attributes, those mapped field names need to be processed by the QueryNameMapper since they become part of the inheriting model's schema. Therefore, when checking if a schema has mapped names (e.g., in `schemaHasMappedNames`), both `schema.models` and `schema.typeDefs` must be inspected for `@map` and `map` attributes.
Applied to files:
packages/auth-adapters/better-auth/src/schema-generator.ts
🧬 Code graph analysis (3)
packages/orm/src/client/crud-types.ts (2)
packages/schema/src/schema.ts (2)
SchemaDef(11-19)GetModels(120-120)packages/orm/src/utils/type-utils.ts (2)
OrArray(52-52)XOR(70-70)
packages/auth-adapters/better-auth/src/schema-generator.ts (5)
packages/auth-adapters/better-auth/src/adapter.ts (1)
AdapterConfig(16-35)packages/common-helpers/src/upper-case-first.ts (1)
upperCaseFirst(1-3)packages/language/src/zmodel-code-generator.ts (2)
ZModelCodeGenerator(71-388)fieldType(180-187)packages/language/src/generated/ast.ts (1)
isDataModel(391-393)packages/common-helpers/src/lower-case-first.ts (1)
lowerCaseFirst(1-3)
packages/auth-adapters/better-auth/src/adapter.ts (4)
packages/schema/src/schema.ts (2)
SchemaDef(11-19)GetModels(120-120)packages/orm/src/client/contract.ts (2)
ClientContract(52-172)ModelOperations(803-807)packages/orm/src/client/crud-types.ts (1)
UpdateInput(890-894)packages/auth-adapters/better-auth/src/schema-generator.ts (1)
generateSchema(31-57)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (5)
- GitHub Check: Agent
- GitHub Check: CodeQL analysis (javascript-typescript)
- GitHub Check: claude-review
- GitHub Check: build-test (20.x, postgresql)
- GitHub Check: build-test (20.x, sqlite)
🔇 Additional comments (12)
packages/orm/src/client/crud-types.ts (2)
781-794: ExposingCreateManyInputandCreateInputlooks correct and non-breakingMaking these input helpers exported aligns them with the rest of the CRUD typings and is an additive, type-only change (no runtime impact). The generic parameters and
Withouthandling remain unchanged, so existing internal usage should continue to work as before.
890-894: ExportingUpdateInputcompletes the CRUD input surfacePromoting
UpdateInputto an exported type gives external consumers parity with create inputs and matches the surrounding API design. The alias still composesUpdateScalarInputandUpdateRelationInputidentically, so there’s no behavioral change—just expanded type visibility.packages/auth-adapters/better-auth/tsconfig.json (1)
1-8: LGTM!The TypeScript configuration is standard and appropriate for the new adapter package. Extending the base config and including src/**/* is correct.
packages/language/src/zmodel-code-generator.ts (3)
201-201: Systematic refactor to$refTextlooks good.The change from
resolved(ast.decl).nametoast.decl.$refTextis consistent with the overall refactoring pattern in this file.
267-267: LGTM on reference expression refactor.The systematic change to use
$refTextfor reference expressions, member access, and invocations is consistent and cleaner than the previous approach.Also applies to: 277-277, 282-282
163-163: Refactor to$refTextfor mixins is correct and verified.The change properly uses Langium's built-in
$refTextproperty for cross-reference text, which is the standard approach in this codebase (see line 201 for identical pattern with attributes). Generated schemas correctly produce mixin declarations likemodel M mixes A, B { ... }, and existing E2E tests validate that mixin fields and attributes are properly inherited and generated.packages/auth-adapters/better-auth/vitest.config.ts (1)
1-4: LGTM!Standard Vitest configuration extending the base config.
packages/auth-adapters/better-auth/src/index.ts (1)
1-1: LGTM!Standard re-export pattern for the public API surface.
packages/auth-adapters/better-auth/tsup.config.ts (1)
1-13: LGTM!The tsup configuration is properly set up for dual ESM/CJS output with type declarations and source maps.
packages/auth-adapters/better-auth/package.json (1)
21-36: LGTM on exports configuration!The exports map is properly configured for dual ESM/CJS support with correct type definitions for each format. The explicit
./package.jsonexport is a good practice.packages/auth-adapters/better-auth/eslint.config.js (1)
5-9: Remove the@typescript-eslint/no-unused-expressionsdisable—no violations exist in the adapter code.After analyzing all source files (adapter.ts, index.ts, schema-generator.ts), no patterns that violate this rule were found. All expressions are properly assigned, awaited, or produce side effects. Disabling this rule package-wide unnecessarily relaxes linting standards without justification. The current code adheres to best practices and would pass this rule without modification.
packages/auth-adapters/better-auth/src/adapter.ts (1)
42-241: LGTM with minor concerns - Well-structured adapter implementation.The adapter implementation correctly translates BetterAuth operations to ZenStack ORM calls. The transaction support is properly implemented using the lazy options pattern, and the helper functions appropriately handle field name mapping and operator conversion.
There was a problem hiding this comment.
Pull Request Overview
This pull request introduces a Better-Auth adapter for ZenStack, allowing users to integrate Better-Auth authentication with ZenStack ORM. The implementation includes schema generation capabilities, type exports for better API support, and improvements to the ZModel code generator.
Key Changes
- New
@zenstackhq/better-authpackage providing adapter functionality for Better-Auth integration - Exported additional types (
CreateInput,CreateManyInput,UpdateInput) fromcrud-types.tsto support external adapter implementations - Refactored ZModel code generator to use
$refTextinstead of resolved references for more reliable code generation - Updated schema file search priority in CLI to check
./schema.zmodelbefore./zenstack/schema.zmodel
Reviewed Changes
Copilot reviewed 11 out of 12 changed files in this pull request and generated 14 comments.
Show a summary per file
| File | Description |
|---|---|
| pnpm-lock.yaml | Added Better-Auth dependencies and related packages for authentication support |
| packages/orm/src/client/crud-types.ts | Exported CreateInput, CreateManyInput, and UpdateInput types for external use |
| packages/language/src/zmodel-code-generator.ts | Changed reference resolution to use $refText instead of ref?.name |
| packages/cli/src/actions/action-utils.ts | Reversed schema file search priority order |
| packages/auth-adapters/better-auth/* | New adapter package with configuration, implementation, and schema generation |
Files not reviewed (1)
- pnpm-lock.yaml: Language not supported
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
Claude finished @ymc9's task —— View job Claude finished @ymc9's task —— View job Code Review: Better Auth AdapterReviewing the new better-auth adapter implementation. Here's my analysis checklist: Review Tasks:
📋 Code Review Summary✅ Strengths
|
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (6)
packages/auth-adapters/better-auth/src/adapter.ts (1)
178-186: Type assertion bypasses validation on update payload.Similar to the
updateoperation, theupdateManyoperation passes theupdateparameter directly asdatawithout type validation. While TypeScript infers compatibility, runtime validation would prevent issues if BetterAuth's update structure differs from ZenStack's expectations.packages/auth-adapters/better-auth/src/schema-generator.ts (5)
75-93: Consider adding explicit type annotation for clarity.The
toManyRelationsmap (line 76) lacks an explicit type annotation, which would improve code maintainability and catch potential type errors earlier.Based on learnings: When working with mapped data structures that track relationships, explicit typing helps prevent errors when the structure is complex.
232-240: Hardcoded SQLite database path limits flexibility.The SQLite URL defaults to
'file:./dev.db'(line 236), which is a development-specific path. This should be configurable to support different environments and deployment scenarios.
284-288: Field existence check only verifies name, not definition.The check on line 286 only verifies if a field name exists but doesn't validate whether the field's type, attributes, or other properties have changed. If BetterAuth updates field definitions, the generated schema could become out of sync.
354-356: Foreign key field addition doesn't check for duplicates.When adding a foreign key field (line 355), there's no check to see if the field already exists, unlike the scalar field handling at line 286. This could lead to duplicate fields or errors.
119-132: Type assertion bypasses safety when constructing AST nodes.The AttributeArg is created with
as any(line 123), which bypasses TypeScript's type checking. This pattern is repeated throughout the file (lines 164, 197, 208, 302, 368, 400, 422, 444, 472, 524, 537).
🧹 Nitpick comments (3)
packages/auth-adapters/better-auth/src/adapter.ts (2)
143-159: Consider making the default limit configurable.The
findManyoperation uses a hardcoded default limit of 100 records (line 148). For production use cases, this default might be too restrictive or too permissive depending on the data model and query patterns.Consider adding a
defaultLimitoption toAdapterConfigto allow users to configure this behavior:export interface AdapterConfig { provider: 'sqlite' | 'postgresql'; debugLogs?: DBAdapterDebugLogOption | undefined; usePlural?: boolean | undefined; + defaultLimit?: number | undefined; }Then use it in the adapter:
return await modelDb.findMany({ where: whereClause, - take: limit || 100, + take: limit || config.defaultLimit || 100, skip: offset || 0,
223-230: Non-null assertion assumes lazyOptions is set.The transaction callback uses
lazyOptions!(line 228) andadapterOptions!.config(line 226). While the code structure ensures these are set before transactions run, the non-null assertions reduce type safety.Consider restructuring to avoid non-null assertions:
transaction: (cb) => db.$transaction((tx) => { + if (!lazyOptions) { + throw new Error('Adapter not initialized - call returned function first'); + } const adapter = createAdapterFactory({ - config: adapterOptions!.config, + config: adapterOptions.config, adapter: createCustomAdapter(tx as ClientContract<Schema>), - })(lazyOptions!); + })(lazyOptions); return cb(adapter); }),packages/auth-adapters/better-auth/src/schema-generator.ts (1)
460-486: Simple pluralization won't handle irregular plural forms.The to-many relation naming on line 464 uses
${lowerCaseFirst(relatedModel)}s, which simply appends 's'. This won't work correctly for irregular plurals (e.g., "Person" → "Persons" instead of "People", "Child" → "Childs" instead of "Children").Consider using a pluralization library or allowing custom relation names:
// Option 1: Use a library like 'pluralize' import pluralize from 'pluralize'; const relationName = lowerCaseFirst(pluralize(relatedModel)); // Option 2: Add config option for custom relation naming // This would require extending BetterAuthDBSchema or AdapterConfigAlternatively, document this limitation so users know to manually adjust relation names for irregular plurals after generation.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
packages/auth-adapters/better-auth/package.json(1 hunks)packages/auth-adapters/better-auth/src/adapter.ts(1 hunks)packages/auth-adapters/better-auth/src/schema-generator.ts(1 hunks)
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2025-10-21T16:09:31.218Z
Learnt from: ymc9
Repo: zenstackhq/zenstack-v3 PR: 319
File: packages/runtime/src/client/executor/zenstack-query-executor.ts:63-72
Timestamp: 2025-10-21T16:09:31.218Z
Learning: In ZenStack, TypeDefs can be inherited by models. When a TypeDef contains fields with `map` attributes, those mapped field names need to be processed by the QueryNameMapper since they become part of the inheriting model's schema. Therefore, when checking if a schema has mapped names (e.g., in `schemaHasMappedNames`), both `schema.models` and `schema.typeDefs` must be inspected for `@map` and `map` attributes.
Applied to files:
packages/auth-adapters/better-auth/src/schema-generator.ts
🧬 Code graph analysis (2)
packages/auth-adapters/better-auth/src/schema-generator.ts (5)
packages/auth-adapters/better-auth/src/adapter.ts (1)
AdapterConfig(16-35)packages/common-helpers/src/upper-case-first.ts (1)
upperCaseFirst(1-3)packages/language/src/zmodel-code-generator.ts (2)
ZModelCodeGenerator(71-388)fieldType(180-187)packages/language/src/generated/ast.ts (1)
isDataModel(391-393)packages/common-helpers/src/lower-case-first.ts (1)
lowerCaseFirst(1-3)
packages/auth-adapters/better-auth/src/adapter.ts (4)
packages/schema/src/schema.ts (2)
SchemaDef(11-19)GetModels(120-120)packages/orm/src/client/contract.ts (2)
ClientContract(52-172)ModelOperations(803-807)packages/orm/src/client/crud-types.ts (1)
UpdateInput(890-894)packages/auth-adapters/better-auth/src/schema-generator.ts (1)
generateSchema(30-56)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
- GitHub Check: claude-review
- GitHub Check: build-test (20.x, sqlite)
- GitHub Check: build-test (20.x, postgresql)
🔇 Additional comments (2)
packages/auth-adapters/better-auth/package.json (1)
37-53: Better‑Auth dependency alignment looks good nowpeerDependencies and devDependencies for
@better-auth/core/better-authare now consistently^1.3.0, which matches the tested range and avoids version skew for consumers.packages/auth-adapters/better-auth/src/schema-generator.ts (1)
30-56: LGTM! Clear default schema path resolution.The function correctly handles schema path resolution with sensible defaults (
./schema.zmodelor./zenstack/schema.zmodel) and properly determines overwrite behavior based on file existence.
Summary by CodeRabbit
New Features
Changes
Chores