fix(better-auth): keep schema-generator import lazy in CJS output#2658
Conversation
) When the index entry and schema-generator entry were built in the same tsdown invocation, Rolldown injected a top-level `require("./schema-generator.cjs")` into `index.cjs`. This eagerly loaded `@zenstackhq/language` (Langium) at adapter construction time, which fails on Node CJS hosts because Langium's package.json has no `exports` main, producing ERR_PACKAGE_PATH_NOT_EXPORTED. Split the build into two tsdown invocations and import schema-generator via the package self-subpath (marked `deps.neverBundle`) so the dynamic import stays truly lazy in both CJS and ESM output. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
|
ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
✅ Files skipped from review due to trivial changes (1)
📝 WalkthroughWalkthroughThe Better-Auth adapter build configuration is split into two separate entries: one for the main ChangesSchema Generator Lazy-Loading
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Possibly related issues
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 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 for PR #2658Reviewing the fix for better-auth eager Langium loading issue. Todo List:
Review SummaryThis is a well-executed fix for a complex CJS module loading issue. The PR successfully addresses the root cause of eager Langium imports in CommonJS environments. Code Quality & Implementation ✅Excellent approach: The solution correctly identifies that the bundling strategy was causing sibling chunk registration, leading to unintended eager imports. The fix is surgical and well-reasoned:
Technical Soundness ✅Dynamic import strategy: The change from
Performance Considerations ✅Improved lazy loading: The fix actually enhances performance by:
Security Concerns ✅No security implications: The changes are purely build/bundling related with no security impact. Potential Issues & SuggestionsMinor: Build Configuration ComplexityThe split build approach, while necessary, adds complexity. Consider:
Minor: Error Handling
try {
const generateSchema = (await import('@zenstackhq/better-auth/schema-generator')).generateSchema;
return generateSchema(file, tables, config, options);
} catch (error) {
throw new BetterAuthError('Failed to load schema generator', { cause: error });
}Test Coverage ✅Adequate testing: The PR mentions:
Architecture Impact ✅Future-proof solution: The approach:
Overall AssessmentExcellent fix that demonstrates deep understanding of:
The solution is minimal, targeted, and well-documented. The split build approach is unusual but justified and necessary for this specific use case. Recommendation: ✅ Approve - This is a high-quality fix that solves a real compatibility issue without compromising functionality. |
The package self-import `@zenstackhq/better-auth/schema-generator` resolves through the published `exports` map to `./dist/schema-generator.d.mts`, which doesn't exist on a clean CI build (tsc --noEmit runs before tsdown). Add a tsconfig `paths` mapping so TypeScript resolves the type lookup against the local source file at typecheck time. Runtime resolution remains unchanged. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
|
Claude finished @ymc9's task —— View job Code Review for PR #2658Reviewing the fix for better-auth eager Langium loading issue. Todo List:
Review SummaryThis is a well-executed fix for a complex CJS module loading issue. The PR successfully addresses the root cause of eager Langium imports in CommonJS environments. Code Quality & Implementation ✅Excellent approach: The solution correctly identifies that the bundling strategy was causing sibling chunk registration, leading to unintended eager imports. The fix is surgical and well-reasoned:
Technical Soundness ✅Dynamic import strategy: The change from
Performance Considerations ✅Improved lazy loading: The fix actually enhances performance by:
Security Concerns ✅No security implications: The changes are purely build/bundling related with no security impact. Potential Issues & SuggestionsMinor: Build Configuration ComplexityThe split build approach, while necessary, adds complexity. Consider:
Minor: Error Handling
try {
const generateSchema = (await import('@zenstackhq/better-auth/schema-generator')).generateSchema;
return generateSchema(file, tables, config, options);
} catch (error) {
throw new BetterAuthError('Failed to load schema generator', { cause: error });
}Test Coverage ✅Adequate testing: The PR mentions:
Architecture Impact ✅Future-proof solution: The approach:
Overall AssessmentExcellent fix that demonstrates deep understanding of:
The solution is minimal, targeted, and well-documented. The split build approach is unusual but justified and necessary for this specific use case. Recommendation: ✅ Approve - This is a high-quality fix that solves a real compatibility issue without compromising functionality. |
Summary
ERR_PACKAGE_PATH_NOT_EXPORTEDforlangium/package.jsonon CJS apps (e.g. NestJS) when constructing the better-auth adapter.index.cjshad a top-levelrequire("./schema-generator.cjs")that eagerly pulled@zenstackhq/language(Langium) at adapter load time. Langium'spackage.jsonlacks anexportsmain, so resolution from CJS failed.await import('./schema-generator')insidecreateSchemawas supposed to keep that branch lazy, but Rolldown was hoisting a sibling-chunk register into the file top because both entries were built in a single tsdown invocation.Fix
tsdown.config.tsinto two separate tsdown invocations (one per entry) so Rolldown no longer treats them as siblings and stops injecting the eager top-level require.@zenstackhq/better-auth/schema-generator) and added it todeps.neverBundleso Rolldown leaves it as an external reference resolved by Node at first call.After the change,
dist/index.cjsno longer has any top-level reference toschema-generator; only the lazyawait import("@zenstackhq/better-auth/schema-generator")insidecreateSchemaremains. Langium is therefore only loaded when schema generation is actually invoked (e.g. during@better-auth/cli generate), not on adapter construction.Test plan
pnpm testinpackages/auth-adapters/better-authpasses (6/6, includingcli-generate.test.tswhich exercises the dynamic import path)dist/index.cjshas no top-levelrequire("./schema-generator.cjs")after builddist/index.mjsstill uses lazyawait import(...)only insidecreateSchema🤖 Generated with Claude Code
Summary by CodeRabbit