Skip to content

Conversation

@edouardmercier
Copy link

@edouardmercier edouardmercier commented Oct 1, 2025

  • The generated code does not use the PORT environment variable when the --port CLI is not defined and forces the port to be 3000, while the setupWebServer function port argument already has this default value: proposes to resort to the PORT environment variable in that case.
  • Proposes a new --generate-lib CLI option, which causes the generated main() function to be exported and removing the clean-up code generation, so as to be able to import and run than main() function is an existing code, just like a library.

Summary by CodeRabbit

  • New Features

    • Added a CLI option to generate library-style output that exposes (rather than auto-invokes) the main entrypoint.
  • Refactor

    • Standardized environment variable initialization for more consistent startup.
    • Implemented dynamic port resolution across transports to avoid hard-coded defaults.
    • Centralized startup/cleanup with graceful SIGINT/SIGTERM handling and clearer fatal-error handling.

@coderabbitai
Copy link
Contributor

coderabbitai bot commented Oct 1, 2025

Walkthrough

Introduces a generateLib option throughout CLI, types, and server code generation. Adds conditional export of main, centralized startup/cleanup with signal handling, dynamic port resolution for transports, and switches dotenv usage to named import with immediate config invocation.

Changes

Cohort / File(s) Summary of Changes
Server code generation & bootstrap
src/generator/server-code.ts
Adds conditional export of main based on options.generateLib; introduces a reusable mainDeclaration containing cleanup() and SIGINT/SIGTERM handlers; centralizes startup sequence and moves previous inline startup/cleanup into the block; replaces dotenv.config() with import { config } from 'dotenv'; config();; implements dynamic port resolution (webServicerPortNumberStatement) and uses it for both web and streamable-http transport setup calls.
CLI option parsing
src/index.ts
Adds -l, --generate-lib <boolean> CLI option (parsed to boolean via the existing normalizer) and wires it into parsed CliOptions.
Types update
src/types/index.ts
Adds optional generateLib?: boolean to CliOptions with documentation describing library vs. runnable generation.

Sequence Diagram(s)

sequenceDiagram
  autonumber
  participant U as User (CLI)
  participant C as CLI Parser
  participant G as Code Generator
  participant S as Generated Server
  participant OS as OS Signals

  U->>C: Run generator with options (incl. --generate-lib)
  C->>G: Parsed CliOptions { generateLib, port, ... }
  G->>S: Emit server code (named dotenv import + config(), port resolution, mainDeclaration)

  alt generateLib === false
    note over S: main() defined and invoked
    S->>S: Resolve port (options.port or default)  %% color: #DDEBF7
    S->>S: setupWebServer / setupStreamableHttpServer using resolved port
    S->>OS: Register SIGINT/SIGTERM handlers
    OS-->>S: Signal received
    S->>S: cleanup() then exit
  else generateLib === true
    note over S: main() exported, not invoked
    S-->>U: Library exposes main() for caller control
  end
Loading

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

Poem

I thump my paw at ports that hide,
A tidy main I now provide.
With signals caught and dotenv primed,
I hop through code that's neatly timed.
Library or run — your call, my friend. 🥕🐇

Pre-merge checks and finishing touches

✅ Passed checks (3 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title Check ✅ Passed The title clearly describes the two primary changes to the generator—correcting the PORT environment variable handling and introducing the --generate-lib CLI option—so it appropriately summarizes the main updates in a concise, topic-focused sentence.
Docstring Coverage ✅ Passed No functions found in the changes. Docstring coverage check skipped.
✨ Finishing touches
  • 📝 Generate Docstrings
🧪 Generate unit tests
  • Create PR with unit tests
  • Post copyable unit tests in a comment

📜 Recent review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 2ac9124 and 5ff7f4f.

📒 Files selected for processing (1)
  • src/index.ts (1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
  • src/index.ts

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.

@edouardmercier edouardmercier changed the title Fixes "PORT" environment variable and propose the "--generate-lib" CLI option Fixes "PORT" environment variable and proposes the "--generate-lib" CLI option Oct 1, 2025
@edouardmercier edouardmercier changed the title Fixes "PORT" environment variable and proposes the "--generate-lib" CLI option Fixes "PORT" environment variable usage and proposes the "--generate-lib" CLI option Oct 1, 2025
Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 1

🧹 Nitpick comments (2)
src/generator/server-code.ts (2)

48-48: Consider simplifying the generated port expression.

The current implementation correctly addresses the PR objective to honor the PORT environment variable when --port is not specified. However, the generated expression could be simplified.

Current generated code when options.port is undefined:

process.env['PORT'] === undefined ? undefined : parseInt(process.env['PORT'], 10)

This can be simplified to:

process.env['PORT'] ? parseInt(process.env['PORT'], 10) : undefined

Apply this diff:

-  const webServicerPortNumberStatement = `${options.port ?? "process.env['PORT'] === undefined ? undefined : parseInt(process.env['PORT'], 10)"}`;
+  const webServicerPortNumberStatement = `${options.port ?? "process.env['PORT'] ? parseInt(process.env['PORT'], 10) : undefined"}`;

112-113: Acknowledge the dotenv import style change.

The switch from default import to named import is a valid modernization, though this change is not mentioned in the PR objectives.

Both forms work correctly with the dotenv package.

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 8ee9fc3 and 2ac9124.

📒 Files selected for processing (3)
  • src/generator/server-code.ts (5 hunks)
  • src/index.ts (1 hunks)
  • src/types/index.ts (1 hunks)
🔇 Additional comments (6)
src/types/index.ts (1)

32-33: LGTM!

The new generateLib field is well-documented and correctly typed. The documentation clearly explains its purpose in controlling whether the generated code invokes main() or exports it as a library.

src/generator/server-code.ts (5)

55-55: LGTM!

The web transport correctly uses the new dynamic port resolution, which honors the PORT environment variable when --port is not specified.


65-65: LGTM!

The StreamableHTTP transport correctly uses the new dynamic port resolution, consistent with the web transport.


85-102: LGTM!

The conditional mainDeclaration block correctly implements the library generation mode:

  • Omits cleanup and signal handlers when generateLib is true
  • Includes proper shutdown handling and main invocation for standalone mode
  • Aligns with the PR objective to allow main() to be imported and called from existing code

182-182: LGTM!

The conditional export correctly implements the library generation mode. When generateLib is true, main() is exported for consumption by other code; otherwise, it remains a private function called by the startup code.


186-186: LGTM!

The mainDeclaration insertion is correctly placed after the main() function definition, ensuring proper code structure in both library and standalone modes.

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.

1 participant