fix(better-auth): lazy-load schema-generator and expose as separate entry point#2611
fix(better-auth): lazy-load schema-generator and expose as separate entry point#2611
Conversation
…entry point Converts the static import of schema-generator to a dynamic import to avoid loading heavy ZModel parsing dependencies until createSchema is actually called. Also exposes schema-generator as a separate package entry point for direct use. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
|
Claude finished @ymc9's task —— View job PR Review: Lazy-load schema-generator ImplementationI've completed a thorough review of this pull request that implements lazy loading for the schema-generator to improve bundle size and performance. Review Tasks:
✅ Overall AssessmentThis is a well-implemented performance optimization that follows good practices for lazy loading. The changes are minimal, focused, and should deliver the intended benefits. 📋 Code Quality & Implementation✅ Strengths:
📝 Minor Observations:
🚀 Performance Considerations✅ Benefits:
🔒 Security Concerns✅ No security issues identified:
🧪 Test Coverage Assessment📋 Current Test Coverage:
💡 Recommendations for Enhanced Testing:
// Suggested test cases:
describe('Lazy loading', () => {
it('should not load schema-generator until createSchema is called', () => {
// Test that importing adapter doesn't pull in schema-generator
});
it('should resolve ./schema-generator export correctly', async () => {
const { generateSchema } = await import('@zenstackhq/better-auth/schema-generator');
expect(typeof generateSchema).toBe('function');
});
});🎯 Recommendations
📝 Implementation Details Review
const generateSchema = (await import('./schema-generator')).generateSchema;
return generateSchema(file, tables, config, options);✅ Correct pattern - preserves all parameters and return types
🏆 Final VerdictThis is an excellent performance optimization that follows TypeScript/Node.js best practices. The implementation is clean, safe, and should deliver meaningful bundle size improvements for consumers who don't use the schema generation features. Approved with confidence! 🚀 |
📝 WalkthroughWalkthroughThe better-auth package now exposes a new public entry point Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes 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 |
ymc9
changed the title
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (1)
packages/auth-adapters/better-auth/src/adapter.ts (1)
210-213: Dynamic import achieves the lazy-load goal; minor readability nit.The change correctly defers loading of the ZModel schema-parsing code to the first
createSchemacall. Node/bundlers cache the imported module, so subsequent invocations won't re-parse it. Relative path'./schema-generator'matches the new tsdown entry and the./schema-generatorsubpath export, so both ESM and CJS consumers will resolve the sibling bundle correctly.Optional: destructure at the await site for a touch more clarity.
♻️ Optional refactor
- createSchema: async ({ file, tables }) => { - const generateSchema = (await import('./schema-generator')).generateSchema; - return generateSchema(file, tables, config, options); - }, + createSchema: async ({ file, tables }) => { + 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 handler lazily imports './schema-generator' but could be clearer by destructuring the exported generateSchema at the await site; update the createSchema implementation (function createSchema) to await import('./schema-generator') and destructure { generateSchema } from the module before calling generateSchema(file, tables, config, options) so the code is more readable while preserving lazy-loading and caching behavior.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@packages/auth-adapters/better-auth/src/adapter.ts`:
- Around line 210-213: The createSchema handler lazily imports
'./schema-generator' but could be clearer by destructuring the exported
generateSchema at the await site; update the createSchema implementation
(function createSchema) to await import('./schema-generator') and destructure {
generateSchema } from the module before calling generateSchema(file, tables,
config, options) so the code is more readable while preserving lazy-loading and
caching behavior.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: aaf68211-b580-4a79-9897-516fc7283748
📒 Files selected for processing (3)
packages/auth-adapters/better-auth/package.jsonpackages/auth-adapters/better-auth/src/adapter.tspackages/auth-adapters/better-auth/tsdown.config.ts
1 participant
Summary
import { generateSchema }inadapter.tsto a dynamicimport()so the heavy ZModel schema-parsing code is only loaded whencreateSchemais actually invokedschema-generatoras a dedicated build entry intsdown.config.tsso it can be bundled and tree-shaken independently./schema-generatoras a new package export inpackage.jsonfor consumers who want to use it directlyTest plan
better-authadapter still works end-to-end withcreateSchemaflow./schema-generatorexport resolves correctly in both CJS and ESM environments🤖 Generated with Claude Code
Summary by CodeRabbit