feat: add trigger support#273
Conversation
Add a `triggers` configuration option to `convexAuth()` that allows registering callbacks for auth table modifications: - onCreate: called after insert operations - onUpdate: called after patch operations (receives old and new doc) - onDelete: placeholder for future delete operation support Triggers are supported for all tables defined in authTables: - users - authAccounts - authSessions - authRefreshTokens - authVerificationCodes - authVerifiers - authRateLimits The AuthTriggers type is derived from authTables for type safety, ensuring table names and document types are enforced at compile time. Triggers run within the same transaction as the auth operation, so errors will cause the operation to roll back. Use cases include audit logging, history tracking, and custom side effects when auth data changes. test: add trigger tests for users and authAccounts tables - Add triggerLog table to test schema for capturing trigger events - Configure triggers in auth.ts using loggedTriggers helper - Test onCreate triggers fire for both users and authAccounts on sign-up - Test onUpdate trigger fires for users on subsequent sign-in - Verify oldDocId is passed correctly to onUpdate handlers
|
@ericsampson is attempting to deploy a commit to the Convex Team on Vercel. A member of the Team first needs to authorize it. |
📝 WalkthroughWalkthroughThis PR adds configurable per-table triggers (onCreate, onUpdate, onDelete) to ConvexAuthConfig and wires a triggered MutationCtx wrapper so configured handlers run automatically during auth-related mutations. Changes
Sequence Diagram(s)sequenceDiagram
participant Client
participant storeImpl as storeImpl\n(with config)
participant createTriggeredCtx as createTriggeredCtx
participant WrappedCtx as WrappedCtx\n(db proxy)
participant Mutation as Mutation Handler
participant DB as Database
participant Trigger as Trigger Handler
Client->>storeImpl: invoke mutation (MutationCtx + config)
storeImpl->>createTriggeredCtx: wrap ctx with trigger logic
createTriggeredCtx-->>storeImpl: WrappedCtx (proxy)
storeImpl->>Mutation: call mutation with WrappedCtx
Mutation->>WrappedCtx: perform db.insert/patch/delete on auth table
WrappedCtx->>DB: execute DB operation
DB-->>WrappedCtx: return newDoc / oldDoc
alt Auth table + triggers configured
WrappedCtx->>Trigger: invoke onCreate/onUpdate/onDelete (ctx, newDoc?, oldDoc?)
Trigger->>DB: optional side-effect operations (e.g., logging)
DB-->>Trigger: ack
Trigger-->>WrappedCtx: resolved
end
WrappedCtx-->>Mutation: return result
Mutation-->>storeImpl: complete
storeImpl-->>Client: return response
Estimated code review effort🎯 4 (Complex) | ⏱️ ~50 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 |
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (5)
test/convex/auth.ts (1)
21-43: Consider using proper document types for stronger type safety.The helper uses
{ _id: string }as the document type, which works but loses type safety. The actualDoc<T>types from the auth library would provide proper field typing.For test code this is acceptable, but if this pattern is intended as an example for users, consider:
🔎 Alternative with proper typing
+import type { Doc } from "@convex-dev/auth/server"; + function loggedTriggers<T extends AuthTableName>(table: T) { return { - onCreate: async (ctx: GenericMutationCtx<DataModel>, doc: { _id: string }) => { + onCreate: async (ctx: GenericMutationCtx<DataModel>, doc: Doc<T>) => { await ctx.db.insert("triggerLog", { trigger: `${table}:onCreate` as const, - docId: doc._id, + docId: doc._id as string, timestamp: Date.now(), }); }, onUpdate: async ( ctx: GenericMutationCtx<DataModel>, - newDoc: { _id: string }, - oldDoc: { _id: string }, + newDoc: Doc<T>, + oldDoc: Doc<T>, ) => { await ctx.db.insert("triggerLog", { trigger: `${table}:onUpdate` as const, - docId: newDoc._id, + docId: newDoc._id as string, timestamp: Date.now(), - oldDocId: oldDoc._id, + oldDocId: oldDoc._id as string, }); }, }; }docs/pages/advanced.mdx (1)
168-183: Example references undefined tables.The example uses
auditLoganduserHistorytables that users would need to define in their schema. Consider adding a brief note or showing the required schema additions.🔎 Suggested addition
Add a note before or after the code block:
> **Note:** The `auditLog` and `userHistory` tables in this example are custom tables > you would need to define in your schema.test/convex/triggers.test.ts (2)
49-90: Solid onUpdate trigger test.The test validates the correct sequence:
- After sign-up: only
onCreatetriggers, noonUpdate- After second sign-in:
onUpdatetriggers fire witholdDocIdcorrectly setOne consideration: the assertion
expect(userUpdateLogs.length).toBeGreaterThanOrEqual(1)on line 85 is somewhat loose. If you expect exactly one update per sign-in,toHaveLength(1)would be more precise.
150-157: Consider extracting shared test setup.The
setupEnvfunction duplicates environment setup that likely exists in other test files. Consider whether this could be shared via a common test utility.src/server/implementation/users.ts (1)
210-261: Account trigger logic handles multiple patch scenarios correctly.The implementation captures
oldDocbefore any patches (line 239) and fetchesnewDocafter all patches complete (line 259), ensuring triggers receive accurate before/after snapshots even when multiple fields are updated (userId, emailVerified, phoneVerified).One observation: with multiple sequential
db.patchcalls (lines 248, 251, 254), each patch is a separate write. While this works correctly for trigger purposes, consolidating into a single patch could reduce write operations if performance becomes a concern.
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (13)
CHANGELOG.mddocs/pages/advanced.mdxdocs/pages/api_reference/server.mdxpackage.jsonsrc/server/implementation/mutations/index.tssrc/server/implementation/mutations/modifyAccount.tssrc/server/implementation/types.tssrc/server/implementation/users.tssrc/server/index.tssrc/server/types.tstest/convex/auth.tstest/convex/schema.tstest/convex/triggers.test.ts
🧰 Additional context used
🧬 Code graph analysis (5)
src/server/types.ts (2)
src/server/implementation/types.ts (1)
AuthTriggers(231-233)src/server/index.ts (1)
AuthTriggers(44-44)
test/convex/auth.ts (3)
src/server/implementation/types.ts (1)
AuthTableName(165-165)src/server/index.ts (2)
AuthTableName(43-43)convexAuth(15-15)test/convex/_generated/dataModel.d.ts (1)
DataModel(60-60)
src/server/implementation/mutations/modifyAccount.ts (2)
src/server/types.ts (1)
ConvexAuthConfig(23-256)src/server/implementation/provider.ts (1)
hash(3-14)
src/server/implementation/users.ts (2)
src/server/index.ts (1)
ConvexAuthConfig(30-30)src/server/types.ts (1)
ConvexAuthConfig(23-256)
src/server/implementation/mutations/index.ts (1)
src/server/implementation/mutations/modifyAccount.ts (1)
modifyAccountImpl(12-54)
🪛 LanguageTool
docs/pages/advanced.mdx
[style] ~152-~152: ‘new record’ might be wordy. Consider a shorter alternative.
Context: ...r types: - onCreate - Called after a new record is inserted - onUpdate - Called after...
(EN_WORDINESS_PREMIUM_NEW_RECORD)
docs/pages/api_reference/server.mdx
[style] ~2404-~2404: ‘new record’ might be wordy. Consider a shorter alternative.
Context: ...blename)<TableName> Called after a new record is inserted. #### onUpdate? > `option...
(EN_WORDINESS_PREMIUM_NEW_RECORD)
🔇 Additional comments (15)
package.json (1)
3-3: LGTM!Version bump to 0.0.91 is appropriate for the new triggers feature.
src/server/implementation/types.ts (1)
156-233: LGTM!Well-designed type system for triggers:
AuthTableNamecorrectly derives fromauthTablesensuring compile-time safety- Trigger handlers receive proper typed
MutationCtxandDoc<TableName>onUpdatecorrectly provides both old and new documents for comparison- The mapped type
AuthTriggerselegantly allows partial configuration per tablesrc/server/index.ts (1)
42-49: LGTM!Clean public API exposure of the new trigger types, allowing consumers to import them directly from
@convex-dev/auth/server.test/convex/schema.ts (1)
24-30: LGTM!Well-designed test table for capturing trigger events:
triggerfield captures the event type (e.g., "users:onCreate")docIdas string is appropriate since it can reference documents from different auth tablesoldDocIdcorrectly optional foronCreatetriggerssrc/server/implementation/mutations/index.ts (1)
148-150: LGTM!Config parameter correctly passed to
modifyAccountImpl, enabling trigger support for account modifications. This follows the same pattern as other mutations that already receive the config.docs/pages/advanced.mdx (1)
219-225: LGTM on important notes!Good coverage of critical behavioral details:
- Transaction scope is clearly explained
- Error handling/rollback behavior is documented
- Full
MutationCtxaccess is highlightedsrc/server/types.ts (3)
18-18: LGTM!The import of
AuthTriggersfrom the implementation types is correctly structured using a type-only import.
226-255: Well-documented triggers configuration.The JSDoc documentation clearly explains the purpose (atomic audit logging, history tracking) and provides a helpful example demonstrating both
onCreateandonUpdatehandlers for multiple tables. The documentation correctly notes that triggers run in the same transaction as the auth operation.
398-398: LGTM!Correctly propagates the
triggersconfiguration to the materialized config, enabling runtime access to trigger handlers.test/convex/triggers.test.ts (2)
13-47: Good test coverage for onCreate triggers.The test properly validates that:
- Both
users:onCreateandauthAccounts:onCreatetriggers fire exactly once- The
docIdin trigger logs matches the actual created documents
92-148: Comprehensive password change trigger test.Good coverage of the password reset flow, verifying that
authAccounts:onUpdatefires after credential modification.src/server/implementation/mutations/modifyAccount.ts (1)
37-51: LGTM! Trigger implementation is correct.The implementation:
- Conditionally captures
oldDoconly when anonUpdatetrigger is configured (efficient)- Re-fetches
newDocafter the patch to get the updated state- Correctly guards trigger invocation with both
oldDocand trigger existence checksThe non-null assertion on line 49 is safe since the patch operation succeeded.
src/server/implementation/users.ts (1)
133-158: LGTM! User trigger implementation is correct.The implementation properly:
- Captures
oldDocbefore the patch operation (line 134-136)- Invokes
onUpdateonly after a successful patch (lines 147-151)- Fetches the fresh document for
onCreateafter insert (lines 154-158)docs/pages/api_reference/server.mdx (2)
1579-1588: LGTM!Clear documentation of the triggers property with correct type reference and link to the advanced usage guide.
2337-2521: Comprehensive API documentation for trigger types.The documentation clearly explains:
AuthTableNameunion type derived fromauthTablesAuthTriggersmapped type structureTableTriggerswith all three handler types- Individual trigger handler signatures with correct parameter types
The code examples effectively illustrate the type structures.
- Add triggeredDb wrapper that uses fn.length to skip unnecessary reads - onCreate: only reads doc if trigger declares doc param (length >= 2) - onUpdate: reads oldDoc only if declared (length >= 3), newDoc if >= 2 - onDelete: now passes id as second param, doc as optional third param - Use union types for trigger signatures to allow optional params - Use AnyCtx (any) for ctx to allow users to access custom tables - Update docs and API reference with new signatures
There was a problem hiding this comment.
Actionable comments posted: 5
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
src/server/implementation/refreshTokens.ts (1)
100-114: Theconfigparameter is unused indeleteAllRefreshTokensbut is deliberately passed by both callers.Line 102 adds a
config: ConvexAuthConfigparameter that is never referenced in the function body (lines 105-113). Both call sites explicitly passconfig:
src/server/implementation/sessions.ts:108src/server/implementation/mutations/refreshSession.ts:54This suggests the parameter is intentionally included for API consistency with other functions in the module that accept triggered contexts, even if not currently used. Consider adding a comment explaining why it's reserved if this pattern is deliberate.
src/server/implementation/mutations/createVerificationCode.ts (1)
39-72: Pass triggered context toupsertUserAndAccountfor trigger execution.The function creates a triggered context (
ctx = createTriggeredCtx(originalCtx, config)at line 28) but then passesoriginalCtxtoupsertUserAndAccount(line 54-55) instead ofctx. SinceupsertUserAndAccountperforms writes (creates/updates users and accounts) that may need to fire triggers, it should receive the triggered context.This is inconsistent with
verifyCodeAndSignIn.ts(line 175-176), which correctly passesctxto the same function. Passctxinstead oforiginalCtxtoupsertUserAndAccount.
🧹 Nitpick comments (2)
src/server/implementation/triggeredDb.ts (1)
61-75: Type safety bypassed withas anyassertions.All trigger lookups use
(triggers as any)[table]?.onCreatewhich bypasses TypeScript's type checking. While this may be necessary for dynamic table access, consider whether the AuthTriggers type can be made more strongly typed to catch misconfigurations at compile time.Also applies to: 81-100, 106-125, 131-140
docs/pages/api_reference/server.mdx (1)
2378-2521: Documentation is thorough and accurate.The documentation for trigger handler types (
TableTriggers,OnCreateTrigger,OnUpdateTrigger,OnDeleteTrigger) is comprehensive, with clear type signatures and descriptions.Optional style improvement: Line 2404 uses "new record" which could be simplified to "record" for conciseness, though this is a minor style preference.
🔎 Optional: Simplify wording
-Called after a new record is inserted. +Called after a record is inserted.
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (15)
docs/pages/advanced.mdxdocs/pages/api_reference/server.mdxsrc/server/implementation/mutations/createVerificationCode.tssrc/server/implementation/mutations/index.tssrc/server/implementation/mutations/invalidateSessions.tssrc/server/implementation/mutations/refreshSession.tssrc/server/implementation/mutations/signOut.tssrc/server/implementation/mutations/userOAuth.tssrc/server/implementation/mutations/verifyCodeAndSignIn.tssrc/server/implementation/refreshTokens.tssrc/server/implementation/sessions.tssrc/server/implementation/triggeredDb.tssrc/server/implementation/types.tstest/convex/auth.tstest/convex/triggers.test.ts
🚧 Files skipped from review as they are similar to previous changes (1)
- test/convex/triggers.test.ts
🧰 Additional context used
🧬 Code graph analysis (9)
src/server/implementation/mutations/signOut.ts (2)
src/server/implementation/types.ts (1)
MutationCtx(138-138)src/server/implementation/sessions.ts (2)
getAuthSessionId(134-141)deleteSession(101-109)
src/server/implementation/mutations/userOAuth.ts (3)
src/server/implementation/types.ts (1)
MutationCtx(138-138)src/server/implementation/provider.ts (2)
GetProviderOrThrowFunc(33-36)Config(38-38)src/server/implementation/triggeredDb.ts (1)
createTriggeredCtx(32-153)
src/server/implementation/sessions.ts (2)
src/server/implementation/triggeredDb.ts (1)
createTriggeredCtx(32-153)src/server/implementation/refreshTokens.ts (1)
deleteAllRefreshTokens(100-114)
src/server/implementation/mutations/refreshSession.ts (2)
src/server/implementation/triggeredDb.ts (1)
createTriggeredCtx(32-153)src/server/implementation/refreshTokens.ts (1)
deleteAllRefreshTokens(100-114)
test/convex/auth.ts (2)
src/server/implementation/types.ts (1)
AuthTriggers(234-236)test-router/convex/auth.ts (1)
convexAuth(4-6)
src/server/implementation/mutations/index.ts (3)
src/server/implementation/mutations/signOut.ts (1)
signOutImpl(11-24)src/server/implementation/mutations/modifyAccount.ts (1)
modifyAccountImpl(12-54)src/server/implementation/mutations/invalidateSessions.ts (1)
invalidateSessionsImpl(24-42)
src/server/implementation/mutations/invalidateSessions.ts (1)
src/server/implementation/sessions.ts (1)
deleteSession(101-109)
src/server/implementation/triggeredDb.ts (1)
src/server/implementation/types.ts (2)
AuthTableName(165-165)MutationCtx(138-138)
src/server/implementation/mutations/verifyCodeAndSignIn.ts (5)
src/server/implementation/types.ts (1)
MutationCtx(138-138)src/server/implementation/provider.ts (2)
GetProviderOrThrowFunc(33-36)Config(38-38)src/server/implementation/triggeredDb.ts (1)
createTriggeredCtx(32-153)src/server/implementation/rateLimit.ts (3)
isSignInRateLimited(6-16)recordFailedSignIn(18-37)resetSignInRateLimit(39-50)src/server/implementation/sessions.ts (3)
getAuthSessionId(134-141)createNewAndDeleteExistingSession(43-56)maybeGenerateTokensForSession(22-41)
🪛 LanguageTool
docs/pages/api_reference/server.mdx
[style] ~2404-~2404: ‘new record’ might be wordy. Consider a shorter alternative.
Context: ...blename)<TableName> Called after a new record is inserted. #### onUpdate? > `option...
(EN_WORDINESS_PREMIUM_NEW_RECORD)
docs/pages/advanced.mdx
[style] ~152-~152: ‘new record’ might be wordy. Consider a shorter alternative.
Context: ...r types: - onCreate - Called after a new record is inserted - onUpdate - Called after...
(EN_WORDINESS_PREMIUM_NEW_RECORD)
🔇 Additional comments (14)
src/server/implementation/mutations/refreshSession.ts (1)
49-54: LGTM! Triggered context properly scoped to failure path.The triggered context is created only for the validation failure branch where session cleanup occurs, ensuring delete triggers fire appropriately. The success paths remain unchanged, which is correct since they don't perform deletions.
test/convex/auth.ts (1)
16-93: LGTM! Comprehensive trigger test configuration.The test setup exercises all three trigger types (onCreate, onUpdate, onDelete) across the main auth tables. Logging to
triggerLogenables verification of trigger execution. The unused_docparameter in onDelete handlers appropriately demonstrates the optional third parameter behavior.src/server/implementation/mutations/signOut.ts (1)
11-19: LGTM! Config parameter properly propagated.The
configparameter is correctly threaded through todeleteSession, which uses it to create a triggered context for firing delete triggers during session cleanup.src/server/implementation/mutations/invalidateSessions.ts (1)
24-38: LGTM! Config parameter properly propagated.The
configparameter is correctly threaded through todeleteSession, enabling trigger support for session invalidation operations. This follows the same pattern assignOut.ts.src/server/implementation/mutations/verifyCodeAndSignIn.ts (2)
17-17: LGTM: Clean separation of contexts for trigger support.The import of
createTriggeredCtxand the immediate creation of a triggered context establishes a clear pattern:originalCtxis used for operations that should not fire triggers (rate limiting, session retrieval), whilectxis used for DB operations that should fire triggers.Also applies to: 29-35
46-77: No concerns with context usage at lines 73-84. The session functions correctly receiveoriginalCtxbecausedeleteSessioncreates its own triggered context internally, andcreateSessionperforms a plain insert without needing triggers. This is the correct pattern for functions that manage their own trigger context creation.src/server/implementation/sessions.ts (1)
18-18: LGTM: Consistent trigger context pattern.The
deleteSessionfunction now acceptsconfigand creates its own triggered context internally. This ensures that delete operations on sessions and refresh tokens fire the appropriate triggers. The pattern is consistent with the broader changes across mutations.Also applies to: 101-109
src/server/implementation/mutations/createVerificationCode.ts (1)
8-8: LGTM: Consistent trigger context creation.The function follows the same pattern as other mutations: rename the parameter to
originalCtxand create a triggered context for DB operations.Also applies to: 22-28
src/server/implementation/mutations/index.ts (1)
112-112: LGTM: Config propagation enables trigger support.The changes correctly propagate the
configparameter tosignOutImpl,modifyAccountImpl, andinvalidateSessionsImpl, enabling these functions to create triggered contexts for DB operations.Also applies to: 149-149, 152-152
src/server/implementation/types.ts (3)
157-165: LGTM: Type-safe table name derivation.The
AuthTableNametype is correctly derived fromauthTablesusingkeyof, ensuring compile-time type safety and automatic updates when auth tables change.
167-194: LGTM: Flexible trigger signatures with optional reads.The trigger types use union types to allow omitting parameters, which enables the trigger system to skip unnecessary document reads by inspecting function arity. The
AnyCtxtype allows triggers to access custom tables beyond auth tables.Note: Using
anyforAnyCtxtrades type safety for flexibility. This is acceptable here since trigger implementations are user-provided and may need access to custom tables, but developers should be aware that type checking within trigger handlers is relaxed.
196-236: LGTM: Well-documented trigger configuration types.The
TableTriggersandAuthTriggerstypes are well-structured with clear documentation and usage examples. The mapped type approach ensures type safety across all auth tables.docs/pages/api_reference/server.mdx (2)
1579-1588: LGTM: Clear documentation for triggers configuration.The documentation clearly describes the
triggersfield, its type, and purpose. The link to usage examples in the advanced docs provides additional guidance.
2337-2375: LGTM: Comprehensive type documentation.The documentation for
AuthTableNameandAuthTriggersis clear and includes both type signatures and source file references, making it easy for developers to understand and use these types.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
docs/pages/advanced.mdx (1)
156-200: Clear and practical usage examples.The code examples effectively demonstrate trigger configuration with proper type annotations and realistic use cases. The examples show both logging and database operations, covering common patterns developers would need.
Consider adding a brief
onDeleteexample for completeness, though the pattern is clear from theonCreateandonUpdateexamples.Optional: Add onDelete example
You could add an
onDeleteexample to one of the table configurations to demonstrate all three trigger types:authSessions: { onDelete: async (ctx, sessionId, doc) => { // Log session termination console.log(`Session ${sessionId} deleted`); }, },
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
CHANGELOG.mddocs/pages/advanced.mdx
✅ Files skipped from review due to trivial changes (1)
- CHANGELOG.md
🧰 Additional context used
🪛 LanguageTool
docs/pages/advanced.mdx
[style] ~152-~152: ‘new record’ might be wordy. Consider a shorter alternative.
Context: ...r types: - onCreate - Called after a new record is inserted - onUpdate - Called after...
(EN_WORDINESS_PREMIUM_NEW_RECORD)
🔇 Additional comments (3)
docs/pages/advanced.mdx (3)
137-154: Well-structured introduction to triggers.The introduction clearly explains the purpose of triggers, provides relevant use cases, and correctly notes that triggers run atomically with auth operations. The trigger types are documented with appropriate descriptions.
Note: The onDelete timing issue (line 154) has already been flagged in a previous review comment.
202-217: Type safety documentation is accurate and helpful.The exported types are clearly documented with their signatures, making it easy for developers to understand the type contracts. The signatures correctly match the usage examples shown earlier in the document.
219-226: Important Notes section covers key behaviors.The notes appropriately document transaction semantics, error handling, context capabilities, and selective trigger firing. These are essential points for developers implementing triggers.
Note: Missing documentation about function.length optimization and nested trigger behavior has already been flagged in a previous review comment.
The trigger is called before deletion so the document can still be read for archiving/logging purposes.
- Explain that triggers must explicitly declare parameters to receive them - Warn about default parameters and destructuring breaking detection - Document that triggers use original ctx so nested triggers don't fire
- Read doc before delete (if needed by trigger) - Perform the delete - Fire the trigger after successful deletion This ensures: - Consistent with onCreate/onUpdate (all fire after the operation) - Trigger only runs if delete succeeds - Doc snapshot still available via pre-read
Pass triggered ctx to all helper functions so triggers fire for: - userOAuth: upsertUserAndAccount - createVerificationCode: getAccountOrThrow, getAuthSessionId, upsertUserAndAccount - verifyCodeAndSignIn: rate limit functions, session functions
- Rename to _rawCtx to signal "don't use directly" - Add no-restricted-syntax rule to error on _rawCtx.* member access - This prevents accidentally bypassing triggers
- Move createTriggeredCtx into mutations/index.ts as private function - storeImpl now wraps context once before dispatching to implementations - Remove individual wrapping from all mutation implementations - Delete triggeredDb.ts (functionality inlined) - Fix deleteSession to not double-wrap (receives already-wrapped ctx) This ensures all auth operations consistently use triggered context without implementations needing to know about triggers.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
src/server/implementation/mutations/index.ts (1)
96-103: Verify trigger receives non-wrapped context intentionally.The trigger receives the original
ctx(not the wrappedtriggeredDbcontext), which is correct for preventing infinite trigger loops. However, this means if a trigger performsctx.db.insert("users", ...), it won't fire anotheronCreatetrigger.This behavior is documented in the docs, but consider adding a brief inline comment here explaining why
ctxis used instead of a context withtriggeredDb.
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
.eslintrc.cjsdocs/pages/advanced.mdxsrc/server/implementation/mutations/index.tssrc/server/implementation/mutations/refreshSession.tssrc/server/implementation/sessions.ts
🚧 Files skipped from review as they are similar to previous changes (2)
- src/server/implementation/mutations/refreshSession.ts
- src/server/implementation/sessions.ts
🧰 Additional context used
🧬 Code graph analysis (1)
src/server/implementation/mutations/index.ts (3)
src/server/implementation/types.ts (2)
AuthTableName(165-165)MutationCtx(138-138)src/server/types.ts (1)
ConvexAuthConfig(23-256)src/server/implementation/mutations/signOut.ts (1)
signOutImpl(11-24)
🪛 LanguageTool
docs/pages/advanced.mdx
[style] ~152-~152: ‘new record’ might be wordy. Consider a shorter alternative.
Context: ...r types: - onCreate - Called after a new record is inserted - onUpdate - Called after...
(EN_WORDINESS_PREMIUM_NEW_RECORD)
🔇 Additional comments (6)
docs/pages/advanced.mdx (1)
137-246: LGTM! Comprehensive trigger documentation.The documentation clearly explains the triggers feature with accurate descriptions:
- Trigger types correctly document timing (onCreate after insert, onUpdate after update, onDelete after deletion with pre-deletion snapshot)
- Function parameter optimization and nested trigger behavior are well documented
- Code example demonstrates all trigger types with realistic use cases
src/server/implementation/mutations/index.ts (4)
82-89: LGTM! Table detection is correct.Using
normalizeIdto detect the table for an ID is the correct Convex pattern. The linear scan over 7 tables is negligible.
108-148: LGTM! Patch and replace handlers correctly implement trigger optimization.The
function.lengthoptimization correctly avoids unnecessary DB reads:
oldDocfetched only whentrigger.length >= 3newDocfetched only whentrigger.length >= 2The duplication between
patchandreplaceis acceptable given their subtle differences.
150-159: LGTM! Delete handler correctly fires trigger after deletion.The implementation correctly:
- Reads the document before deletion (if trigger needs it)
- Performs the delete
- Fires the trigger post-deletion with the pre-deletion snapshot
This aligns with the documented behavior.
228-287: LGTM! Clean centralized context wrapping.The refactoring correctly:
- Wraps the context once at the entry point
- Passes the triggered context to all mutation implementations
- Uses
_rawCtxnaming to signal it shouldn't be used directly (enforced by ESLint).eslintrc.cjs (1)
48-56: LGTM! Good defensive ESLint rule.The
no-restricted-syntaxrule with theMemberExpression[object.name='_rawCtx']selector will catch any property access on_rawCtx, preventing accidental bypass of the triggered context.This is a solid guard to ensure all auth operations consistently fire triggers.
Clarify why triggers receive the original ctx instead of triggeredDb, preventing infinite loops when triggers write to auth tables.
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Nitpick comments (1)
src/server/implementation/mutations/index.ts (1)
82-89: Consider caching table lookups for repeated operations.The linear search through AUTH_TABLES is acceptable given the small array size (7 items), but if performance becomes a concern, consider using a Map for O(1) lookups or caching the table name on the first detection.
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
src/server/implementation/mutations/index.ts
🔇 Additional comments (6)
src/server/implementation/mutations/index.ts (6)
1-2: LGTM!The new imports appropriately support the trigger functionality.
Also applies to: 35-35
56-64: LGTM!The AUTH_TABLES definition is clear and comprehensive for auth table trigger support.
71-78: LGTM!The early return optimization when no triggers are configured is efficient and appropriate.
164-168: LGTM!The passthrough methods for read operations (get, query, normalizeId) and the system property are correctly implemented. Using
.bind(rawDb)ensures the correctthiscontext is maintained.
231-238: LGTM! Clean wrapper pattern implementation.The single wrapping point at the entry of
storeImplensures all mutation implementations consistently receive the triggered context. The_rawCtxnaming convention clearly indicates the unwrapped context, and the comment effectively documents the design decision.
94-168: The triggered db wrapper is complete for Convex 1.17.0 MutationCtx.db API.The implemented wrapper correctly covers the full GenericDatabaseWriter surface:
insert,patch,replace,delete,get,query,normalizeId, andsystem. There are no additional mutation methods likedb.table()on MutationCtx.db in Convex 1.17.0 (thetable()method belongs to GenericDatabaseWriterWithTable, which is not used in this context). All write operations that should trigger callbacks are properly wrapped, and read-only operations are appropriately delegated.Likely an incorrect or invalid review comment.
Add a
triggersconfiguration option toconvexAuth()that allows registering callbacks for auth table modifications:This mirrors the design/functionality implemented for Convex Better Auth
Triggers are supported for all tables defined in authTables.
The AuthTriggers type is derived from authTables for type safety, ensuring table names and document types are enforced at compile time.
Triggers run within the same transaction as the auth operation, so errors will cause the operation to roll back.
Use cases include audit logging, history tracking, and custom side effects when auth data changes.
test: add trigger tests for users and authAccounts tables
By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.
Summary by CodeRabbit
New Features
Documentation
Tests
Chores
✏️ Tip: You can customize this high-level summary in your review settings.