feat: add @please-auth/firestore adapter package#2
Merged
Conversation
There was a problem hiding this comment.
6 issues found across 18 files
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="packages/firestore/src/types.ts">
<violation number="1" location="packages/firestore/src/types.ts:21">
P3: Fix the `collections` JSDoc to use the actual plural keys (`users`, `sessions`, `accounts`, `verifications`).</violation>
</file>
<file name="packages/firestore/src/firestore.ts">
<violation number="1" location="packages/firestore/src/firestore.ts:16">
P2: Fix the JSDoc example import path to `@please-auth/firestore`; the current path is incorrect for this package.
(Based on your team's feedback about package naming under the @please-auth scope.) [FEEDBACK_USED]</violation>
<violation number="2" location="packages/firestore/src/firestore.ts:27">
P1: When `name` is omitted, avoid `apps[0]` because it may select the wrong Firebase app in multi-app runtimes. Prefer reusing the default app explicitly.</violation>
</file>
<file name="packages/firestore/src/adapter.ts">
<violation number="1" location="packages/firestore/src/adapter.ts:178">
P2: `findMany` ID-in fast path ignores `sortBy`. When the where clause is `id in [...]`, results are returned in the order of the input IDs array rather than sorted by the requested field.</violation>
<violation number="2" location="packages/firestore/src/adapter.ts:297">
P1: `updateMany` ignores the `transaction` parameter. When invoked inside a Firestore transaction, the query read uses `query.get()` instead of `transaction.get(query)`, and mutations go through `batchUpdate` (which uses `db.batch()`), bypassing the transaction entirely. This breaks transactional atomicity.</violation>
<violation number="3" location="packages/firestore/src/adapter.ts:358">
P1: `deleteMany` ignores the `transaction` parameter. Same issue as `updateMany`: reads use `query.get()` directly and deletes go through `batchDelete` (`db.batch()`), bypassing the transaction when called in a transactional context.</violation>
</file>
Architecture diagram
sequenceDiagram
participant App as App (Server)
participant BA as Better Auth Core
participant Adapter as @please-auth/firestore
participant Mapper as Field Mapper
participant FS as Cloud Firestore (Admin SDK)
Note over App,FS: Initialization Flow
App->>Adapter: NEW: initFirestore(config)
Adapter->>FS: Check existing Firebase apps
FS-->>Adapter: Return Firestore instance
Adapter-->>App: db instance
App->>BA: betterAuth({ database: firestoreAdapter({ db, namingStrategy }) })
Note over BA,FS: Runtime Query Flow (e.g., findOne / findMany)
BA->>Adapter: findOne({ model, where, select })
alt NEW: Fast Path (ID-based lookup)
Adapter->>FS: doc(id).get()
else Standard Query
Adapter->>Mapper: CHANGED: toFirestore(field names)
Mapper-->>Adapter: Mapped names (e.g. snake_case)
Adapter->>FS: collection.where(...).get()
end
FS-->>Adapter: DocumentData
opt NEW: Client-side Filtering
Note right of Adapter: Logic for 'contains', 'ends_with', or >10 'not_in'
Adapter->>Adapter: matchesAllClientFilters(data)
end
Adapter->>Mapper: CHANGED: fromFirestore(data)
Note right of Mapper: Converts Timestamps to Date & keys to camelCase
Mapper-->>Adapter: Cleaned Record
Adapter-->>BA: Result
Note over BA,FS: Bulk Operation Flow (updateMany / deleteMany)
BA->>Adapter: NEW: updateMany({ model, where, update })
Adapter->>FS: Query for document references
FS-->>Adapter: List of DocumentRefs
loop NEW: WriteBatch Chunking (500 docs / batch)
Adapter->>FS: batch.update(ref, data)
Adapter->>FS: batch.commit()
end
Adapter-->>BA: Affected Count
Note over BA,FS: Transaction Flow
BA->>Adapter: Transactional Operation
Adapter->>FS: db.runTransaction(async tx => { ... })
Adapter->>Adapter: Pass 'transaction' object to CRUD methods
Adapter->>FS: CHANGED: transaction.get(ref) / transaction.set(ref)
FS-->>BA: Atomic Success/Failure
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
amondnet
force-pushed
the
amondnet/minsu-lee/add-firestore-adatper
branch
from
March 24, 2026 02:33
f9f8933 to
20e98ab
Compare
- Batch ops throw richer errors with committedCount on partial failure - matchesClientFilter returns false for unknown operators (fail-closed) - applyOperator returns null for unknown operators (client-side fallback) - updateMany/deleteMany/count now respect transaction context - Eliminate unnecessary Firestore re-read after create - Enforce Firestore in operator 30-value limit - Propagate debugLogs to transaction adapter config
amondnet
force-pushed
the
amondnet/minsu-lee/add-firestore-adatper
branch
from
March 24, 2026 03:22
b1eff6e to
bb274ca
Compare
- Fix JSDoc collections key names in types.ts - Fix import path in firestore.ts JSDoc example - Use getApp() instead of apps[0] to avoid wrong app in multi-app runtimes - Apply sortBy to findMany ID-in fast path
There was a problem hiding this comment.
2 issues found across 3 files (changes from recent commits).
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="packages/firestore/src/adapter.ts">
<violation number="1" location="packages/firestore/src/adapter.ts:185">
P2: Sorting is applied after projection, so `sortBy` breaks when the sort field is not included in `select`.</violation>
</file>
<file name="packages/firestore/src/firestore.ts">
<violation number="1" location="packages/firestore/src/firestore.ts:28">
P2: Using `getApp()` as the unnamed fallback can throw when only named Firebase apps exist. Reuse an existing app instance from `getApps()` instead of requiring a default app.</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
- adapter.ts: Fix sortBy breaking when sort field not included in select by sorting on full (unprojected) records before applying projection - firestore.ts: Replace getApp() fallback with apps[0] to avoid throw when only named Firebase apps exist
There was a problem hiding this comment.
2 issues found across 2 files (changes from recent commits).
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="packages/firestore/src/firestore.ts">
<violation number="1" location="packages/firestore/src/firestore.ts:28">
P2: Selecting `apps[0]` when `name` is omitted can bind to the wrong Firebase app in multi-app runtimes; pick the default app explicitly (with fallback) instead of relying on array order.</violation>
</file>
<file name="packages/firestore/src/adapter.ts">
<violation number="1" location="packages/firestore/src/adapter.ts:210">
P2: The new `id in [...]` projection path can drop `id` when `select` is used, causing inconsistent result shape versus other adapter paths.</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
- firestore.ts: Prefer default Firebase app by name ("[DEFAULT]") before
falling back to apps[0] to avoid binding to wrong app in multi-app runtimes
- adapter.ts: Always include `id` in projected results for consistent
result shape across all adapter paths
- Add @antfu/eslint-config + eslint to root devDependencies - Add eslint.config.mjs with antfu preset - Replace biome with eslint in firestore package scripts - Add lint:package (publint --strict) to firestore package - Fix all eslint style issues (semicolons, quotes, brace-style)
- Use { query: { token } } instead of { token } to match better-auth
client GET endpoint signature
- Add @vitest/coverage-istanbul to root devDependencies
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
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.
Summary
@please-auth/firestore패키지 추가: better-auth용 Cloud Firestore 어댑터CustomAdapter인터페이스 완전 구현 (create, findOne, findMany, update, updateMany, delete, deleteMany, count)initFirestore서버리스 헬퍼, snake_case 네이밍 전략, WriteBatch 벌크 연산 지원Test plan
namingStrategy: "snake_case"옵션 동작 확인starts_with,contains,ends_with연산자 동작 확인Summary by cubic
Add
@please-auth/firestore, a Cloud Firestore adapter forbetter-auth, and migrate the repo to a Bun + Turborepo monorepo. Improves query performance and transactions, fixessortBywith projections, ensuresidis always included, and makes serverless Firestore init safer.New Features
CustomAdapterCRUD incl.count, bulkupdateMany/deleteMany, batch writes, and transaction support.id/id in [...], OR,starts_with, nativenot-in≤10, enforcedin≤30);contains/ends_withare client-side; unknown operators fail-closed.select,sortBy,offset; automaticTimestamp→Date;sortByworks even if the sort field isn’t inselect; results always includeid.namingStrategy: "snake_case", collection overrides, anddebugLogs(propagates to transactions).initFirestoresafely reuses apps, prefers"[DEFAULT]"before falling back to the first app; no extra read after create; batch ops throw richer errors withcommittedCount.Migration
@please-auth/firestorewith peersbetter-authandfirebase-admin.better-authwithdatabase: firestoreAdapter({ db }).initFirestore({ ... }), enablenamingStrategy: "snake_case", override collections, and add indexverifications(identifier asc, createdAt desc).@antfu/eslint-configand addpublint; fix waitlist tests to use{ query: { token } }; add@vitest/coverage-istanbul.Written for commit c77070c. Summary will update on new commits.