Skip to content

Conversation

nbayindirli
Copy link
Contributor

@nbayindirli nbayindirli commented Sep 9, 2025

why?

  • in order to migrate the repo to TS, starting with evm

how?

  • by refactoring evm/cli-utils (and common/cli-utils dependency)

Greptile Summary

Updated On: 2025-09-09 04:17:55 UTC

This PR refactors the CLI utilities from JavaScript to TypeScript as part of a broader repository migration strategy, starting with the EVM module. The changes convert two critical CLI utility files (common/cli-utils.js and evm/cli-utils.js) to TypeScript equivalents with proper type annotations and modern ES6 module syntax.

The refactoring introduces a new BaseOptions interface in common/cli-utils.ts that provides structured typing for configuration options shared across multiple modules. The EVM-specific utilities extend this with an EvmOptions interface that includes optional properties like artifactPath, contractName, deployMethod, salt, skipExisting, upgrade, and predictOnly. All existing functionality is preserved while adding compile-time type checking and better IDE support.

Key architectural changes include converting CommonJS require() statements to ES6 imports, replacing module.exports with named exports, and adding comprehensive TypeScript type annotations to function parameters and return values. The migration maintains the same API surface while modernizing the codebase foundation for future TypeScript adoption across other modules like CosmWasm, Stellar, and Sui.

Important Files Changed

Changed Files
Filename Score Overview
common/cli-utils.ts 3/5 Converts core CLI utilities from JS to TS with new BaseOptions interface, but introduces potential breaking changes for existing CommonJS imports
evm/cli-utils.ts 5/5 Clean TypeScript conversion of EVM CLI utilities with proper type annotations and EvmOptions interface extending BaseOptions

Confidence score: 3/5

  • This PR introduces potential breaking changes for existing JavaScript modules that import these utilities using CommonJS syntax
  • Score reflects the risk of import compatibility issues across the codebase during the migration transition period
  • Pay close attention to common/cli-utils.ts as it's a shared dependency that could break other modules still using require() statements

Sequence Diagram

sequenceDiagram
    participant User
    participant CLI as "CLI Program"
    participant CommonUtils as "common/cli-utils"
    participant EvmUtils as "evm/cli-utils"
    participant FileSystem as "File System"
    participant Commander as "Commander Library"

    User->>CLI: "Execute CLI command with options"
    CLI->>EvmUtils: "addEvmOptions(program, options)"
    EvmUtils->>CommonUtils: "addBaseOptions(program, options)"
    CommonUtils->>FileSystem: "fs.readdirSync(CHAIN_CONFIG_PATH)"
    FileSystem-->>CommonUtils: "chain environment files"
    CommonUtils->>Commander: "new Option() for env, privateKey, chainNames"
    Commander-->>CommonUtils: "configured options"
    CommonUtils->>CLI: "program with base options"
    EvmUtils->>Commander: "new Option() for verify, artifactPath, contractName"
    Commander-->>EvmUtils: "configured EVM options"
    EvmUtils-->>CLI: "program with all options"
    CLI->>CommonUtils: "addStoreOptions(program)"
    CommonUtils->>Commander: "new Option() for artifact-dir, version"
    Commander-->>CommonUtils: "store options"
    CommonUtils->>CLI: "hook('preAction', validation)"
    CLI-->>User: "CLI ready with all options configured"
Loading

@nbayindirli nbayindirli requested a review from a team as a code owner September 9, 2025 04:16
Copy link
Contributor

@greptile-apps greptile-apps bot left a comment

Choose a reason for hiding this comment

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

2 files reviewed, no comments

Edit Code Review Bot Settings | Greptile

cursor[bot]

This comment was marked as outdated.

@nbayindirli nbayindirli requested a review from a team as a code owner September 25, 2025 18:19
Copy link
Contributor

@kulikthebird kulikthebird left a comment

Choose a reason for hiding this comment

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

Looks good, thanks for the next TS refactoring

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.

4 participants