Skip to content

Don't hydrate Elements that have dangerouslySetInnerHTML and suppressHydrationWarning#588

Open
everettbu wants to merge 1 commit into
mainfrom
feature/suppressHydrationWarningMeansNoHydration
Open

Don't hydrate Elements that have dangerouslySetInnerHTML and suppressHydrationWarning#588
everettbu wants to merge 1 commit into
mainfrom
feature/suppressHydrationWarningMeansNoHydration

Conversation

@everettbu

Copy link
Copy Markdown

Mirror of facebook/react#35871
Original author: morbidick


Summary

In React 18 it was possible to set dangerouslySetInnerHTML and suppressHydrationWarning for elements you would know that change before hydration, this could be:

  • react rendered script tags that get modified by the included scripts
  • Server Side Includes thar replace parts of the dom

examples are: react/react#32975 or react/react#24430

if you ask a LLM how to fix those warnings they persist on suppressHydrationWarning still being the way to go with React 19

How did you test this change?

I added a simple test case and had a local page where i tried it.

@greptile-apps

greptile-apps Bot commented Feb 23, 2026

Copy link
Copy Markdown

Greptile Summary

This PR restores React 18 behavior where elements with both dangerouslySetInnerHTML and suppressHydrationWarning skip hydration validation, allowing the DOM to be mutated between SSR and hydration without triggering mismatch errors.

  • The core change in ReactFiberHydrationContext.js sets nextHydratableInstance = null to skip child hydration for elements with both props, which correctly preserves mutated DOM content and avoids hydration mismatch throws.
  • The ReactDOMComponent.js change in diffHydratedGenericElement (line 2621-2624) is dead code — the function already bails out at line 2591 when suppressHydrationWarning is set, so the new guard in the dangerouslySetInnerHTML case is unreachable.
  • The hydrateProperties guard (line 3229) is a defensive addition that is redundant in practice, since React forbids using dangerouslySetInnerHTML and string children simultaneously.
  • The test validates the core behavior but does not explicitly assert the absence of hydration warnings.

Confidence Score: 3/5

  • The core reconciler change is sound, but includes dead code in ReactDOMComponent.js that may mislead future contributors.
  • The reconciler change correctly skips child hydration and sibling handling is preserved. However, the diffHydratedGenericElement change is unreachable dead code due to an existing early bail-out, which indicates incomplete understanding of the existing code flow. The defensive guard in hydrateProperties is practically redundant. The test covers the happy path but doesn't explicitly verify the absence of warnings.
  • packages/react-dom-bindings/src/client/ReactDOMComponent.js contains dead code that should be reviewed.

Important Files Changed

Filename Overview
packages/react-reconciler/src/ReactFiberHydrationContext.js Core change: skips child hydration by setting nextHydratableInstance = null when both dangerouslySetInnerHTML and suppressHydrationWarning are set. This correctly preserves mutated DOM content during hydration. Sibling hydration remains functional because popHydrationState uses getNextHydratableSibling(fiber.stateNode) to find the next sibling.
packages/react-dom-bindings/src/client/ReactDOMComponent.js Contains two changes: (1) a dead code addition in diffHydratedGenericElement that is unreachable due to an earlier continue when suppressHydrationWarning is true, and (2) a defensive guard in hydrateProperties to skip text content checks — redundant in practice since dangerouslySetInnerHTML and string children cannot coexist.
packages/react-dom/src/tests/ReactServerRenderingHydration-test.js Adds a test that validates hydration preserves mutated innerHTML when both dangerouslySetInnerHTML and suppressHydrationWarning are set. Covers sibling elements. Does not explicitly assert the absence of hydration warnings.

Last reviewed commit: 98001e4

@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.

3 files reviewed, 1 comment

Edit Code Review Agent Settings | Greptile

Comment on lines +2621 to +2624
// Skip innerHTML comparison only when suppressHydrationWarning is also set
if (props.suppressHydrationWarning === true) {
continue;
}

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Unreachable code due to earlier bail-out

This check can never evaluate to true. Earlier in the same for loop (line 2591), there is an unconditional continue when props.suppressHydrationWarning === true:

if (props.suppressHydrationWarning === true) {
  // Don't bother comparing. We're ignoring all these warnings.
  continue;
}

That continue fires for every prop key iteration when suppressHydrationWarning is set, which means execution never reaches the switch statement — and by extension, never reaches this case 'dangerouslySetInnerHTML' branch. The new guard at line 2622 is dead code.

This is not a functional bug (the behavior is the same either way — dangerouslySetInnerHTML diffs are already skipped when suppressHydrationWarning is set), but the code is misleading since it suggests this check is the mechanism providing the skip behavior.

Prompt To Fix With AI
This is a comment left during a code review.
Path: packages/react-dom-bindings/src/client/ReactDOMComponent.js
Line: 2621-2624

Comment:
**Unreachable code due to earlier bail-out**

This check can never evaluate to `true`. Earlier in the same `for` loop (line 2591), there is an unconditional `continue` when `props.suppressHydrationWarning === true`:

```js
if (props.suppressHydrationWarning === true) {
  // Don't bother comparing. We're ignoring all these warnings.
  continue;
}
```

That `continue` fires for **every** prop key iteration when `suppressHydrationWarning` is set, which means execution never reaches the `switch` statement — and by extension, never reaches this `case 'dangerouslySetInnerHTML'` branch. The new guard at line 2622 is dead code.

This is not a functional bug (the behavior is the same either way — `dangerouslySetInnerHTML` diffs are already skipped when `suppressHydrationWarning` is set), but the code is misleading since it suggests this check is the mechanism providing the skip behavior.

How can I resolve this? If you propose a fix, please make it concise.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants