Conversation
📝 WalkthroughWalkthroughThis PR bumps the package version to 3.6.3 across 24 packages, exposes a new Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~15 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 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 PR Review: v3.6.3 Release (dev → main)I've completed my review of this v3.6.3 release PR. Here's my comprehensive feedback: ✅ Code Quality & Best PracticesVersion Management
Performance ImprovementsThe major change in
🔍 Code AnalysisBetter Auth Adapter (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
packages/plugins/policy/src/policy-handler.ts (1)
208-241:⚠️ Potential issue | 🟡 MinorUse
buildIsFalsefor consistency with the NULL-handling pattern established at line 777.The negation of
fieldLevelFilteruseslogicalNot, which emitsNOT fieldLevelFilter(treating NULL as NULL in the WHERE clause). Elsewhere in this file (lines 777, 1168), policy expressions are negated usingbuildIsFalse, which explicitly handles NULL viaCOALESCE(expr, FALSE) = FALSE. Consider using the same helper here for consistency:Suggested change
- new ExpressionWrapper( - conjunction(this.dialect, [updateFilter, logicalNot(this.dialect, fieldLevelFilter)]), - ), + new ExpressionWrapper( + conjunction(this.dialect, [updateFilter, buildIsFalse(fieldLevelFilter, this.dialect)]), + ),Also, line 234 accesses
preUpdateResult.rows[0].$conditionwithout optional chaining, while the analogous line 952 usesresult.rows[0]?.$condition— minor consistency nit.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/plugins/policy/src/policy-handler.ts` around lines 208 - 241, Replace the logicalNot usage so the negation of fieldLevelFilter uses the existing NULL-safe helper buildIsFalse (i.e., use buildIsFalse(this.dialect, fieldLevelFilter) inside the conjunction for violatingRowsQuery) to match the NULL-handling pattern used elsewhere; also access the select result defensively by changing preUpdateResult.rows[0].$condition to preUpdateResult.rows[0]?.$condition to match the optional-chaining style used in other checks.
🧹 Nitpick comments (1)
packages/auth-adapters/better-auth/src/adapter.ts (1)
210-213: Lazy load looks correct; minor readability nit.Dynamic import of
./schema-generatordefers the heavy schema-generation deps (e.g.,@zenstackhq/language,fs) untilcreateSchemaactually runs, which is the right call. The relative path will resolve to the siblingdist/schema-generator.mjschunk produced by the new tsdown entry. Optional readability tweak:Cleaner destructuring
- const generateSchema = (await import('./schema-generator')).generateSchema; - return generateSchema(file, tables, config, options); + const { generateSchema } = await import('./schema-generator'); + return generateSchema(file, tables, config, options);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/auth-adapters/better-auth/src/adapter.ts` around lines 210 - 213, The createSchema implementation should use cleaner destructuring when lazy-importing the module: change the dynamic import inside createSchema to destructure generateSchema from the imported module (import('./schema-generator')) and then call generateSchema(file, tables, config, options); this keeps the lazy-load behavior but improves readability around createSchema, generateSchema, and the parameters file, tables, config, options.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Outside diff comments:
In `@packages/plugins/policy/src/policy-handler.ts`:
- Around line 208-241: Replace the logicalNot usage so the negation of
fieldLevelFilter uses the existing NULL-safe helper buildIsFalse (i.e., use
buildIsFalse(this.dialect, fieldLevelFilter) inside the conjunction for
violatingRowsQuery) to match the NULL-handling pattern used elsewhere; also
access the select result defensively by changing
preUpdateResult.rows[0].$condition to preUpdateResult.rows[0]?.$condition to
match the optional-chaining style used in other checks.
---
Nitpick comments:
In `@packages/auth-adapters/better-auth/src/adapter.ts`:
- Around line 210-213: The createSchema implementation should use cleaner
destructuring when lazy-importing the module: change the dynamic import inside
createSchema to destructure generateSchema from the imported module
(import('./schema-generator')) and then call generateSchema(file, tables,
config, options); this keeps the lazy-load behavior but improves readability
around createSchema, generateSchema, and the parameters file, tables, config,
options.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 6642996d-c66b-4372-92c9-25c4360addb6
📒 Files selected for processing (28)
package.jsonpackages/auth-adapters/better-auth/package.jsonpackages/auth-adapters/better-auth/src/adapter.tspackages/auth-adapters/better-auth/tsdown.config.tspackages/cli/package.jsonpackages/clients/client-helpers/package.jsonpackages/clients/tanstack-query/package.jsonpackages/common-helpers/package.jsonpackages/config/eslint-config/package.jsonpackages/config/tsdown-config/package.jsonpackages/config/typescript-config/package.jsonpackages/config/vitest-config/package.jsonpackages/create-zenstack/package.jsonpackages/ide/vscode/package.jsonpackages/language/package.jsonpackages/orm/package.jsonpackages/plugins/policy/package.jsonpackages/plugins/policy/src/policy-handler.tspackages/schema/package.jsonpackages/sdk/package.jsonpackages/server/package.jsonpackages/testtools/package.jsonpackages/zod/package.jsonsamples/orm/package.jsontests/e2e/package.jsontests/regression/package.jsontests/runtimes/bun/package.jsontests/runtimes/edge-runtime/package.json
1 participant
Summary by CodeRabbit
Chores
New Features
Bug Fixes