Skip to content

Conversation

@ymc9
Copy link
Member

@ymc9 ymc9 commented Nov 15, 2025

fixes #378

Summary by CodeRabbit

  • New Features
    • Added support for customizable import file extensions in generated TypeScript schemas.
    • Enhanced CLI options handling to propagate global settings to per-plugin configurations for improved schema generation control.

Copilot AI review requested due to automatic review settings November 15, 2025 22:36
@coderabbitai
Copy link

coderabbitai bot commented Nov 15, 2025

Walkthrough

Enables CLI-level options (lite, liteOnly) to merge into per-plugin options for @core/typescript provider, and introduces a new importWithFileExtension parameter to customize TypeScript schema import paths during generation.

Changes

Cohort / File(s) Summary
CLI option merging
packages/cli/src/actions/generate.ts
When @core/typescript plugin is found, CLI-level lite and liteOnly options are now injected into per-plugin options if not already defined, ensuring CLI flags take effect.
Plugin option propagation
packages/cli/src/plugins/typescript.ts
Added optional importWithFileExtension string parameter; validated and passed through to TsSchemaGenerator.generate.
Core schema generation
packages/sdk/src/ts-schema-generator.ts
Extended TsSchemaGeneratorOptions with optional importWithFileExtension field; updated generateSchemaStatements, generateModelsAndTypeDefs, generateInputTypes, and generateSchemaImport to accept and utilize this option for customizing import paths.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

  • Option merging logic in generate.ts requires validation to ensure CLI flags properly override/supplement plugin-level options
  • Type signature changes and parameter propagation across three files need consistency verification
  • generateSchemaImport extension logic should be reviewed for edge cases and path construction correctness

Possibly related PRs

Poem

🐰 Hops with glee through CLI flags,
Lite and liteOnly now shine bright,
Extensions dance in import paths,
Options merge left and right,
Schema generation takes flight!

Pre-merge checks and finishing touches

