Fix TypeScript error in processPromise return type

[justin808]

Summary

• Fixed TypeScript compilation error in serverRenderReactComponent.ts that was blocking the release script
• The error occurred because TypeScript couldn't narrow the type properly after the isValidElement check in the .then() callback
• Added explicit type assertion to FinalHtmlResult since after the isValidElement check, the remaining types (string and ServerRenderHashRenderedHtml) are all valid FinalHtmlResult types

Test plan

• TypeScript type-check passes (pnpm run type-check)
• Build completes successfully (pnpm run build)
• ESLint passes (pnpm run lint)

🤖 Generated with Claude Code

Summary by CodeRabbit

• Refactor
  • Improved type safety for server-side rendering operations to ensure more robust handling of promise resolutions.

_{✏️ Tip: You can customize this high-level summary in your review settings.}

[coderabbitai]

Walkthrough

Added FinalHtmlResult type import from ./types/index.ts to serverRenderReactComponent.ts and applied it as a type cast in the processPromise function to ensure Promise resolution results are properly typed as either string or ServerRenderHashRenderedHtml.

Changes

Cohort / File(s)	Summary
Type import and casting packages/react-on-rails/src/serverRenderReactComponent.ts	Added FinalHtmlResult type import from ./types/index.ts; applied type cast to promiseResult in processPromise function for proper Promise resolution type handling

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~8 minutes

• Verify FinalHtmlResult type definition matches the expected union of string and ServerRenderHashRenderedHtml
• Confirm the cast placement in processPromise correctly handles all Promise resolution paths
• Check that this type cast resolves any prior type safety warnings or errors

Possibly related PRs

• Refactor serverRenderReactComponent function #1653: Both PRs modify serverRenderReactComponent's type handling for promise and render results using FinalHtmlResult-like types

Suggested labels

bug

Suggested reviewers

• alexeyr-ci

Poem

🐰 A type came dancing, clear and bright,
Cast in the promise, holding tight,
FinalHtmlResult, friend so true,
Now the code flows safe and new! ✨

Pre-merge checks and finishing touches

❌ Failed checks (1 warning)

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.

✅ Passed checks (2 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title 'Fix TypeScript error in processPromise return type' directly and clearly describes the main change: resolving a TypeScript compilation error in the processPromise function's return type by adding explicit type assertion.

✨ 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 jg/fix-ts-render-result-type

---

📜 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 5d5900c and 60df1c0.

📒 Files selected for processing (1)

• packages/react-on-rails/src/serverRenderReactComponent.ts (2 hunks)

🧰 Additional context used

📓 Path-based instructions (3)

**/*.{rb,js,ts,jsx,tsx,json,yml,yaml,md}

📄 CodeRabbit inference engine (CLAUDE.md)

**/*.{rb,js,ts,jsx,tsx,json,yml,yaml,md}: ALWAYS run bundle exec rubocop and fix ALL violations before every commit/push
ALWAYS ensure files end with a newline character before committing

Files:

• packages/react-on-rails/src/serverRenderReactComponent.ts

**/*.{js,ts,jsx,tsx,json,css,scss,md}

📄 CodeRabbit inference engine (CLAUDE.md)

ALWAYS let Prettier handle ALL formatting - never manually format code

Files:

• packages/react-on-rails/src/serverRenderReactComponent.ts

**/*.{js,ts,jsx,tsx}

📄 CodeRabbit inference engine (CLAUDE.md)

Only use pnpm as the JavaScript package manager, never use npm or yarn

Files:

• packages/react-on-rails/src/serverRenderReactComponent.ts

🧠 Learnings (2)

📓 Common learnings

Learnt from: CR
Repo: shakacode/react_on_rails PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-10T09:27:45.093Z
Learning: Applies to /CHANGELOG.md : Update `/CHANGELOG.md` for open-source features, bug fixes, breaking changes, deprecations, and performance improvements using format: `[PR 1818](https://github.com/shakacode/react_on_rails/pull/1818) by [username](https://github.com/username)`

Learnt from: AbanoubGhadban
Repo: shakacode/react_on_rails PR: 1644
File: node_package/src/RSCWebpackLoader.ts:0-0
Timestamp: 2025-02-13T19:09:15.991Z
Learning: In React Server Components webpack loader, using `new Function('return import("react-server-dom-webpack/node-loader")')()` is necessary as a workaround to bypass TypeScript compilation issues with direct dynamic imports.

📚 Learning: 2025-02-13T19:09:15.991Z

Learnt from: AbanoubGhadban
Repo: shakacode/react_on_rails PR: 1644
File: node_package/src/RSCWebpackLoader.ts:0-0
Timestamp: 2025-02-13T19:09:15.991Z
Learning: In React Server Components webpack loader, using `new Function('return import("react-server-dom-webpack/node-loader")')()` is necessary as a workaround to bypass TypeScript compilation issues with direct dynamic imports.

Applied to files:

• packages/react-on-rails/src/serverRenderReactComponent.ts

🧬 Code graph analysis (1)

packages/react-on-rails/src/serverRenderReactComponent.ts (1)

packages/react-on-rails/src/types/index.ts (1)

• FinalHtmlResult (246-246)

⏰ 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). (5)

