Skip to content

Conversation

@justin808
Copy link
Member

@justin808 justin808 commented Nov 8, 2025

Summary

Fixes TypeScript compilation error in tests/registerServerComponent.client.test.jsx by changing React imports from default import syntax to namespace import syntax.

Changes:

  • Updated RSCRoute.tsx to use import * as React from 'react'
  • Updated RSCProvider.tsx to use import * as React from 'react'

Root Cause:
The project's tsconfig.json has esModuleInterop: false, but @types/react uses export = syntax. This combination requires namespace imports (import * as React) rather than default imports (import React).

Before:

import React, { Component, use, type ReactNode } from 'react';

After:

import * as React from 'react';
import { Component, use, type ReactNode } from 'react';

Test Plan

  • Verified tests/registerServerComponent.client.test.jsx now passes
  • Ran all non-RSC tests in react-on-rails-pro package - all passing
  • Ran linting checks (ESLint, Prettier, RuboCop) - all passing
  • Pre-commit hooks passed automatically

🤖 Generated with Claude Code


This change is Reviewable

Summary by CodeRabbit

  • Refactor
    • Internal code organization improvements with no changes to user-facing functionality or behavior.

Changes React imports from default import syntax to namespace import
syntax in RSCRoute.tsx and RSCProvider.tsx. This fixes TypeScript
compilation errors when esModuleInterop is set to false in tsconfig.

Before: import React, { ... } from 'react'
After: import * as React from 'react'

This is required because @types/react uses 'export =' syntax, which
can only be default-imported when esModuleInterop is enabled.

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude <[email protected]>
@coderabbitai
Copy link
Contributor

coderabbitai bot commented Nov 8, 2025

Walkthrough

Import statements in two React component files are reorganized by splitting single import lines into separate namespace and named imports. No behavioral or functional changes occur—only module import structure is modified.

Changes

Cohort / File(s) Change Summary
Import statement restructuring
packages/react-on-rails-pro/src/RSCProvider.tsx, packages/react-on-rails-pro/src/RSCRoute.tsx
Split single React import line into two separate statements: namespace import (import * as React) and named imports (createContext, useContext, type ReactNode)

Estimated code review effort

🎯 1 (Trivial) | ⏱️ ~2 minutes

  • Import reorganization only; no logic changes, type changes, or API modifications
  • Purely cosmetic refactoring across consistent pattern in two files

Possibly related PRs

Suggested labels

bug

Suggested reviewers

  • AbanoubGhadban

Poem

🐰 Imports hop and reorganize,
Namespace and named, a cleaner sunrise,
React bows split with stylish care,
No bugs to chase, just structure fair! ✨

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 accurately summarizes the main change: converting React imports to namespace-import syntax to fix compatibility with esModuleInterop: false, which is the primary objective of the PR.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
✨ 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 justin808/fix-react-import-esmodule

📜 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 21ca217 and ba964a1.

📒 Files selected for processing (2)
  • packages/react-on-rails-pro/src/RSCProvider.tsx (1 hunks)
  • packages/react-on-rails-pro/src/RSCRoute.tsx (1 hunks)
🧰 Additional context used
🧠 Learnings (13)
📓 Common learnings
Learnt from: justin808
Repo: shakacode/react_on_rails PR: 1770
File: lib/generators/react_on_rails/templates/base/base/app/javascript/src/HelloWorld/ror_components/HelloWorld.client.jsx:2-2
Timestamp: 2025-09-16T08:01:11.146Z
Learning: React on Rails uses webpack CSS Modules configuration with namedExports: true, which requires the import syntax `import * as style from './file.module.css'` rather than the default export pattern. This configuration enables better tree shaking and bundle size optimization for CSS modules.
Learnt from: alexeyr-ci2
Repo: shakacode/react_on_rails PR: 1732
File: spec/dummy/client/app-react16/startup/ReduxSharedStoreApp.client.jsx:40-44
Timestamp: 2025-04-26T21:55:55.874Z
Learning: In the react_on_rails project, files under `app-react16` directories are copied/moved to corresponding `/app` directories during the conversion process (removing the `-react16` suffix), which affects their relative import paths at runtime.
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.
Learnt from: alexeyr-ci
Repo: shakacode/react_on_rails PR: 1687
File: node_package/src/buildConsoleReplay.ts:12-12
Timestamp: 2025-01-23T18:19:25.261Z
Learning: When reviewing imports, carefully distinguish between default exports and named exports, as they represent different usage patterns. The presence of an import from a file doesn't necessarily mean all exported members are being used.
📚 Learning: 2025-04-26T21:55:55.874Z
Learnt from: alexeyr-ci2
Repo: shakacode/react_on_rails PR: 1732
File: spec/dummy/client/app-react16/startup/ReduxSharedStoreApp.client.jsx:40-44
Timestamp: 2025-04-26T21:55:55.874Z
Learning: In the react_on_rails project, files under `app-react16` directories are copied/moved to corresponding `/app` directories during the conversion process (removing the `-react16` suffix), which affects their relative import paths at runtime.

Applied to files:

  • packages/react-on-rails-pro/src/RSCRoute.tsx
  • packages/react-on-rails-pro/src/RSCProvider.tsx
📚 Learning: 2025-09-16T08:01:11.146Z
Learnt from: justin808
Repo: shakacode/react_on_rails PR: 1770
File: lib/generators/react_on_rails/templates/base/base/app/javascript/src/HelloWorld/ror_components/HelloWorld.client.jsx:2-2
Timestamp: 2025-09-16T08:01:11.146Z
Learning: React on Rails uses webpack CSS Modules configuration with namedExports: true, which requires the import syntax `import * as style from './file.module.css'` rather than the default export pattern. This configuration enables better tree shaking and bundle size optimization for CSS modules.

Applied to files:

  • packages/react-on-rails-pro/src/RSCRoute.tsx
  • packages/react-on-rails-pro/src/RSCProvider.tsx
📚 Learning: 2025-07-08T05:57:29.630Z
Learnt from: AbanoubGhadban
Repo: shakacode/react_on_rails PR: 1745
File: node_package/src/RSCRequestTracker.ts:8-14
Timestamp: 2025-07-08T05:57:29.630Z
Learning: The global `generateRSCPayload` function in React on Rails Pro (RORP) is provided by the framework during rendering requests, not implemented in application code. The `declare global` statements are used to document the expected interface that RORP will inject at runtime.

Applied to files:

  • packages/react-on-rails-pro/src/RSCRoute.tsx
  • packages/react-on-rails-pro/src/RSCProvider.tsx
📚 Learning: 2024-12-12T13:07:09.929Z
Learnt from: alexeyr-ci
Repo: shakacode/react_on_rails PR: 1644
File: node_package/src/ReactOnRailsRSC.ts:87-87
Timestamp: 2024-12-12T13:07:09.929Z
Learning: When handling errors in 'node_package/src/ReactOnRailsRSC.ts', include the error stack in error messages in development and test environments to aid debugging.

Applied to files:

  • packages/react-on-rails-pro/src/RSCRoute.tsx
  • packages/react-on-rails-pro/src/RSCProvider.tsx
📚 Learning: 2025-06-09T07:58:02.646Z
Learnt from: AbanoubGhadban
Repo: shakacode/react_on_rails PR: 1696
File: node_package/src/RSCPayloadGenerator.ts:124-168
Timestamp: 2025-06-09T07:58:02.646Z
Learning: In React Server Components (RSC) implementations, explicit error handling in RSC payload generation streams (like in RSCPayloadGenerator.ts) is not needed because errors will propagate to client components that fetch the RSC payloads (such as RSCRoute components). React's built-in error handling mechanisms and ErrorBoundary components are sufficient for handling these errors at the component level, which allows for better user experience with appropriate error UI fallbacks.

Applied to files:

  • packages/react-on-rails-pro/src/RSCRoute.tsx
  • packages/react-on-rails-pro/src/RSCProvider.tsx
📚 Learning: 2025-02-13T16:50:47.848Z
Learnt from: AbanoubGhadban
Repo: shakacode/react_on_rails PR: 1644
File: node_package/src/clientStartup.ts:18-21
Timestamp: 2025-02-13T16:50:47.848Z
Learning: In the react_on_rails module, the `reactOnRailsPageUnloaded` function in clientStartup.ts is intentionally kept private as it's only used internally as a callback for `onPageUnloaded`.

Applied to files:

  • packages/react-on-rails-pro/src/RSCRoute.tsx
📚 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-pro/src/RSCRoute.tsx
  • packages/react-on-rails-pro/src/RSCProvider.tsx
📚 Learning: 2025-02-12T16:38:06.537Z
Learnt from: Romex91
Repo: shakacode/react_on_rails PR: 1697
File: package-scripts.yml:28-28
Timestamp: 2025-02-12T16:38:06.537Z
Learning: The file `node_package/lib/ReactOnRails.full.js` is autogenerated during the build process and should not be present in the repository.

Applied to files:

  • packages/react-on-rails-pro/src/RSCRoute.tsx
  • packages/react-on-rails-pro/src/RSCProvider.tsx
📚 Learning: 2025-02-18T13:08:01.477Z
Learnt from: AbanoubGhadban
Repo: shakacode/react_on_rails PR: 1644
File: lib/react_on_rails/helper.rb:190-197
Timestamp: 2025-02-18T13:08:01.477Z
Learning: RSC support validation in React on Rails Pro is handled through a chain of validations:
1. Pro version check in `run_stream_inside_fiber`
2. RSC support check during pack generation via `ReactOnRailsPro.configuration.enable_rsc_support`
3. RSC support validation during component registration
This makes additional validation in the helper methods unnecessary.

Applied to files:

  • packages/react-on-rails-pro/src/RSCRoute.tsx
  • packages/react-on-rails-pro/src/RSCProvider.tsx
📚 Learning: 2025-02-18T13:08:01.477Z
Learnt from: AbanoubGhadban
Repo: shakacode/react_on_rails PR: 1644
File: lib/react_on_rails/helper.rb:190-197
Timestamp: 2025-02-18T13:08:01.477Z
Learning: RSC support validation is handled in deeper level calls of the React on Rails Pro codebase, so it doesn't need to be validated again in the `rsc_payload_react_component` helper method.

