Skip to content

Conversation

@Tobbe
Copy link
Member

@Tobbe Tobbe commented Dec 27, 2025

It's scary working on the CLI because it doesn't have any types. It's one of the oldest parts of the code base and it's still written in JavaScript. I'm going to start working on adding types a few files at a time

@netlify
Copy link

netlify bot commented Dec 27, 2025

Deploy Preview for cedarjs canceled.

Name Link
🔨 Latest commit 1b538b1
🔍 Latest deploy log https://app.netlify.com/projects/cedarjs/deploys/695000fdd4e91900084d5c56

@github-actions github-actions bot added this to the chore milestone Dec 27, 2025
@greptile-apps
Copy link
Contributor

greptile-apps bot commented Dec 27, 2025

Greptile Summary

This PR converts several CLI JavaScript files to TypeScript as part of an incremental migration to improve type safety in the CLI codebase.

Key Changes

  • Core files converted: ports.js, locking.js, updateCheck.js renamed to .ts with proper type annotations
  • Command files converted: dev.js and devHandler.js converted to TypeScript with interface definitions (DevHandlerOptions)
  • Test files updated: All associated test files (5 files) converted from .js to .ts with improved mock typing
  • Config improvements: DEFAULT_CONFIG exported from project-config and debugPort type expanded to support boolean
  • Documentation: Added TypeScript style guide to GEMINI.md mandating @ts-expect-error over @ts-ignore

Type Safety Improvements

  • Added proper function signatures with parameter and return types
  • Introduced interfaces for options objects (DevHandlerOptions, UpdateData)
  • Improved error handling with proper type narrowing (e instanceof Object && 'message' in e)
  • Test mocks properly typed using vi.mocked() and generic type parameters

Temporary Workarounds

Several @ts-expect-error comments were added for JS files that haven't been converted yet (colors, exit, generatePrismaClient, getPaths, project, background). These will be addressed in future conversion PRs.

Confidence Score: 5/5

  • This PR is safe to merge with minimal risk
  • Score reflects a well-executed JS-to-TS conversion with no logic changes, comprehensive test coverage updates, proper type annotations, and adherence to project style guidelines. All test files were updated alongside source files, error handling was improved, and the changes are purely additive in terms of type safety
  • No files require special attention

Important Files Changed

Filename Overview
packages/cli/src/lib/ports.ts Converted from JS to TS with proper type annotations and simplified error handling
packages/cli/src/lib/locking.ts Converted from JS to TS with type annotations, improved error handling
packages/cli/src/lib/updateCheck.ts Converted from JS to TS with UpdateData interface and improved error handling
packages/cli/src/commands/dev.ts Converted from JS to TS, added Argv type from yargs and proper array configuration
packages/cli/src/commands/devHandler.ts Converted from JS to TS with DevHandlerOptions interface, improved error handling but has @ts-expect-error on line 79
packages/project-config/src/config.ts Exported DEFAULT_CONFIG and updated debugPort type to allow boolean values

Sequence Diagram

sequenceDiagram
    participant User
    participant CLI as CLI (dev.ts)
    participant Handler as devHandler.ts
    participant Config as project-config
    participant Ports as ports.ts
    participant Prisma as generatePrismaClient
    participant Concurrent as concurrently

    User->>CLI: yarn cedar dev
    CLI->>Handler: handler(options)
    
    Handler->>Config: getConfig()
    Config-->>Handler: config (port numbers, etc)
    
    Handler->>Ports: getFreePort(apiPreferredPort)
    Ports-->>Handler: apiAvailablePort
    
    Handler->>Ports: getFreePort(webPreferredPort, excludePorts)
    Ports-->>Handler: webAvailablePort
    
    alt ports changed
        Handler->>User: exitWithError (port conflict)
    end
    
    Handler->>Prisma: generatePrismaClient()
    Prisma-->>Handler: success/error
    
    Handler->>Handler: shutdownPort(apiAvailablePort)
    Handler->>Handler: shutdownPort(webAvailablePort)
    
    Handler->>Config: getConfigPath()
    Config-->>Handler: redwoodConfigPath
    
    Handler->>Handler: Build jobs config (api, web, gen)
    
    Handler->>Concurrent: concurrently(jobs)
    Concurrent-->>User: Start dev servers
Loading

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.

12 files reviewed, 1 comment

Edit Code Review Agent Settings | Greptile

@nx-cloud
Copy link

nx-cloud bot commented Dec 27, 2025

🤖 Nx Cloud AI Fix

Ensure the fix-ci command is configured to always run in your CI pipeline to get automatic fixes in future runs. For more information, please see https://nx.dev/ci/features/self-healing-ci


View your CI Pipeline Execution ↗ for commit 1b538b1

Command Status Duration Result
nx run-many -t build:pack --exclude create-ceda... ✅ Succeeded 2s View ↗
nx run-many -t build ✅ Succeeded 9s View ↗
nx run-many -t test --minWorkers=1 --maxWorkers=4 ✅ Succeeded 1m 41s View ↗
nx run-many -t test:types ✅ Succeeded 9s View ↗

☁️ Nx Cloud last updated this comment at 2025-12-27 16:16:28 UTC

@nx-cloud
Copy link

nx-cloud bot commented Dec 27, 2025

🤖 Nx Cloud AI Fix

Ensure the fix-ci command is configured to always run in your CI pipeline to get automatic fixes in future runs. For more information, please see https://nx.dev/ci/features/self-healing-ci


View your CI Pipeline Execution ↗ for commit 1b538b1

Command Status Duration Result
nx run-many -t build ✅ Succeeded 5s View ↗

☁️ Nx Cloud last updated this comment at 2025-12-27 15:56:40 UTC

@Tobbe Tobbe force-pushed the tobbe-cli-ts-part-I branch from de43992 to 1b538b1 Compare December 27, 2025 15:53
@Tobbe Tobbe merged commit 299ba78 into main Dec 27, 2025
42 checks passed
@Tobbe Tobbe deleted the tobbe-cli-ts-part-I branch December 27, 2025 20:08
@Tobbe Tobbe modified the milestones: chore, v2.2.1 Dec 29, 2025
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