Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Disable esModuleInterop #1699

Merged
merged 1 commit into from
Feb 26, 2025
Merged

Disable esModuleInterop #1699

merged 1 commit into from
Feb 26, 2025

Conversation

alexeyr-ci
Copy link
Collaborator

@alexeyr-ci alexeyr-ci commented Feb 25, 2025

Summary

Disable esModuleInterop to improve interoperability.

https://evertpot.com/universal-commonjs-esm-typescript-packages/ says:

On the surface esModuleInterop seems like it would be helpful, but it has a major problem: if our libraries use esModuleInterop, anyone who uses that library is forced to also turn it on.

So turning by turning esModuleInterop off we don’t force anyone downstream into a specific setting. I generally recommend anyone writing libraries to turn this off to reduce friction and ironically increase interopability.

Shopify/flash-list#707 is an example of this issue.

It should also decrease bundle size slightly by not generating import helpers.

Pull Request checklist

- [ ] Add/update test to cover these changes
- [ ] Update documentation

  • Update CHANGELOG file

This change is Reviewable

Summary by CodeRabbit

  • Documentation

    • Updated the changelog to note a configuration update for module interoperability.
  • Refactor

    • Standardized how external libraries and components are referenced for a smoother code structure.
  • Chores

    • Adjusted configuration settings to enhance long-term stability and maintainability.

Copy link
Contributor

coderabbitai bot commented Feb 25, 2025

Walkthrough

This pull request updates several module import statements throughout the codebase. The changes replace default imports with namespace imports for React, ReactDOM, ReactDOMServer, PropTypes, and createReactClass across source and test files. Additionally, the tsconfig.json is modified to disable the esModuleInterop option. A new entry is added in CHANGELOG.md under the "Fixed" section referencing PR 1699 and contributor alexeyr-ci.

Changes

File(s) Change Summary
CHANGELOG.md Added a new entry under "Fixed" noting that esModuleInterop is now disabled (PR 1699, contributor: alexeyr-ci).
node_package/src/clientStartup.ts
node_package/src/reactHydrateOrRender.ts
node_package/src/reactApis.ts
Updated import of ReactDOM from default to namespace (import * as ReactDOM from 'react-dom').
node_package/src/createReactOutput.ts
node_package/src/handleError.ts (for React)
Changed import of React from default to namespace (import * as React from 'react').
node_package/src/handleError.ts
node_package/src/serverRenderReactComponent.ts
node_package/src/streamServerRenderedReactComponent.ts
Changed import of ReactDOMServer from default to namespace (import * as ReactDOMServer from 'react-dom/server'). In streamServerRenderedReactComponent.ts, updated type reference for PipeableStream to ReactDOMServer.PipeableStream.
node_package/tests/ComponentRegistry.test.js
node_package/tests/ReactOnRails.test.js
node_package/tests/renderFunction.test.js
Updated imports for React and createReactClass from default to namespace.
node_package/tests/serverRenderReactComponent.test.js Changed import of React from default to namespace (import * as React from 'react').
node_package/tests/streamServerRenderedReactComponent.test.jsx Modified imports for React and PropTypes to namespace, and updated JSX to use React.Suspense instead of an unqualified Suspense.
tsconfig.json Updated "esModuleInterop" from true to false in compilerOptions.

Possibly related PRs

Poem

I'm a rabbit with a code-filled heart,
Hopping through imports to play my part.
I switched defaults to namespaces with a joyful leap,
And in tsconfig, esModuleInterop now lies asleep.
With every change, my whiskers twitch in delight—
Carrot-powered code shines ever so bright!
🐇✨


📜 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 923525d and dc74d0c.

📒 Files selected for processing (14)
  • CHANGELOG.md (1 hunks)
  • node_package/src/clientStartup.ts (1 hunks)
  • node_package/src/createReactOutput.ts (1 hunks)
  • node_package/src/handleError.ts (1 hunks)
  • node_package/src/reactApis.ts (1 hunks)
  • node_package/src/reactHydrateOrRender.ts (1 hunks)
  • node_package/src/serverRenderReactComponent.ts (1 hunks)
  • node_package/src/streamServerRenderedReactComponent.ts (2 hunks)
  • node_package/tests/ComponentRegistry.test.js (1 hunks)
  • node_package/tests/ReactOnRails.test.js (1 hunks)
  • node_package/tests/renderFunction.test.js (1 hunks)
  • node_package/tests/serverRenderReactComponent.test.js (1 hunks)
  • node_package/tests/streamServerRenderedReactComponent.test.jsx (2 hunks)
  • tsconfig.json (1 hunks)
✅ Files skipped from review due to trivial changes (9)
  • node_package/src/reactHydrateOrRender.ts
  • node_package/src/reactApis.ts
  • node_package/src/clientStartup.ts
  • node_package/tests/renderFunction.test.js
  • node_package/src/serverRenderReactComponent.ts
  • node_package/tests/ReactOnRails.test.js
  • node_package/src/createReactOutput.ts
  • node_package/src/handleError.ts
  • node_package/tests/ComponentRegistry.test.js
⏰ Context from checks skipped due to timeout of 90000ms (5)
  • GitHub Check: build (newest)
  • GitHub Check: rspec-package-tests (newest)
  • GitHub Check: build-dummy-app-webpack-test-bundles (oldest)
  • GitHub Check: build
  • GitHub Check: rspec-package-tests (oldest)
🔇 Additional comments (6)
tsconfig.json (1)

4-4: Approval: Disabling esModuleInterop improves library interoperability

This change addresses the core objective of the PR by disabling the esModuleInterop TypeScript compiler option. When a library uses esModuleInterop: true, it forces downstream consumers to adopt the same setting, creating dependency constraints. By setting it to false, you're ensuring that your library will work with a wider range of TypeScript configurations without requiring specific compiler options.

CHANGELOG.md (1)

23-23: Well-documented change in CHANGELOG

The addition to the CHANGELOG is clear and follows the project's conventions. This properly documents the change to disable esModuleInterop and will help users understand what's changed in this version.

node_package/tests/serverRenderReactComponent.test.js (1)

4-4: Import style updated for esModuleInterop compatibility

Changed from default import to namespace import, which is the correct pattern when esModuleInterop is disabled. This ensures the code works with the new TypeScript configuration while maintaining identical functionality.

node_package/tests/streamServerRenderedReactComponent.test.jsx (1)

5-6: Import and component references updated for esModuleInterop compatibility

The imports have been changed from default to namespace imports to align with the disabled esModuleInterop setting. Additionally, the Suspense component now needs to be referenced with the full namespace (React.Suspense) since it's no longer directly imported.

This maintains identical functionality while ensuring compatibility with the updated TypeScript configuration.

Also applies to: 26-28

node_package/src/streamServerRenderedReactComponent.ts (2)

1-1: Import style correctly updated to support disabling esModuleInterop

The import statement has been properly changed from a default import to a namespace import, which aligns with the PR objective of disabling esModuleInterop. This change will improve interoperability with libraries that don't use this TypeScript setting.


37-38: Type references correctly updated to use namespace

The type references for PipeableStream have been properly updated to use the namespace syntax (ReactDOMServer.PipeableStream), maintaining type safety while adapting to the new import style. This is the correct approach when switching from named imports to namespace imports.

✨ Finishing Touches
  • 📝 Generate Docstrings (Beta)

Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media?

❤️ Share
🪧 Tips

Chat

There are 3 ways to chat with CodeRabbit:

  • Review comments: Directly reply to a review comment made by CodeRabbit. Example:
    • I pushed a fix in commit <commit_id>, please review it.
    • Generate unit testing code for this file.
    • Open a follow-up GitHub issue for this discussion.
  • Files and specific lines of code (under the "Files changed" tab): Tag @coderabbitai in a new review comment at the desired location with your query. Examples:
    • @coderabbitai generate unit testing code for this file.
    • @coderabbitai modularize this function.
  • PR comments: Tag @coderabbitai in a new PR comment to ask questions about the PR branch. For the best results, please provide a very specific query, as very limited context is provided in this mode. Examples:
    • @coderabbitai gather interesting stats about this repository and render them as a table. Additionally, render a pie chart showing the language distribution in the codebase.
    • @coderabbitai read src/utils.ts and generate unit testing code.
    • @coderabbitai read the files in the src/scheduler package and generate a class diagram using mermaid and a README in the markdown format.
    • @coderabbitai help me debug CodeRabbit configuration file.

Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments.

CodeRabbit Commands (Invoked using PR comments)

  • @coderabbitai pause to pause the reviews on a PR.
  • @coderabbitai resume to resume the paused reviews.
  • @coderabbitai review to trigger an incremental review. This is useful when automatic reviews are disabled for the repository.
  • @coderabbitai full review to do a full review from scratch and review all the files again.
  • @coderabbitai summary to regenerate the summary of the PR.
  • @coderabbitai generate docstrings to generate docstrings for this PR. (Beta)
  • @coderabbitai resolve resolve all the CodeRabbit review comments.
  • @coderabbitai configuration to show the current CodeRabbit configuration for the repository.
  • @coderabbitai help to get help.

Other keywords and placeholders

  • Add @coderabbitai ignore anywhere in the PR description to prevent this PR from being reviewed.
  • Add @coderabbitai summary to generate the high-level summary at a specific location in the PR description.
  • Add @coderabbitai anywhere in the PR title to generate the title automatically.

CodeRabbit Configuration File (.coderabbit.yaml)

  • You can programmatically configure CodeRabbit by adding a .coderabbit.yaml file to the root of your repository.
  • Please see the configuration documentation for more information.
  • If your editor has YAML language server enabled, you can add the path at the top of this file to enable auto-completion and validation: # yaml-language-server: $schema=https://coderabbit.ai/integrations/schema.v2.json

Documentation and Community

  • Visit our Documentation for detailed information on how to use CodeRabbit.
  • Join our Discord Community to get help, request features, and share feedback.
  • Follow us on X/Twitter for updates and announcements.

@alexeyr-ci alexeyr-ci force-pushed the alexeyr/disable-esModuleInterop branch 2 times, most recently from 40d069e to 19be6d1 Compare February 25, 2025 11:29
@alexeyr-ci alexeyr-ci force-pushed the alexeyr/disable-esModuleInterop branch from 19be6d1 to dc74d0c Compare February 25, 2025 12:03
@Judahmeek Judahmeek merged commit d80824f into master Feb 26, 2025
11 checks passed
@Judahmeek Judahmeek deleted the alexeyr/disable-esModuleInterop branch February 26, 2025 18:29
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.

3 participants