Applied to files:

  • packages/react-on-rails-pro/src/RSCRoute.tsx
  • packages/react-on-rails-pro/src/RSCProvider.tsx
📚 Learning: 2025-06-11T12:34:58.182Z
Learnt from: AbanoubGhadban
Repo: shakacode/react_on_rails PR: 1696
File: node_package/src/RSCProvider.tsx:0-0
Timestamp: 2025-06-11T12:34:58.182Z
Learning: `RSCProvider` intentionally keeps failed `getServerComponent` promises cached (even if rejected) to avoid repeated fetch attempts; callers must use `refetchComponent` to retry.

Applied to files:

  • packages/react-on-rails-pro/src/RSCRoute.tsx
📚 Learning: 2024-10-08T20:53:47.076Z
Learnt from: theforestvn88
Repo: shakacode/react_on_rails PR: 1620
File: spec/dummy/client/app/startup/HelloTurboStream.jsx:3-3
Timestamp: 2024-10-08T20:53:47.076Z
Learning: The `RailsContext` import in `spec/dummy/client/app/startup/HelloTurboStream.jsx` is used later in the project, as clarified by the user theforestvn88.

Applied to files:

  • packages/react-on-rails-pro/src/RSCProvider.tsx
🔇 Additional comments (2)
packages/react-on-rails-pro/src/RSCProvider.tsx (1)

17-18: LGTM! Consistent with RSCRoute.tsx.

The import pattern correctly addresses the esModuleInterop: false constraint and maintains consistency with the other file in this PR. All imported identifiers are used.

packages/react-on-rails-pro/src/RSCRoute.tsx (1)

19-20: Review comment is incorrect—files in different scopes use different TypeScript configurations.

The codebase intentionally uses different TypeScript configurations:

  • packages/react-on-rails-pro/src/ (including RSCRoute.tsx): Uses root tsconfig.json with esModuleInterop: false, correctly requiring the split import pattern.
  • react_on_rails_pro/spec/dummy/: Uses @tsconfig/create-react-app (which has esModuleInterop: true by default), correctly using the old import React, { ... } pattern.
  • lib/generators/: Template files outside src/**/* scope, not governed by the main TypeScript configuration.

The files found with the "old" import pattern are not inconsistencies—they are correctly using their respective import patterns for their own TypeScript configurations. The verification suggestion to check for codebase-wide consistency is invalid because these files legitimately belong to different configuration scopes.

Likely an incorrect or invalid review comment.


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.

@claude
Copy link

claude bot commented Nov 8, 2025

Code Review - PR #1946

✅ Overall Assessment

This is a well-executed fix that correctly addresses a TypeScript compilation error. The changes are minimal, targeted, and align with the existing codebase patterns.


🎯 Strengths

  1. Correct Fix: The change from default imports to namespace imports is the proper solution when esModuleInterop: false is set in tsconfig.json. This is a TypeScript configuration requirement, not a style choice.

  2. Consistency: The modified files now match the import pattern already used in other files in the same package:

    • registerServerComponent/client.tsx (line 15)
    • getReactServerComponent.client.ts (line 15)
    • Other RSC-related files already use import * as React
  3. Minimal Scope: Only 2 files changed with 4 additions and 2 deletions - exactly what's needed, nothing more.

  4. Good Documentation: The PR description clearly explains the root cause and the technical reasoning.

  5. Proper Testing: According to the PR description, tests pass and linting checks passed.


📋 Observations

  1. Complete Coverage: I verified that no other files in packages/react-on-rails-pro/src/ are still using the old import React, pattern. This PR appears to address all remaining instances.

  2. Named Exports: The separation of the namespace import from named exports is clean:

    import * as React from 'react';
    import { createContext, useContext, type ReactNode } from 'react';

✨ No Issues Found

  • Code Quality: Follows TypeScript best practices
  • Best Practices: Aligns with existing codebase patterns
  • Potential Bugs: None identified
  • Performance: No impact (compile-time only change)
  • Security: Not applicable
  • Test Coverage: Existing tests verify the fix works

🚀 Recommendation

APPROVE - This PR is ready to merge. It's a straightforward TypeScript compatibility fix that maintains consistency with the rest of the codebase.


📝 Minor Suggestion (Optional)

Consider adding a comment in tsconfig.json explaining why esModuleInterop: false is required, if not already documented elsewhere. This would help future contributors understand why namespace imports are necessary. Example:

{
  "compilerOptions": {
    // Set to false for compatibility with @types/react export = syntax
    // Requires namespace imports: import * as React from 'react'
    "esModuleInterop": false,
    ...
  }
}

But this is a nice-to-have, not a blocker.

@coderabbitai coderabbitai bot added the bug label Nov 8, 2025
@justin808 justin808 merged commit 1e68434 into master Nov 8, 2025
22 of 23 checks passed
@justin808 justin808 deleted the justin808/fix-react-import-esmodule branch November 8, 2025 22:26
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants