Skip to content

[noop] Fix createContainer argument order in the Fiber implementation#645

Closed
everettbu wants to merge 9 commits into
mainfrom
sebbie/03-02-_noop_match_fiber_host_config
Closed

[noop] Fix createContainer argument order in the Fiber implementation#645
everettbu wants to merge 9 commits into
mainfrom
sebbie/03-02-_noop_match_fiber_host_config

Conversation

@everettbu

Copy link
Copy Markdown

Mirror of facebook/react#35945
Original author: eps1lon


...and fix most of the type issues in the Fiber implementation of react-noop-renderer

Stacked on react/react#35944

Only addressed the most obvious issues. There's still some issues relating to loosely accessing object properties but those don't feel urgent. Especially because this is a test-only renderer.

The argument order of createContainer was the main concern.

eps1lon added 8 commits March 2, 2026 19:18
...like for the other renderers
Notable `now` and `logRecoverableError` were removed
due to being unused by Fiber.

The rest is either adding throwing shims for missing
host config properties or using the shared shims
when the whole feature isn't implemented.
They can't be imported from `react-reconciler`
since that package is treated as an external of react-noop-renderer
@everettbu everettbu added CLA Signed React Core Team Opened by a member of the React Core Team labels Mar 2, 2026
@everettbu
everettbu marked this pull request as ready for review March 2, 2026 23:16
@greptile-apps

greptile-apps Bot commented Mar 2, 2026

Copy link
Copy Markdown

Greptile Summary

This PR fixes the createContainer argument order in the noop renderer's createLegacyRoot (where isStrictMode and concurrentUpdatesByDefaultOverride were swapped) and adds comprehensive Flow type checking support for react-noop-renderer.

  • Argument order fix: In createLegacyRoot, the isStrictMode (boolean) and concurrentUpdatesByDefaultOverride (null | boolean) arguments to createContainer were in the wrong order — now corrected to match the reconciler's function signature
  • New noop host config modules: Type definitions and shim modules extracted from createReactNoop.js into dedicated files (ReactFiberConfigNoop.js, ReactFiberConfigNoopHydration.js, etc.), following the same pattern used by other renderers
  • New fork config shims: Added *.noop.js fork files for ReactFiberConfig, ReactFizzConfig, ReactFlightServerConfig, ReactFlightClientConfig, and ReactServerStreamConfig to enable proper Flow type checking
  • Flow integration: Removed the blanket Flow ignore for react-noop-renderer and added a proper noop entry in inlinedHostConfigs.js so Flow can now type-check the noop renderer
  • Host config restructuring: createReactNoop.js now splits the host config into sharedHostConfig, mutationHostConfig, and persistenceHostConfig objects, composed at the end based on the useMutation flag
  • Jest setup consolidation: Moved the persistent renderer mock from setupTests.persistent.js and setupTests.xplat.js into setupHostConfigs.js to reduce duplication
  • Type corrections: Fixed Segment.children type in ReactNoopServer.js from a single element to an array (matching actual usage), added Thenable import in Flight client, and added numerous $FlowFixMe suppressions for known type gaps

Confidence Score: 4/5

  • This PR is safe to merge — it's a test-only renderer with type fixes and the argument order correction is verified against the reconciler's function signature.
  • The changes are well-structured: the createContainer argument order fix in createLegacyRoot is clearly correct when compared to the reconciler's function signature, the new config modules follow established patterns from other renderers, and the host config restructuring is a clean refactor. The numerous $FlowFixMe suppressions indicate remaining type gaps but the PR description explicitly acknowledges this as intentional ("Only addressed the most obvious issues"). The noop renderer is test-only infrastructure, limiting blast radius. Score is 4 rather than 5 due to the volume of $FlowFixMe suppressions that represent deferred type safety work.
  • packages/react-noop-renderer/src/createReactNoop.js deserves the most attention as the core file with the argument order fix and structural refactoring.

Important Files Changed

Filename Overview
packages/react-noop-renderer/src/createReactNoop.js Core file: fixes createContainer argument order in createLegacyRoot (isStrictMode/concurrentUpdatesByDefaultOverride swap), extracts types to shared modules, restructures host config into shared/mutation/persistence sections, adds comprehensive Flow type annotations.
packages/react-noop-renderer/src/ReactFiberConfigNoop.js New file: central type definitions (Container, Instance, TextInstance, etc.) and re-exports from sub-config modules for noop renderer.
packages/react-reconciler/src/forks/ReactFiberConfig.noop.js New file: noop fork config shim for the reconciler, re-exports from ReactFiberConfigNoop and bridges $$$config to module exports.
scripts/shared/inlinedHostConfigs.js Adds new 'noop' inlined host config entry with proper entry points and paths for Flow type checking.
scripts/flow/config/flowconfig Removes the Flow ignore for react-noop-renderer since it now has its own proper inlined host config entry.
scripts/jest/setupHostConfigs.js Consolidates persistent mock setup into setupHostConfigs.js, removing duplication from setupTests.persistent.js and setupTests.xplat.js.
packages/react-noop-renderer/src/ReactNoopFlightClient.js Adds multiple $FlowFixMe suppressions for type issues discovered during noop typechecking; adds Thenable import.
packages/react-noop-renderer/src/ReactNoopFlightServer.js Adds $FlowFixMe suppressions, widens Destination type to include strings alongside Uint8Array.
packages/react-noop-renderer/src/ReactNoopServer.js Fixes Segment.children type from single element to Array, fixes Placeholder.parent to include Segment, adds multiple $FlowFixMe suppressions.
packages/react-reconciler/src/tests/ViewTransitionReactServer-test.js Changes test gate from 'enableViewTransition

Last reviewed commit: fe1ec4f

@greptile-apps greptile-apps Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

30 files reviewed, no comments

Edit Code Review Agent Settings | Greptile

@everettbu

Copy link
Copy Markdown
Author

Upstream PR was closed or merged. Code is synced via branch mirror.

@everettbu everettbu closed this Mar 4, 2026
@everettbu
everettbu deleted the sebbie/03-02-_noop_match_fiber_host_config branch March 4, 2026 13:40
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

CLA Signed React Core Team Opened by a member of the React Core Team

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants