feat(cli): auto-detect import file extension from tsconfig#2705
Conversation
The TypeScript generator's `importWithFileExtension` option had to be set manually for projects using native ESM module resolution (`node16`/`nodenext`), which requires explicit `.js` extensions on relative imports. Without it, generated code failed to type-check in those projects. When the option is not explicitly declared, the TypeScript plugin now inspects the nearest `tsconfig.json` to the output directory and emits a `.js` extension for `node16`/`nodenext` resolution, while keeping the idiomatic extensionless form for `bundler`/`node` resolution. An explicit option still takes precedence. Also include `test/**/*.ts` in the CLI package's tsconfig (matching the zod/testtools convention) so test files type-check, fixing a handful of pre-existing strict-mode errors surfaced by that change. Fixes #2686 Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (8)
📝 WalkthroughWalkthroughThe PR implements auto-detection of TypeScript module resolution settings to determine whether generated schema imports should include ChangesTypeScript Import Extension Auto-Detection
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 ESLint
ESLint install timed out. The project may have too many dependencies for the sandbox. 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 Auto-Detect Import File Extension FeatureTodo List
Review SummaryThis is a well-implemented feature that solves a real pain point for ESM projects. The implementation is thoughtful, thorough, and follows good engineering practices. ✅ Strengths1. Correct TypeScript Resolution Logic (
2. Clean Integration (
3. Comprehensive Test Coverage (
4. Backward Compatibility
🔍 Code Quality ObservationsPerformance ✅
Security ✅
Error Handling ✅
🎯 Technical AccuracyThe logic correctly implements TypeScript's module resolution behavior:
📋 Test QualityThe tests are exemplary:
🛡️ Minor Considerations1. TypeScript Version Compatibility
2. Tsconfig Location
3. Future Module Kinds
🎉 Overall AssessmentThis PR demonstrates excellent engineering practices:
The auto-detection logic is technically sound and the implementation is production-ready. The tests provide high confidence that this works correctly across different project configurations. Recommendation: ✅ Approve - This is a high-quality implementation that will significantly improve the developer experience for ESM projects. |
Summary
Closes #2686.
Generated TypeScript schema files import from each other with relative specifiers (
./schema). Projects using native ESM module resolution (node16/nodenext) require an explicit.jsextension on such imports, otherwise the generated code fails to type-check. Until now this required manually setting theimportWithFileExtensionplugin option.This PR makes the TypeScript plugin auto-detect the right extension when the option is not explicitly declared:
tsconfig.jsonto the output directory (resolvingextendschains andmodule/moduleResolutiondefaults)..jsfornode16/nodenextresolution (where it is required).bundler/noderesolution (the maximally-compatible form for those toolchains).importWithFileExtensionstill takes precedence; passing""disables the extension entirely.Why not just emit
.jsunconditionally?Extensionless imports only fail under native Node ESM — which is exactly the
node16/nodenextcase that detection already covers. For every other target (bundler/node), extensionless is the idiomatic form and avoids changing output for existing users. Auto-detection emits the form each project would hand-write, and is guaranteed safe regardless of how a downstream bundler resolves.js→.ts.Changes
packages/sdk/src/tsconfig-utils.ts(new) —detectImportFileExtension(dir)helper, exported from the SDK.packages/cli/src/plugins/typescript.ts— wire auto-detection in; only runs when the option isundefined.packages/cli/test/import-extension.test.ts(new) — end-to-end generate-then-tsctests covering bundler (no extension, compiles), nodenext (auto-adds.js, compiles), a negative control proving the extension is load-bearing under nodenext, and explicit-override precedence.packages/cli/tsconfig.json— includetest/**/*.ts(matching thezod/testtoolsconvention) so test files type-check; this surfaced and fixed a few pre-existing strict-mode errors ints-schema-gen,proxy, anddb/pulltests.Testing
pnpm --filter @zenstackhq/cli test import-extension— 4 passing e2e tests (realzenstack generate+tsc --noEmit).tsc --noEmit -p packages/cli/tsconfig.json— clean, now including tests.pnpm --filter @zenstackhq/sdk buildandpnpm --filter @zenstackhq/cli build— clean.🤖 Generated with Claude Code
Summary by CodeRabbit
New Features
Improvements
Tests