• GitHub Check: dummy-app-integration-tests (3.4, 22, latest)
• GitHub Check: precompile-check
• GitHub Check: build
• GitHub Check: check-bundle-size
• GitHub Check: claude-review

🔇 Additional comments (2)

packages/react-on-rails/src/serverRenderReactComponent.ts (2)

12-12: LGTM! Import necessary for type assertion below.

The addition of FinalHtmlResult to the import list is required for the type assertion in the processPromise function.

---

75-76: Type assertion is correct and necessary.

The type assertion on line 76 is justified and safe. After the isValidElement check eliminates ReactElement from the union, the remaining type is string | ServerRenderHashRenderedHtml, which is exactly FinalHtmlResult. The assertion tells TypeScript's type narrowing to handle the Promise callback limitation. All function return paths correctly match the RenderStateHtml return type.

---

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

• X
• Mastodon
• Reddit
• LinkedIn

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

[claude]

Code Review

Overview

This PR fixes a TypeScript compilation error by adding an explicit type assertion in the processPromise function. The fix is technically sound and safe.

---

✅ Strengths

1. Type Safety is Preserved

   • The type assertion is correct: after the isValidElement check filters out ReactElement, the remaining types are indeed string | ServerRenderHashRenderedHtml, which exactly matches FinalHtmlResult
   • Looking at the type definitions:
     • CreateReactOutputAsyncResult = Promise<string | ServerRenderHashRenderedHtml | ReactElement>
     • FinalHtmlResult = string | ServerRenderHashRenderedHtml
   • The type narrowing logic is sound

2. Clear Documentation

   • The inline comment explains why the assertion is needed, making the code self-documenting

3. Minimal Change

   • Only adds what's necessary to fix the TypeScript error
   • No over-engineering or unnecessary refactoring

4. Proper Testing

   • Per the PR description, the author verified:
     • TypeScript type-check passes
     • Build completes successfully
     • ESLint passes

---

🔍 Analysis

Why TypeScript couldn't narrow the type automatically:

The issue occurs because the .then() callback creates a new scope where TypeScript's control flow analysis can't track that isValidElement has already eliminated the ReactElement possibility from the union type. This is a known TypeScript limitation with async contexts.

Why the fix is safe:

The logic flow guarantees type safety:

1. promiseResult has type string | ServerRenderHashRenderedHtml | ReactElement
2. The if (isValidElement(promiseResult)) branch handles the ReactElement case
3. The remaining code path can only have string | ServerRenderHashRenderedHtml
4. This exactly matches FinalHtmlResult

---

📋 Minor Suggestions (Optional)

While the fix is correct, here are some optional considerations:

1. Alternative approach (if you want to avoid type assertions entirely):

   return result.then((promiseResult) => {
     if (isValidElement(promiseResult)) {
       return processReactElement(promiseResult);
     }
     // Explicit narrow to FinalHtmlResult
     const htmlResult: FinalHtmlResult = promiseResult;
     return htmlResult;
   });

   This achieves the same result but uses a type annotation instead of a type assertion. However, the current approach is perfectly fine.

2. Consider adding a runtime check (defense in depth):

   if (typeof promiseResult \!== 'string' && typeof promiseResult \!== 'object') {
     throw new Error('Unexpected promise result type: ' + typeof promiseResult);
   }

   But this is probably overkill given the type system already enforces this.

---

✅ Verdict

Approve with confidence. This is a well-reasoned fix for a TypeScript type narrowing limitation. The type assertion is safe, well-documented, and follows best practices.

🚀 Pre-merge Checklist

Per CLAUDE.md requirements, ensure before merging:

• TypeScript type-check passes (pnpm run type-check)
• Build succeeds (pnpm run build)
• Linting passes (pnpm run lint)
• RuboCop passes (bundle exec rubocop) - N/A for TS-only change
• Files end with newlines - Should be handled by pre-commit hooks

No blocking issues found. ✨

---

Reviewed by Claude Code following project guidelines in CLAUDE.md

[github-actions]

size-limit report 📦

Path	Size
react-on-rails/client bundled (gzip)	62.33 KB (0%)
react-on-rails/client bundled (gzip) (time)	62.33 KB (0%)
react-on-rails/client bundled (brotli)	53.57 KB (0%)
react-on-rails/client bundled (brotli) (time)	53.57 KB (0%)
react-on-rails-pro/client bundled (gzip)	63.39 KB (0%)
react-on-rails-pro/client bundled (gzip) (time)	63.39 KB (0%)
react-on-rails-pro/client bundled (brotli)	54.53 KB (0%)
react-on-rails-pro/client bundled (brotli) (time)	54.53 KB (0%)
registerServerComponent/client bundled (gzip)	71.12 KB (0%)
registerServerComponent/client bundled (gzip) (time)	71.12 KB (0%)
registerServerComponent/client bundled (brotli)	61.27 KB (0%)
registerServerComponent/client bundled (brotli) (time)	61.27 KB (0%)
wrapServerComponentRenderer/client bundled (gzip)	65.75 KB (0%)
wrapServerComponentRenderer/client bundled (gzip) (time)	65.75 KB (0%)
wrapServerComponentRenderer/client bundled (brotli)	56.42 KB (0%)
wrapServerComponentRenderer/client bundled (brotli) (time)	56.42 KB (0%)