Walkthrough

Added two new public exports to the react-on-rails-pro-node-renderer package: ./master and ./worker, each referencing a JavaScript implementation and a TypeScript declaration. No other functional code changes were introduced.

Changes

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

Suggested labels

review-needed, full-ci

Suggested reviewers

• AbanoubGhadban
• alexeyr-ci

Poem

🐇 I hopped through package.json with care,
Two new doors opened — master and worker there.
Types and JS stacked in tidy rows,
Exports now shine where the rabbit goes. ✨

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 @coderabbitai help to get the list of available commands and usage tips.}

size-limit report 📦

Greptile Overview

Greptile Summary

Adds ./master and ./worker exports to the node-renderer package, enabling customers to import these modules directly for custom Fastify configuration.

• Enables direct imports: import masterRun from 'react-on-rails-pro-node-renderer/master' and import { configureFastify } from 'react-on-rails-pro-node-renderer/worker'
• Follows existing export pattern using types and default fields for TypeScript and JavaScript files
• Source files (src/master.ts, src/worker.ts) exist and will compile to lib/master.js, lib/master.d.ts, lib/worker.js, lib/worker.d.ts via TypeScript build
• Solves customer need to add custom routes (e.g., /health endpoints) without patching package.json
• Makes future upgrades easier by eliminating need for manual package.json patches

Confidence Score: 5/5

• This PR is safe to merge with no risk
• The change is minimal and straightforward - it only adds two new export entries to package.json following the exact same pattern as existing exports. The referenced source files exist and are already used internally by the package. No code logic is modified, and the change is purely additive (enabling new imports without breaking existing functionality).
• No files require special attention

Important Files Changed

Sequence Diagram

sequenceDiagram
    participant Customer as Customer Code
    participant Worker as worker.js
    participant Fastify as Fastify Instance
    participant Master as master.js
    
    Note over Customer: Before PR: Patch package.json
    Note over Customer: After PR: Direct import
    
    Customer->>Worker: import { configureFastify } from 'react-on-rails-pro-node-renderer/worker'
    Customer->>Worker: configureFastify((app) => { app.get('/health', ...) })
    Worker->>Fastify: Register custom route
    
    Customer->>Master: import masterRun from 'react-on-rails-pro-node-renderer/master'
    Customer->>Master: masterRun(config)
    Master->>Worker: Fork workers with custom config

Code Review: Add master and worker exports to node-renderer package

Overall Assessment

This PR is well-structured and safe to merge after addressing the recommendations below. The change is minimal, focused, and solves a real customer need.

Code Quality - Strengths

• Clean, minimal change - only adds 8 lines to package.json
• Follows existing export patterns in the package (matches ./integrations/* pattern)
• Consistent with export patterns in sibling packages
• TypeScript declarations are automatically generated by the build
• Source files (src/master.ts, src/worker.ts) exist and export appropriate functions
• PR description clearly explains the business value

Security Considerations

No security concerns identified. These modules are already part of the package and used internally. No new dependencies, no sensitive data exposed.

(Review continued in next comment...)

Critical Testing Requirements (BLOCKING)

These tests MUST be completed before merge (per CLAUDE.md):

Based on the project guidelines in .claude/docs/testing-build-scripts.md:

1. Test clean install (MOST CRITICAL):

   • cd packages/react-on-rails-pro-node-renderer
   • rm -rf node_modules
   • pnpm install --frozen-lockfile

2. Test build produces all expected files:

   • pnpm run build
   • ls -la lib/master.js lib/master.d.ts lib/worker.js lib/worker.d.ts

3. Test yalc publish (critical for local development):

   • pnpm run yalc:publish

4. Test imports in consuming project

Why this matters:

• Package.json export changes have historically caused silent failures (see CLAUDE.md examples)
• The customer is currently patching package.json - we need to ensure our exports work correctly
• Yalc publish failures won't be caught by CI

Documentation & Testing Recommendations

1. Add usage example to PR description showing how to add a health endpoint using configureFastify

2. Consider adding a test in tests/worker.test.ts that verifies the exports are accessible

3. Update changelog - Per CLAUDE.md, this is a user-visible feature addition and should be documented in CHANGELOG_PRO.md:

   • Section: New Features
   • Format: PR 2326 by justin808: Add master and worker exports to node-renderer package, enabling custom Fastify configuration for infrastructure needs like health endpoints

Performance Considerations

No performance impact - export declarations are compile-time only, no runtime overhead.

Approval Checklist

Before merging:

• Run pnpm run build and verify all 4 files are generated
• Run pnpm run yalc:publish successfully
• Test importing the new exports in a consuming project
• Update CHANGELOG_PRO.md
• Run bundle exec rubocop
• Verify CI passes

Overall: Strong PR. Minimal risk, clear business value, well-implemented.

Code Review - PR 2326

This PR adds ./master and ./worker exports to the react-on-rails-pro-node-renderer package.

Positive Findings

Code Quality

• Clean implementation: minimal, focused, follows existing patterns
• Consistent with existing exports: matches ./integrations/* pattern
• Type safety maintained: both .d.ts and .js files properly specified

Security

• No security concerns - only exposes existing, intentionally public API modules
• The configureFastify function (worker.ts:61-63) is designed for extensibility
• Master process management is appropriately isolated

Architecture

• Well-designed extensibility: configureFastify shows this use case was already considered
• Customer need is legitimate: Dockerized deployments need health check endpoints
• Reduces friction: eliminates package.json patching on every upgrade

CRITICAL Recommendation: Build Verification

Per CLAUDE.md testing guidelines, you MUST verify the build produces expected files:

cd packages/react-on-rails-pro-node-renderer && pnpm run clean && pnpm run build && ls -la lib/master.* lib/worker.* && pnpm run yalc:publish

Why this matters: The project had a 7-week silent failure when package.json paths didn't match actual build outputs (see .claude/docs/testing-build-scripts.md).

Other Recommendations

1. Update test plan checkboxes in PR description after completing verification
2. Consider documenting the configureFastify pattern in Pro docs (not a blocker)
3. TypeScript source verified: src/master.ts and src/worker.ts exist, tsconfig has declaration:true

Test Coverage Assessment

Current coverage is adequate for this change:

• What's being added: Package export declarations (metadata only)
• What needs testing: Build process produces files, imports work
• No unit tests needed (this is infrastructure, not logic)

Merge Readiness

Status: Ready to merge after build verification

Before merging:

1. Run build verification commands above
2. Test importing in a consuming project
3. Update test plan checkboxes
4. Monitor CI (all checks passing/skipped as expected)

After merging:

• Consider adding this pattern to Pro documentation
• Watch for customer questions about custom Fastify configuration

Export Syntax Correctness

The chosen export syntax is correct for modern Node.js packages. It provides TypeScript types, runtime module, and compatibility with both CommonJS and ES modules.

Customer Impact

This is a quality-of-life improvement for Pro customers:

• Removes maintenance burden (no more package.json patching)
• Future-proof (survives package upgrades)
• Enables legitimate use case (health checks in Docker)

Great work! This is a clean, focused change that solves a real customer need. Just verify the build locally and you're good to merge.

Code Review: Add master and worker exports to node-renderer package

Summary

This PR adds public exports for master.ts and worker.ts modules in the react-on-rails-pro-node-renderer package, allowing customers to import these modules directly for custom configuration.

✅ Strengths

1. Clear Use Case: Addresses a real customer need - enabling health check endpoints in Dockerized environments without requiring package.json patches.

2. Minimal Change: Clean and non-invasive - just adding two export entries to package.json.

3. Follows Package Standards: Uses modern Node.js package.json exports field with proper TypeScript type definitions.

4. Good Changelog Entry: Pro changelog updated with proper formatting.

🔍 Code Quality Analysis

Package.json Exports - VERIFIED:

• ✅ Source files exist: src/master.ts and src/worker.ts
• ✅ TypeScript build config will compile these to lib/ directory
• ✅ Includes both type definitions and runtime JavaScript
• ✅ Consistent with existing ./integrations/* export pattern

API Surface Exposed:

master.ts exports:

• masterRun(runningConfig?: Partial<Config>) - Main entry point for master process

worker.ts exports:

• run(config: Partial<Config>) - Main entry point for worker process
• configureFastify(configFunction: FastifyConfigFunction) - Key function customers need
• disableHttp2() - Test utility

📋 Testing Recommendations

According to CLAUDE.md, verify:

1. Build artifacts exist after build:

   cd packages/react-on-rails-pro-node-renderer
   pnpm run build
   ls -la lib/master.* lib/worker.*

2. Test imports work in a consuming project

3. Verify yalc publish succeeds (critical per CLAUDE.md)

🎯 Potential Issues & Questions

1. Missing Documentation (Minor): No README explaining how to use these new exports. Consider adding examples of using configureFastify() to add custom routes.

2. Public API Stability (Question): By exposing these modules, they become part of the public API. Are you comfortable committing to the stability of these APIs, or should there be a disclaimer that these are advanced exports that may change?

3. Type Safety Enhancement: Consider documenting safe practices for configureFastify().

🔒 Security: ✅ Low Risk

• This is a Pro package for internal infrastructure
• Exposing Fastify for customization is the explicit goal
• Changes are additive only

⚡ Performance: ✅ No Impact

• No runtime performance changes
• Simply exposes existing modules via package.json exports

🧪 Test Coverage: ✅ Adequate

• Existing tests continue to pass
• No new tests needed for export configuration

🎬 Final Verdict

Recommendation: ✅ APPROVE with minor suggestions

This is a well-scoped, low-risk change that solves a real customer problem.

Before Merging:

1. ✅ Wait for CI to pass (currently in progress)
2. ✅ Manually verify build artifacts include lib/master.* and lib/worker.*
3. ✅ Test imports in a consuming project
4. 📝 Consider adding usage documentation (optional but recommended)

Post-Merge:

1. Monitor customer feedback
2. Consider adding examples/docs in future PR if requested

Great work on this customer-focused improvement! 🎉

Bencher Report

Bencher Report