❌ Failed checks (1 warning, 1 inconclusive)
Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. You can run @coderabbitai generate docstrings to improve docstring coverage.
Out of Scope Changes check ❓ Inconclusive The PR introduces importWithFileExtension support which is not explicitly mentioned in issue #378, potentially representing scope creep beyond the --lite flag propagation requirement. Clarify whether importWithFileExtension is a necessary implementation detail for #378 or a separate feature that should be addressed in a different PR.
✅ Passed checks (3 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The PR title clearly describes the main feature: allowing file extension imports in generated schemas, which aligns with the changes across all three modified files.
Linked Issues check ✅ Passed The PR addresses issue #378 by ensuring CLI flags --lite and --lite-only are properly propagated to the @core/typescript plugin via augmented pluginOptions with default values.
✨ Finishing touches
  • 📝 Generate docstrings
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch feat/ts-schema-import-extension

📜 Recent review details

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 70b970f and 08d787b.

📒 Files selected for processing (3)
  • packages/cli/src/actions/generate.ts (1 hunks)
  • packages/cli/src/plugins/typescript.ts (1 hunks)
  • packages/sdk/src/ts-schema-generator.ts (6 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
packages/cli/src/plugins/typescript.ts (1)
packages/sdk/src/ts-schema-generator.ts (1)
  • TsSchemaGenerator (61-1630)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
  • GitHub Check: Agent
  • GitHub Check: build-test (20.x, postgresql)
  • GitHub Check: build-test (20.x, sqlite)
🔇 Additional comments (6)
packages/cli/src/actions/generate.ts (1)

89-101: LGTM! CLI option merging correctly implemented.

The logic properly merges CLI-level lite and liteOnly options into plugin options for the @core/typescript provider when they are not already defined. The === undefined checks correctly allow explicit false values in plugin options to override CLI flags, which is the expected behavior.

This implementation directly addresses issue #378 by ensuring CLI flags are effective even when the TypeScript plugin is explicitly defined.

packages/cli/src/plugins/typescript.ts (1)

25-36: LGTM! Clean validation and pass-through for new option.

The validation correctly ensures that importWithFileExtension, if provided, must be a string. The error message is clear, and the type assertion is safe after the validation check. The option is properly threaded through to the schema generator.

packages/sdk/src/ts-schema-generator.ts (4)

58-58: LGTM! Clean type extension.

The new optional importWithFileExtension option is properly added to TsSchemaGeneratorOptions.


1295-1303: LGTM! Consistent option propagation.

The importWithFileExtension option is correctly passed through to generateSchemaImport when generating model and type definition imports.


1429-1474: LGTM! Robust implementation of file extension handling.

The implementation correctly:

  • Updates the method signature to accept importWithFileExtension
  • Constructs the base import path based on useLite flag
  • Handles both formats: extensions with (.js) and without (js) leading dots
  • Generates the final import declaration with the computed path

The defensive handling of the leading dot ensures flexibility for users while maintaining correctness.


1493-1501: LGTM! Consistent option propagation.

The importWithFileExtension option is correctly passed through to generateSchemaImport when generating input type imports, maintaining consistency with the model/typedef generation flow.


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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@claude
Copy link

claude bot commented Nov 15, 2025

Claude finished @ymc9's task —— View job


Code Review Complete ✅

Reviewed PR #412 - feat(cli): allow importing with file extension in generated schema

Review Tasks:

  • Read and analyze changed files
  • Check code quality and best practices
  • Identify potential bugs or issues
  • Evaluate performance considerations
  • Review security concerns
  • Assess test coverage

Summary

This PR adds support for importing generated schema files with file extensions (e.g., .js) through a new importWithFileExtension plugin option. The implementation is clean and follows the existing patterns in the codebase.

✅ Code Quality & Best Practices

  • Well-structured: Changes follow existing patterns and conventions in all three files
  • Type-safe: Proper TypeScript types and validation for the new option
  • Clear interface: Simple string option that's easy to understand and use
  • Consistent naming: Uses importWithFileExtension consistently across all files
  • Good error handling: Validates that the option is a string if provided

✅ Implementation Details

The changes span three key files:

packages/cli/src/actions/generate.ts:39 - Updates CLI help text to demonstrate usage
packages/cli/src/plugins/typescript.ts:26-29 - Adds validation and passes option to generator
packages/sdk/src/ts-schema-generator.ts:58, 1434, 1462-1466, 1499 - Core implementation that appends file extension to import paths

🟡 Areas for Improvement

  1. Test Coverage: No tests were added for this feature. Consider adding:

    • Unit tests for the file extension logic in TsSchemaGenerator
    • Integration tests to verify generated imports are correct
    • Edge case tests (empty string, special characters, etc.)
  2. Documentation: The help text shows usage but could be more descriptive about when/why to use this feature

  3. Validation Enhancement: Current validation only checks if it's a string. Consider:

    if (importWithFileExtension && (typeof importWithFileExtension !== 'string' || importWithFileExtension.trim() === '')) {
        throw new Error('The "importWithFileExtension" option must be a non-empty string if specified.');
    }

✅ Security & Performance

  • No security concerns: Simple string concatenation with user-provided input that's validated
  • Minimal performance impact: Only affects import path generation during build time
  • Safe defaults: Option is undefined by default, maintaining backward compatibility

🎯 Specific Code Comments

typescript.ts:26-29: Good validation, but consider checking for empty strings too.

ts-schema-generator.ts:1462-1466: Smart handling of extensions with/without leading dot:

importFrom += importWithFileExtension.startsWith('.')
    ? importWithFileExtension
    : `.${importWithFileExtension}`;

🏆 Overall Assessment

This is a solid, focused PR that addresses a specific need (fixing #378). The implementation is clean, follows existing patterns, and maintains backward compatibility. The main area for improvement is adding test coverage to ensure the feature works correctly in various scenarios.

Recommendation: ✅ Approve with suggestions - The core implementation is good, but adding tests would strengthen confidence in the feature.


Copilot finished reviewing on behalf of ymc9 November 15, 2025 22:40
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull Request Overview

This PR adds the importWithFileExtension option to the TypeScript schema generator, allowing users to specify file extensions (e.g., .js, .mjs) for generated import statements. This is particularly useful for ESM projects that require explicit file extensions in import paths.

Key Changes:

  • Added importWithFileExtension option to TsSchemaGeneratorOptions type
  • Modified generateSchemaImport method to append the specified extension to relative imports
  • Added validation in the TypeScript plugin to ensure the option is a string when provided

Reviewed Changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.

File Description
packages/sdk/src/ts-schema-generator.ts Added importWithFileExtension option to the generator options and updated generateSchemaImport method to conditionally append file extensions to relative imports (./schema and ./schema-lite)
packages/cli/src/plugins/typescript.ts Added validation for the importWithFileExtension plugin option and passed it through to the TsSchemaGenerator
packages/cli/src/actions/generate.ts Refactored CLI option merging logic for the typescript plugin to be more explicit and maintainable (no functional changes related to the new feature)

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

@ymc9 ymc9 merged commit 160437a into dev Nov 15, 2025
12 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants