From 4dff0e62b2320d8c97746a16c95efd9c9ad0bc07 Mon Sep 17 00:00:00 2001 From: Ricky Date: Fri, 13 Dec 2024 11:05:10 -0500 Subject: [PATCH 1/3] Remove consoleManagedByDevToolsDuringStrictMode (#31755) This is enabled everywhere except the test renderers, which don't use it. --- .../src/ReactFiberDevToolsHook.js | 43 +- .../src/__tests__/ReactStrictMode-test.js | 596 ++++++------------ packages/shared/ReactFeatureFlags.js | 2 - .../forks/ReactFeatureFlags.native-fb.js | 1 - .../forks/ReactFeatureFlags.native-oss.js | 1 - .../forks/ReactFeatureFlags.test-renderer.js | 2 - ...actFeatureFlags.test-renderer.native-fb.js | 1 - .../ReactFeatureFlags.test-renderer.www.js | 2 - .../shared/forks/ReactFeatureFlags.www.js | 2 - 9 files changed, 201 insertions(+), 449 deletions(-) diff --git a/packages/react-reconciler/src/ReactFiberDevToolsHook.js b/packages/react-reconciler/src/ReactFiberDevToolsHook.js index 25ee5ee70cf88..98bad19a8e296 100644 --- a/packages/react-reconciler/src/ReactFiberDevToolsHook.js +++ b/packages/react-reconciler/src/ReactFiberDevToolsHook.js @@ -19,7 +19,6 @@ type DevToolsProfilingHooks = any; import {DidCapture} from './ReactFiberFlags'; import { - consoleManagedByDevToolsDuringStrictMode, enableProfilerTimer, enableSchedulingProfiler, } from 'shared/ReactFeatureFlags'; @@ -38,7 +37,6 @@ import { unstable_setDisableYieldValue, } from './Scheduler'; import {setSuppressWarning} from 'shared/consoleWithStackDev'; -import {disableLogs, reenableLogs} from 'shared/ConsolePatchingDev'; declare const __REACT_DEVTOOLS_GLOBAL_HOOK__: Object | void; @@ -188,36 +186,25 @@ export function onCommitUnmount(fiber: Fiber) { } export function setIsStrictModeForDevtools(newIsStrictMode: boolean) { - if (consoleManagedByDevToolsDuringStrictMode) { - if (typeof log === 'function') { - // We're in a test because Scheduler.log only exists - // in SchedulerMock. To reduce the noise in strict mode tests, - // suppress warnings and disable scheduler yielding during the double render - unstable_setDisableYieldValue(newIsStrictMode); - setSuppressWarning(newIsStrictMode); - } + if (typeof log === 'function') { + // We're in a test because Scheduler.log only exists + // in SchedulerMock. To reduce the noise in strict mode tests, + // suppress warnings and disable scheduler yielding during the double render + unstable_setDisableYieldValue(newIsStrictMode); + setSuppressWarning(newIsStrictMode); + } - if (injectedHook && typeof injectedHook.setStrictMode === 'function') { - try { - injectedHook.setStrictMode(rendererID, newIsStrictMode); - } catch (err) { - if (__DEV__) { - if (!hasLoggedError) { - hasLoggedError = true; - console.error( - 'React instrumentation encountered an error: %s', - err, - ); - } + if (injectedHook && typeof injectedHook.setStrictMode === 'function') { + try { + injectedHook.setStrictMode(rendererID, newIsStrictMode); + } catch (err) { + if (__DEV__) { + if (!hasLoggedError) { + hasLoggedError = true; + console.error('React instrumentation encountered an error: %s', err); } } } - } else { - if (newIsStrictMode) { - disableLogs(); - } else { - reenableLogs(); - } } } diff --git a/packages/react/src/__tests__/ReactStrictMode-test.js b/packages/react/src/__tests__/ReactStrictMode-test.js index db60674ba64bf..863f84ebdb291 100644 --- a/packages/react/src/__tests__/ReactStrictMode-test.js +++ b/packages/react/src/__tests__/ReactStrictMode-test.js @@ -20,8 +20,6 @@ let useState; let useReducer; let assertConsoleErrorDev; -const ReactFeatureFlags = require('shared/ReactFeatureFlags'); - describe('ReactStrictMode', () => { beforeEach(() => { jest.resetModules(); @@ -1111,450 +1109,228 @@ describe('context legacy', () => { console.log.mockRestore(); }); - if (ReactFeatureFlags.consoleManagedByDevToolsDuringStrictMode) { - it('does not disable logs for class double render', async () => { - let count = 0; - class Foo extends React.Component { - render() { - count++; - console.log('foo ' + count); - return null; - } + it('does not disable logs for class double render', async () => { + let count = 0; + class Foo extends React.Component { + render() { + count++; + console.log('foo ' + count); + return null; } + } - const container = document.createElement('div'); - const root = ReactDOMClient.createRoot(container); - await act(() => { - root.render( - - - , - ); - }); - expect(count).toBe(__DEV__ ? 2 : 1); - expect(console.log).toBeCalledTimes(__DEV__ ? 2 : 1); - // Note: we should display the first log because otherwise - // there is a risk of suppressing warnings when they happen, - // and on the next render they'd get deduplicated and ignored. - expect(console.log).toBeCalledWith('foo 1'); + const container = document.createElement('div'); + const root = ReactDOMClient.createRoot(container); + await act(() => { + root.render( + + + , + ); }); + expect(count).toBe(__DEV__ ? 2 : 1); + expect(console.log).toBeCalledTimes(__DEV__ ? 2 : 1); + // Note: we should display the first log because otherwise + // there is a risk of suppressing warnings when they happen, + // and on the next render they'd get deduplicated and ignored. + expect(console.log).toBeCalledWith('foo 1'); + }); - it('does not disable logs for class double ctor', async () => { - let count = 0; - class Foo extends React.Component { - constructor(props) { - super(props); - count++; - console.log('foo ' + count); - } - render() { - return null; - } + it('does not disable logs for class double ctor', async () => { + let count = 0; + class Foo extends React.Component { + constructor(props) { + super(props); + count++; + console.log('foo ' + count); } - - const container = document.createElement('div'); - const root = ReactDOMClient.createRoot(container); - await act(() => { - root.render( - - - , - ); - }); - expect(count).toBe(__DEV__ ? 2 : 1); - expect(console.log).toBeCalledTimes(__DEV__ ? 2 : 1); - // Note: we should display the first log because otherwise - // there is a risk of suppressing warnings when they happen, - // and on the next render they'd get deduplicated and ignored. - expect(console.log).toBeCalledWith('foo 1'); - }); - - it('does not disable logs for class double getDerivedStateFromProps', async () => { - let count = 0; - class Foo extends React.Component { - state = {}; - static getDerivedStateFromProps() { - count++; - console.log('foo ' + count); - return {}; - } - render() { - return null; - } + render() { + return null; } + } - const container = document.createElement('div'); - const root = ReactDOMClient.createRoot(container); - await act(() => { - root.render( - - - , - ); - }); - expect(count).toBe(__DEV__ ? 2 : 1); - expect(console.log).toBeCalledTimes(__DEV__ ? 2 : 1); - // Note: we should display the first log because otherwise - // there is a risk of suppressing warnings when they happen, - // and on the next render they'd get deduplicated and ignored. - expect(console.log).toBeCalledWith('foo 1'); + const container = document.createElement('div'); + const root = ReactDOMClient.createRoot(container); + await act(() => { + root.render( + + + , + ); }); + expect(count).toBe(__DEV__ ? 2 : 1); + expect(console.log).toBeCalledTimes(__DEV__ ? 2 : 1); + // Note: we should display the first log because otherwise + // there is a risk of suppressing warnings when they happen, + // and on the next render they'd get deduplicated and ignored. + expect(console.log).toBeCalledWith('foo 1'); + }); - it('does not disable logs for class double shouldComponentUpdate', async () => { - let count = 0; - class Foo extends React.Component { - state = {}; - shouldComponentUpdate() { - count++; - console.log('foo ' + count); - return {}; - } - render() { - return null; - } + it('does not disable logs for class double getDerivedStateFromProps', async () => { + let count = 0; + class Foo extends React.Component { + state = {}; + static getDerivedStateFromProps() { + count++; + console.log('foo ' + count); + return {}; } - - const container = document.createElement('div'); - const root = ReactDOMClient.createRoot(container); - await act(() => { - root.render( - - - , - ); - }); - await act(() => { - root.render( - - - , - ); - }); - - expect(count).toBe(__DEV__ ? 2 : 1); - expect(console.log).toBeCalledTimes(__DEV__ ? 2 : 1); - // Note: we should display the first log because otherwise - // there is a risk of suppressing warnings when they happen, - // and on the next render they'd get deduplicated and ignored. - expect(console.log).toBeCalledWith('foo 1'); - }); - - it('does not disable logs for class state updaters', async () => { - let inst; - let count = 0; - class Foo extends React.Component { - state = {}; - render() { - inst = this; - return null; - } + render() { + return null; } + } - const container = document.createElement('div'); - const root = ReactDOMClient.createRoot(container); - await act(() => { - root.render( - - - , - ); - }); - await act(() => { - inst.setState(() => { - count++; - console.log('foo ' + count); - return {}; - }); - }); - - expect(count).toBe(__DEV__ ? 2 : 1); - expect(console.log).toBeCalledTimes(__DEV__ ? 2 : 1); - // Note: we should display the first log because otherwise - // there is a risk of suppressing warnings when they happen, - // and on the next render they'd get deduplicated and ignored. - expect(console.log).toBeCalledWith('foo 1'); + const container = document.createElement('div'); + const root = ReactDOMClient.createRoot(container); + await act(() => { + root.render( + + + , + ); }); + expect(count).toBe(__DEV__ ? 2 : 1); + expect(console.log).toBeCalledTimes(__DEV__ ? 2 : 1); + // Note: we should display the first log because otherwise + // there is a risk of suppressing warnings when they happen, + // and on the next render they'd get deduplicated and ignored. + expect(console.log).toBeCalledWith('foo 1'); + }); - it('does not disable logs for function double render', async () => { - let count = 0; - function Foo() { + it('does not disable logs for class double shouldComponentUpdate', async () => { + let count = 0; + class Foo extends React.Component { + state = {}; + shouldComponentUpdate() { count++; console.log('foo ' + count); - return null; + return {}; } - - const container = document.createElement('div'); - const root = ReactDOMClient.createRoot(container); - await act(() => { - root.render( - - - , - ); - }); - expect(count).toBe(__DEV__ ? 2 : 1); - expect(console.log).toBeCalledTimes(__DEV__ ? 2 : 1); - // Note: we should display the first log because otherwise - // there is a risk of suppressing warnings when they happen, - // and on the next render they'd get deduplicated and ignored. - expect(console.log).toBeCalledWith('foo 1'); - }); - - it('does not disable logs for effect double invoke', async () => { - let create = 0; - let cleanup = 0; - function Foo() { - React.useEffect(() => { - create++; - console.log('foo create ' + create); - return () => { - cleanup++; - console.log('foo cleanup ' + cleanup); - }; - }); + render() { return null; } + } - const container = document.createElement('div'); - const root = ReactDOMClient.createRoot(container); - await act(() => { - root.render( - - - , - ); - }); - expect(create).toBe(__DEV__ ? 2 : 1); - expect(cleanup).toBe(__DEV__ ? 1 : 0); - expect(console.log).toBeCalledTimes(__DEV__ ? 3 : 1); - // Note: we should display the first log because otherwise - // there is a risk of suppressing warnings when they happen, - // and on the next render they'd get deduplicated and ignored. - expect(console.log).toBeCalledWith('foo create 1'); - if (__DEV__) { - expect(console.log).toBeCalledWith('foo cleanup 1'); - } + const container = document.createElement('div'); + const root = ReactDOMClient.createRoot(container); + await act(() => { + root.render( + + + , + ); }); - } else { - it('disable logs for class double render', async () => { - let count = 0; - class Foo extends React.Component { - render() { - count++; - console.log('foo ' + count); - return null; - } - } - - const container = document.createElement('div'); - const root = ReactDOMClient.createRoot(container); - await act(() => { - root.render( - - - , - ); - }); - expect(count).toBe(__DEV__ ? 2 : 1); - expect(console.log).toBeCalledTimes(1); - // Note: we should display the first log because otherwise - // there is a risk of suppressing warnings when they happen, - // and on the next render they'd get deduplicated and ignored. - expect(console.log).toBeCalledWith('foo 1'); + await act(() => { + root.render( + + + , + ); }); - it('disables logs for class double ctor', async () => { - let count = 0; - class Foo extends React.Component { - constructor(props) { - super(props); - count++; - console.log('foo ' + count); - } - render() { - return null; - } - } - - const container = document.createElement('div'); - const root = ReactDOMClient.createRoot(container); - await act(() => { - root.render( - - - , - ); - }); - expect(count).toBe(__DEV__ ? 2 : 1); - expect(console.log).toBeCalledTimes(1); - // Note: we should display the first log because otherwise - // there is a risk of suppressing warnings when they happen, - // and on the next render they'd get deduplicated and ignored. - expect(console.log).toBeCalledWith('foo 1'); - }); + expect(count).toBe(__DEV__ ? 2 : 1); + expect(console.log).toBeCalledTimes(__DEV__ ? 2 : 1); + // Note: we should display the first log because otherwise + // there is a risk of suppressing warnings when they happen, + // and on the next render they'd get deduplicated and ignored. + expect(console.log).toBeCalledWith('foo 1'); + }); - it('disable logs for class double getDerivedStateFromProps', async () => { - let count = 0; - class Foo extends React.Component { - state = {}; - static getDerivedStateFromProps() { - count++; - console.log('foo ' + count); - return {}; - } - render() { - return null; - } + it('does not disable logs for class state updaters', async () => { + let inst; + let count = 0; + class Foo extends React.Component { + state = {}; + render() { + inst = this; + return null; } + } - const container = document.createElement('div'); - const root = ReactDOMClient.createRoot(container); - await act(() => { - root.render( - - - , - ); - }); - expect(count).toBe(__DEV__ ? 2 : 1); - expect(console.log).toBeCalledTimes(1); - // Note: we should display the first log because otherwise - // there is a risk of suppressing warnings when they happen, - // and on the next render they'd get deduplicated and ignored. - expect(console.log).toBeCalledWith('foo 1'); + const container = document.createElement('div'); + const root = ReactDOMClient.createRoot(container); + await act(() => { + root.render( + + + , + ); }); - - it('disable logs for class double shouldComponentUpdate', async () => { - let count = 0; - class Foo extends React.Component { - state = {}; - shouldComponentUpdate() { - count++; - console.log('foo ' + count); - return {}; - } - render() { - return null; - } - } - - const container = document.createElement('div'); - const root = ReactDOMClient.createRoot(container); - await act(() => { - root.render( - - - , - ); - }); - await act(() => { - root.render( - - - , - ); + await act(() => { + inst.setState(() => { + count++; + console.log('foo ' + count); + return {}; }); - expect(count).toBe(__DEV__ ? 2 : 1); - expect(console.log).toBeCalledTimes(1); - // Note: we should display the first log because otherwise - // there is a risk of suppressing warnings when they happen, - // and on the next render they'd get deduplicated and ignored. - expect(console.log).toBeCalledWith('foo 1'); }); - it('disable logs for class state updaters', async () => { - let inst; - let count = 0; - class Foo extends React.Component { - state = {}; - render() { - inst = this; - return null; - } - } + expect(count).toBe(__DEV__ ? 2 : 1); + expect(console.log).toBeCalledTimes(__DEV__ ? 2 : 1); + // Note: we should display the first log because otherwise + // there is a risk of suppressing warnings when they happen, + // and on the next render they'd get deduplicated and ignored. + expect(console.log).toBeCalledWith('foo 1'); + }); - const container = document.createElement('div'); - const root = ReactDOMClient.createRoot(container); - await act(() => { - root.render( - - - , - ); - }); - await act(() => { - inst.setState(() => { - count++; - console.log('foo ' + count); - return {}; - }); - }); + it('does not disable logs for function double render', async () => { + let count = 0; + function Foo() { + count++; + console.log('foo ' + count); + return null; + } - expect(count).toBe(__DEV__ ? 2 : 1); - expect(console.log).toBeCalledTimes(1); - // Note: we should display the first log because otherwise - // there is a risk of suppressing warnings when they happen, - // and on the next render they'd get deduplicated and ignored. - expect(console.log).toBeCalledWith('foo 1'); + const container = document.createElement('div'); + const root = ReactDOMClient.createRoot(container); + await act(() => { + root.render( + + + , + ); }); + expect(count).toBe(__DEV__ ? 2 : 1); + expect(console.log).toBeCalledTimes(__DEV__ ? 2 : 1); + // Note: we should display the first log because otherwise + // there is a risk of suppressing warnings when they happen, + // and on the next render they'd get deduplicated and ignored. + expect(console.log).toBeCalledWith('foo 1'); + }); - it('disable logs for function double render', async () => { - let count = 0; - function Foo() { - count++; - console.log('foo ' + count); - return null; - } - - const container = document.createElement('div'); - const root = ReactDOMClient.createRoot(container); - await act(() => { - root.render( - - - , - ); + it('does not disable logs for effect double invoke', async () => { + let create = 0; + let cleanup = 0; + function Foo() { + React.useEffect(() => { + create++; + console.log('foo create ' + create); + return () => { + cleanup++; + console.log('foo cleanup ' + cleanup); + }; }); - expect(count).toBe(__DEV__ ? 2 : 1); - expect(console.log).toBeCalledTimes(1); - // Note: we should display the first log because otherwise - // there is a risk of suppressing warnings when they happen, - // and on the next render they'd get deduplicated and ignored. - expect(console.log).toBeCalledWith('foo 1'); - }); - - it('disable logs for effect double invoke', async () => { - let create = 0; - let cleanup = 0; - function Foo() { - React.useEffect(() => { - create++; - console.log('foo create ' + create); - return () => { - cleanup++; - console.log('foo cleanup ' + cleanup); - }; - }); - return null; - } + return null; + } - const container = document.createElement('div'); - const root = ReactDOMClient.createRoot(container); - await act(() => { - root.render( - - - , - ); - }); - expect(create).toBe(__DEV__ ? 2 : 1); - expect(cleanup).toBe(__DEV__ ? 1 : 0); - expect(console.log).toBeCalledTimes(1); - // Note: we should display the first log because otherwise - // there is a risk of suppressing warnings when they happen, - // and on the next render they'd get deduplicated and ignored. - expect(console.log).toBeCalledWith('foo create 1'); + const container = document.createElement('div'); + const root = ReactDOMClient.createRoot(container); + await act(() => { + root.render( + + + , + ); }); - } + expect(create).toBe(__DEV__ ? 2 : 1); + expect(cleanup).toBe(__DEV__ ? 1 : 0); + expect(console.log).toBeCalledTimes(__DEV__ ? 3 : 1); + // Note: we should display the first log because otherwise + // there is a risk of suppressing warnings when they happen, + // and on the next render they'd get deduplicated and ignored. + expect(console.log).toBeCalledWith('foo create 1'); + if (__DEV__) { + expect(console.log).toBeCalledWith('foo cleanup 1'); + } + }); }); }); diff --git a/packages/shared/ReactFeatureFlags.js b/packages/shared/ReactFeatureFlags.js index 6ccf1ce8dd828..f8aadb9789406 100644 --- a/packages/shared/ReactFeatureFlags.js +++ b/packages/shared/ReactFeatureFlags.js @@ -297,6 +297,4 @@ export const enableUpdaterTracking = __PROFILE__; // Internal only. export const enableGetInspectorDataForInstanceInProduction = false; -export const consoleManagedByDevToolsDuringStrictMode = true; - export const enableDO_NOT_USE_disableStrictPassiveEffect = false; diff --git a/packages/shared/forks/ReactFeatureFlags.native-fb.js b/packages/shared/forks/ReactFeatureFlags.native-fb.js index 850249b8f79fd..37991d4c8ffbd 100644 --- a/packages/shared/forks/ReactFeatureFlags.native-fb.js +++ b/packages/shared/forks/ReactFeatureFlags.native-fb.js @@ -32,7 +32,6 @@ export const { } = dynamicFlags; // The rest of the flags are static for better dead code elimination. -export const consoleManagedByDevToolsDuringStrictMode = true; export const debugRenderPhaseSideEffectsForStrictMode = __DEV__; export const disableClientCache = true; export const disableCommentsAsDOMContainers = true; diff --git a/packages/shared/forks/ReactFeatureFlags.native-oss.js b/packages/shared/forks/ReactFeatureFlags.native-oss.js index 419cee942a9cc..eb40dc9123752 100644 --- a/packages/shared/forks/ReactFeatureFlags.native-oss.js +++ b/packages/shared/forks/ReactFeatureFlags.native-oss.js @@ -20,7 +20,6 @@ export const debugRenderPhaseSideEffectsForStrictMode = __DEV__; // All other flags // ----------------------------------------------------------------------------- export const alwaysThrottleRetries = false; -export const consoleManagedByDevToolsDuringStrictMode = true; export const disableClientCache = true; export const disableCommentsAsDOMContainers = true; export const disableDefaultPropsExceptForClasses = true; diff --git a/packages/shared/forks/ReactFeatureFlags.test-renderer.js b/packages/shared/forks/ReactFeatureFlags.test-renderer.js index c591794c20e83..6673d056d9a09 100644 --- a/packages/shared/forks/ReactFeatureFlags.test-renderer.js +++ b/packages/shared/forks/ReactFeatureFlags.test-renderer.js @@ -61,8 +61,6 @@ export const enableLazyContextPropagation = true; export const enableContextProfiling = false; export const enableLegacyHidden = false; -export const consoleManagedByDevToolsDuringStrictMode = false; - export const enableTransitionTracing = false; export const useModernStrictMode = false; diff --git a/packages/shared/forks/ReactFeatureFlags.test-renderer.native-fb.js b/packages/shared/forks/ReactFeatureFlags.test-renderer.native-fb.js index 7eef951628bba..3ac86950277d4 100644 --- a/packages/shared/forks/ReactFeatureFlags.test-renderer.native-fb.js +++ b/packages/shared/forks/ReactFeatureFlags.test-renderer.native-fb.js @@ -11,7 +11,6 @@ import typeof * as FeatureFlagsType from 'shared/ReactFeatureFlags'; import typeof * as ExportsType from './ReactFeatureFlags.test-renderer'; export const alwaysThrottleRetries = false; -export const consoleManagedByDevToolsDuringStrictMode = false; export const debugRenderPhaseSideEffectsForStrictMode = false; export const disableClientCache = true; export const disableCommentsAsDOMContainers = true; diff --git a/packages/shared/forks/ReactFeatureFlags.test-renderer.www.js b/packages/shared/forks/ReactFeatureFlags.test-renderer.www.js index 2ca5c62988aa8..8ebb4d0dbf8db 100644 --- a/packages/shared/forks/ReactFeatureFlags.test-renderer.www.js +++ b/packages/shared/forks/ReactFeatureFlags.test-renderer.www.js @@ -63,8 +63,6 @@ export const enableLazyContextPropagation = true; export const enableContextProfiling = false; export const enableLegacyHidden = false; -export const consoleManagedByDevToolsDuringStrictMode = false; - export const enableTransitionTracing = false; export const useModernStrictMode = false; diff --git a/packages/shared/forks/ReactFeatureFlags.www.js b/packages/shared/forks/ReactFeatureFlags.www.js index 40e996b571a96..7a5761932c820 100644 --- a/packages/shared/forks/ReactFeatureFlags.www.js +++ b/packages/shared/forks/ReactFeatureFlags.www.js @@ -106,8 +106,6 @@ export const enableComponentStackLocations = true; export const disableTextareaChildren = __EXPERIMENTAL__; -export const consoleManagedByDevToolsDuringStrictMode = true; - export const enableFizzExternalRuntime = true; export const passChildrenWhenCloningPersistedNodes = false; From 17ca4b157fcba6c734583513353ba72376a7ba2d Mon Sep 17 00:00:00 2001 From: Jack Pope Date: Fri, 13 Dec 2024 11:26:44 -0500 Subject: [PATCH 2/3] Fix useResourceEffect in Fizz (#31758) We're seeing errors when testing useResourceEffect in SSR and it turns out we're missing the noop dispatcher function on Fizz. I tested a local build with this change and it resolved the late mutation errors in the e2e tests. --- .../ReactDOMServerIntegrationHooks-test.js | 48 +++++++++++++++++++ packages/react-server/src/ReactFizzHooks.js | 6 +++ 2 files changed, 54 insertions(+) diff --git a/packages/react-dom/src/__tests__/ReactDOMServerIntegrationHooks-test.js b/packages/react-dom/src/__tests__/ReactDOMServerIntegrationHooks-test.js index e1aeea70fd806..b79e59ad004dd 100644 --- a/packages/react-dom/src/__tests__/ReactDOMServerIntegrationHooks-test.js +++ b/packages/react-dom/src/__tests__/ReactDOMServerIntegrationHooks-test.js @@ -27,6 +27,7 @@ let useRef; let useImperativeHandle; let useInsertionEffect; let useLayoutEffect; +let useResourceEffect; let useDebugValue; let forwardRef; let yieldedValues; @@ -51,6 +52,7 @@ function initModules() { useImperativeHandle = React.useImperativeHandle; useInsertionEffect = React.useInsertionEffect; useLayoutEffect = React.useLayoutEffect; + useResourceEffect = React.experimental_useResourceEffect; forwardRef = React.forwardRef; yieldedValues = []; @@ -653,6 +655,52 @@ describe('ReactDOMServerHooks', () => { }); }); + describe('useResourceEffect', () => { + gate(flags => { + if (flags.enableUseResourceEffectHook) { + const yields = []; + itRenders( + 'should ignore resource effects on the server', + async render => { + function Counter(props) { + useResourceEffect( + () => { + yieldValue('created on client'); + return {resource_counter: props.count}; + }, + [props.count], + resource => { + resource.resource_counter = props.count; + yieldValue('updated on client'); + }, + [props.count], + () => { + yieldValue('cleanup on client'); + }, + ); + return ; + } + + const domNode = await render(); + yields.push(clearLog()); + expect(domNode.tagName).toEqual('SPAN'); + expect(domNode.textContent).toEqual('Count: 0'); + }, + ); + + it('verifies yields in order', () => { + expect(yields).toEqual([ + ['Count: 0'], // server render + ['Count: 0'], // server stream + ['Count: 0', 'created on client'], // clean render + ['Count: 0', 'created on client'], // hydrated render + // nothing yielded for bad markup + ]); + }); + } + }); + }); + describe('useContext', () => { itThrowsWhenRendering( 'if used inside a class component', diff --git a/packages/react-server/src/ReactFizzHooks.js b/packages/react-server/src/ReactFizzHooks.js index 78e114e373e6b..84b9448ca32f6 100644 --- a/packages/react-server/src/ReactFizzHooks.js +++ b/packages/react-server/src/ReactFizzHooks.js @@ -43,6 +43,7 @@ import { enableUseEffectEventHook, enableUseMemoCacheHook, enableAsyncActions, + enableUseResourceEffectHook, } from 'shared/ReactFeatureFlags'; import is from 'shared/objectIs'; import { @@ -870,6 +871,11 @@ if (enableAsyncActions) { HooksDispatcher.useFormState = useActionState; HooksDispatcher.useActionState = useActionState; } +if (enableUseResourceEffectHook) { + HooksDispatcher.useResourceEffect = supportsClientAPIs + ? noop + : clientHookNotSupported; +} export let currentResumableState: null | ResumableState = (null: any); export function setCurrentResumableState( From fb12845d779667b35cc7f44eee6bea47f4db72ba Mon Sep 17 00:00:00 2001 From: Ricky Date: Fri, 13 Dec 2024 12:26:40 -0500 Subject: [PATCH 3/3] Remove disableIEWorkarounds (#31756) Based off https://github.com/facebook/react/pull/31755 This is landed everywhere. --- .../src/client/ReactDOMComponent.js | 60 ++++---------- .../src/client/setInnerHTML.js | 82 ------------------- .../__tests__/dangerouslySetInnerHTML-test.js | 69 ---------------- .../__tests__/trustedTypes-test.internal.js | 50 ----------- packages/shared/ReactFeatureFlags.js | 4 - .../forks/ReactFeatureFlags.native-fb.js | 1 - .../forks/ReactFeatureFlags.native-oss.js | 1 - .../forks/ReactFeatureFlags.test-renderer.js | 1 - ...actFeatureFlags.test-renderer.native-fb.js | 1 - .../ReactFeatureFlags.test-renderer.www.js | 1 - .../shared/forks/ReactFeatureFlags.www.js | 1 - scripts/jest/setupTests.www.js | 1 - 12 files changed, 17 insertions(+), 255 deletions(-) delete mode 100644 packages/react-dom-bindings/src/client/setInnerHTML.js diff --git a/packages/react-dom-bindings/src/client/ReactDOMComponent.js b/packages/react-dom-bindings/src/client/ReactDOMComponent.js index 9f7b23026f758..ac95c91c596a8 100644 --- a/packages/react-dom-bindings/src/client/ReactDOMComponent.js +++ b/packages/react-dom-bindings/src/client/ReactDOMComponent.js @@ -16,7 +16,6 @@ import { possibleRegistrationNames, } from '../events/EventRegistry'; -import {canUseDOM} from 'shared/ExecutionEnvironment'; import {checkHtmlStringCoercion} from 'shared/CheckStringCoercion'; import {checkAttributeStringCoercion} from 'shared/CheckStringCoercion'; import {checkControlledValueProps} from '../shared/ReactControlledValuePropTypes'; @@ -50,7 +49,6 @@ import { } from './ReactDOMTextarea'; import {validateTextNesting} from './validateDOMNesting'; import {track} from './inputValueTracking'; -import setInnerHTML from './setInnerHTML'; import setTextContent from './setTextContent'; import { createDangerousStringForStyles, @@ -66,7 +64,6 @@ import {validateProperties as validateUnknownProperties} from '../shared/ReactDO import sanitizeURL from '../shared/sanitizeURL'; import { - disableIEWorkarounds, enableTrustedTypesIntegration, enableFilterEmptyStringAttributesDOM, } from 'shared/ReactFeatureFlags'; @@ -83,19 +80,8 @@ let didWarnFormActionTarget = false; let didWarnFormActionMethod = false; let didWarnForNewBooleanPropsWithEmptyValue: {[string]: boolean}; let didWarnPopoverTargetObject = false; -let canDiffStyleForHydrationWarning; if (__DEV__) { didWarnForNewBooleanPropsWithEmptyValue = {}; - // IE 11 parses & normalizes the style attribute as opposed to other - // browsers. It adds spaces and sorts the properties in some - // non-alphabetical order. Handling that would require sorting CSS - // properties in the client & server versions or applying - // `expectedStyle` to a temporary DOM node to read its `style` attribute - // normalized. Since it only affects IE, we're skipping style warnings - // in that browser completely in favor of doing all that work. - // See https://github.com/facebook/react/issues/11807 - canDiffStyleForHydrationWarning = - disableIEWorkarounds || (canUseDOM && !document.documentMode); } function validatePropertiesInDevelopment(type: string, props: any) { @@ -579,11 +565,7 @@ function setProp( 'Can only set one of `children` or `props.dangerouslySetInnerHTML`.', ); } - if (disableIEWorkarounds) { - domElement.innerHTML = nextHtml; - } else { - setInnerHTML(domElement, nextHtml); - } + domElement.innerHTML = nextHtml; } } break; @@ -939,11 +921,7 @@ function setPropOnCustomElement( 'Can only set one of `children` or `props.dangerouslySetInnerHTML`.', ); } - if (disableIEWorkarounds) { - domElement.innerHTML = nextHtml; - } else { - setInnerHTML(domElement, nextHtml); - } + domElement.innerHTML = nextHtml; } } break; @@ -1931,27 +1909,23 @@ function diffHydratedStyles( } return; } - if (canDiffStyleForHydrationWarning) { - // First we compare the string form and see if it's equivalent. - // This lets us bail out on anything that used to pass in this form. - // It also lets us compare anything that's not parsed by this browser. - const clientValue = createDangerousStringForStyles(value); - const serverValue = domElement.getAttribute('style'); + // First we compare the string form and see if it's equivalent. + // This lets us bail out on anything that used to pass in this form. + // It also lets us compare anything that's not parsed by this browser. + const clientValue = createDangerousStringForStyles(value); + const serverValue = domElement.getAttribute('style'); - if (serverValue === clientValue) { - return; - } - const normalizedClientValue = - normalizeMarkupForTextOrAttribute(clientValue); - const normalizedServerValue = - normalizeMarkupForTextOrAttribute(serverValue); - if (normalizedServerValue === normalizedClientValue) { - return; - } - - // Otherwise, we create the object from the DOM for the diff view. - serverDifferences.style = getStylesObjectFromElement(domElement); + if (serverValue === clientValue) { + return; + } + const normalizedClientValue = normalizeMarkupForTextOrAttribute(clientValue); + const normalizedServerValue = normalizeMarkupForTextOrAttribute(serverValue); + if (normalizedServerValue === normalizedClientValue) { + return; } + + // Otherwise, we create the object from the DOM for the diff view. + serverDifferences.style = getStylesObjectFromElement(domElement); } function hydrateAttribute( diff --git a/packages/react-dom-bindings/src/client/setInnerHTML.js b/packages/react-dom-bindings/src/client/setInnerHTML.js deleted file mode 100644 index 5f0d630fd38b5..0000000000000 --- a/packages/react-dom-bindings/src/client/setInnerHTML.js +++ /dev/null @@ -1,82 +0,0 @@ -/** - * Copyright (c) Meta Platforms, Inc. and affiliates. - * - * This source code is licensed under the MIT license found in the - * LICENSE file in the root directory of this source tree. - * - * @flow - */ - -/* globals MSApp */ - -import {SVG_NAMESPACE} from './DOMNamespaces'; -import {enableTrustedTypesIntegration} from 'shared/ReactFeatureFlags'; - -// SVG temp container for IE lacking innerHTML -let reusableSVGContainer: HTMLElement; - -function setInnerHTMLImpl( - node: Element, - html: {valueOf(): {toString(): string, ...}, ...}, -): void { - if (node.namespaceURI === SVG_NAMESPACE) { - if (__DEV__) { - if (enableTrustedTypesIntegration) { - // TODO: reconsider the text of this warning and when it should show - // before enabling the feature flag. - if (typeof trustedTypes !== 'undefined') { - console.error( - "Using 'dangerouslySetInnerHTML' in an svg element with " + - 'Trusted Types enabled in an Internet Explorer will cause ' + - 'the trusted value to be converted to string. Assigning string ' + - "to 'innerHTML' will throw an error if Trusted Types are enforced. " + - "You can try to wrap your svg element inside a div and use 'dangerouslySetInnerHTML' " + - 'on the enclosing div instead.', - ); - } - } - } - if (!('innerHTML' in node)) { - // IE does not have innerHTML for SVG nodes, so instead we inject the - // new markup in a temp node and then move the child nodes across into - // the target node - reusableSVGContainer = - reusableSVGContainer || document.createElement('div'); - reusableSVGContainer.innerHTML = - '' + html.valueOf().toString() + ''; - const svgNode = reusableSVGContainer.firstChild; - while (node.firstChild) { - node.removeChild(node.firstChild); - } - // $FlowFixMe[incompatible-use] - // $FlowFixMe[incompatible-type] - while (svgNode.firstChild) { - node.appendChild(svgNode.firstChild); - } - return; - } - } - node.innerHTML = (html: any); -} - -let setInnerHTML: ( - node: Element, - html: {valueOf(): {toString(): string, ...}, ...}, -) => void = setInnerHTMLImpl; -// $FlowFixMe[cannot-resolve-name] -if (typeof MSApp !== 'undefined' && MSApp.execUnsafeLocalFunction) { - /** - * Create a function which has 'unsafe' privileges (required by windows8 apps) - */ - setInnerHTML = function ( - node: Element, - html: {valueOf(): {toString(): string, ...}, ...}, - ): void { - // $FlowFixMe[cannot-resolve-name] - return MSApp.execUnsafeLocalFunction(function () { - return setInnerHTMLImpl(node, html); - }); - }; -} - -export default setInnerHTML; diff --git a/packages/react-dom/src/client/__tests__/dangerouslySetInnerHTML-test.js b/packages/react-dom/src/client/__tests__/dangerouslySetInnerHTML-test.js index 64b4c0b0f7c98..ccbd41c2f30ad 100644 --- a/packages/react-dom/src/client/__tests__/dangerouslySetInnerHTML-test.js +++ b/packages/react-dom/src/client/__tests__/dangerouslySetInnerHTML-test.js @@ -27,73 +27,4 @@ describe('dangerouslySetInnerHTML', () => { expect(container.firstChild.innerHTML).toBe('

Hello

'); }); }); - - describe('when the node does not have an innerHTML property', () => { - let innerHTMLDescriptor; - - // In some versions of IE (TODO: which ones?) SVG nodes don't have - // innerHTML. To simulate this, we will take it off the Element prototype - // and put it onto the HTMLDivElement prototype. We expect that the logic - // checks for existence of innerHTML on SVG, and if one doesn't exist, falls - // back to using appendChild and removeChild. - - beforeEach(() => { - innerHTMLDescriptor = Object.getOwnPropertyDescriptor( - Element.prototype, - 'innerHTML', - ); - delete Element.prototype.innerHTML; - Object.defineProperty( - HTMLDivElement.prototype, - 'innerHTML', - innerHTMLDescriptor, - ); - }); - - afterEach(() => { - delete HTMLDivElement.prototype.innerHTML; - Object.defineProperty( - Element.prototype, - 'innerHTML', - innerHTMLDescriptor, - ); - }); - - // @gate !disableIEWorkarounds - it('sets innerHTML on it', async () => { - const html = ''; - const container = document.createElementNS( - 'http://www.w3.org/2000/svg', - 'svg', - ); - const root = ReactDOMClient.createRoot(container); - await act(() => { - root.render(); - }); - const circle = container.firstChild.firstChild; - expect(circle.tagName).toBe('circle'); - }); - - // @gate !disableIEWorkarounds - it('clears previous children', async () => { - const firstHtml = ''; - const secondHtml = ''; - - const container = document.createElementNS( - 'http://www.w3.org/2000/svg', - 'svg', - ); - const root = ReactDOMClient.createRoot(container); - await act(() => { - root.render(); - }); - const rect = container.firstChild.firstChild; - expect(rect.tagName).toBe('rect'); - await act(() => { - root.render(); - }); - const circle = container.firstChild.firstChild; - expect(circle.tagName).toBe('circle'); - }); - }); }); diff --git a/packages/react-dom/src/client/__tests__/trustedTypes-test.internal.js b/packages/react-dom/src/client/__tests__/trustedTypes-test.internal.js index abe707badd5ee..923ee1f5d81a6 100644 --- a/packages/react-dom/src/client/__tests__/trustedTypes-test.internal.js +++ b/packages/react-dom/src/client/__tests__/trustedTypes-test.internal.js @@ -206,56 +206,6 @@ describe('when Trusted Types are available in global object', () => { } }); - describe('dangerouslySetInnerHTML in svg elements in Internet Explorer', () => { - let innerHTMLDescriptor; - - // simulate svg elements in Internet Explorer which don't have 'innerHTML' property - beforeEach(() => { - innerHTMLDescriptor = Object.getOwnPropertyDescriptor( - Element.prototype, - 'innerHTML', - ); - delete Element.prototype.innerHTML; - Object.defineProperty( - HTMLDivElement.prototype, - 'innerHTML', - innerHTMLDescriptor, - ); - }); - - afterEach(() => { - delete HTMLDivElement.prototype.innerHTML; - Object.defineProperty( - Element.prototype, - 'innerHTML', - innerHTMLDescriptor, - ); - }); - - // @gate !disableIEWorkarounds - it('should log a warning', async () => { - class Component extends React.Component { - render() { - return ; - } - } - const root = ReactDOMClient.createRoot(container); - await expect(async () => { - await act(() => { - root.render(); - }); - }).toErrorDev( - "Using 'dangerouslySetInnerHTML' in an svg element with " + - 'Trusted Types enabled in an Internet Explorer will cause ' + - 'the trusted value to be converted to string. Assigning string ' + - "to 'innerHTML' will throw an error if Trusted Types are enforced. " + - "You can try to wrap your svg element inside a div and use 'dangerouslySetInnerHTML' " + - 'on the enclosing div instead.', - ); - expect(container.innerHTML).toBe('unsafe html'); - }); - }); - it('should warn once when rendering script tag in jsx on client', async () => { const root = ReactDOMClient.createRoot(container); await expect(async () => { diff --git a/packages/shared/ReactFeatureFlags.js b/packages/shared/ReactFeatureFlags.js index f8aadb9789406..edaf4b2419136 100644 --- a/packages/shared/ReactFeatureFlags.js +++ b/packages/shared/ReactFeatureFlags.js @@ -203,10 +203,6 @@ export const disableLegacyContextForFunctionComponents = true; // TODO: clean up legacy once tests pass WWW. export const useModernStrictMode = true; -// Not ready to break experimental yet. -// Remove IE and MsApp specific workarounds for innerHTML -export const disableIEWorkarounds = true; - // Filter certain DOM attributes (e.g. src, href) if their values are empty // strings. This prevents e.g. from making an unnecessary HTTP // request for certain browsers. diff --git a/packages/shared/forks/ReactFeatureFlags.native-fb.js b/packages/shared/forks/ReactFeatureFlags.native-fb.js index 37991d4c8ffbd..1ecbcfe28f5ce 100644 --- a/packages/shared/forks/ReactFeatureFlags.native-fb.js +++ b/packages/shared/forks/ReactFeatureFlags.native-fb.js @@ -36,7 +36,6 @@ export const debugRenderPhaseSideEffectsForStrictMode = __DEV__; export const disableClientCache = true; export const disableCommentsAsDOMContainers = true; export const disableDefaultPropsExceptForClasses = true; -export const disableIEWorkarounds = true; export const disableInputAttributeSyncing = false; export const disableLegacyContext = false; export const disableLegacyContextForFunctionComponents = false; diff --git a/packages/shared/forks/ReactFeatureFlags.native-oss.js b/packages/shared/forks/ReactFeatureFlags.native-oss.js index eb40dc9123752..0d97b0b68d263 100644 --- a/packages/shared/forks/ReactFeatureFlags.native-oss.js +++ b/packages/shared/forks/ReactFeatureFlags.native-oss.js @@ -23,7 +23,6 @@ export const alwaysThrottleRetries = false; export const disableClientCache = true; export const disableCommentsAsDOMContainers = true; export const disableDefaultPropsExceptForClasses = true; -export const disableIEWorkarounds = true; export const disableInputAttributeSyncing = false; export const disableLegacyContext = true; export const disableLegacyContextForFunctionComponents = true; diff --git a/packages/shared/forks/ReactFeatureFlags.test-renderer.js b/packages/shared/forks/ReactFeatureFlags.test-renderer.js index 6673d056d9a09..28f96fb84e6bc 100644 --- a/packages/shared/forks/ReactFeatureFlags.test-renderer.js +++ b/packages/shared/forks/ReactFeatureFlags.test-renderer.js @@ -29,7 +29,6 @@ export const enablePostpone = false; export const enableHalt = false; export const disableCommentsAsDOMContainers = true; export const disableInputAttributeSyncing = false; -export const disableIEWorkarounds = true; export const enableScopeAPI = false; export const enableCreateEventHandleAPI = false; export const enableSuspenseCallback = false; diff --git a/packages/shared/forks/ReactFeatureFlags.test-renderer.native-fb.js b/packages/shared/forks/ReactFeatureFlags.test-renderer.native-fb.js index 3ac86950277d4..fe3e2bd4813b8 100644 --- a/packages/shared/forks/ReactFeatureFlags.test-renderer.native-fb.js +++ b/packages/shared/forks/ReactFeatureFlags.test-renderer.native-fb.js @@ -15,7 +15,6 @@ export const debugRenderPhaseSideEffectsForStrictMode = false; export const disableClientCache = true; export const disableCommentsAsDOMContainers = true; export const disableDefaultPropsExceptForClasses = true; -export const disableIEWorkarounds = true; export const disableInputAttributeSyncing = false; export const disableLegacyContext = false; export const disableLegacyContextForFunctionComponents = false; diff --git a/packages/shared/forks/ReactFeatureFlags.test-renderer.www.js b/packages/shared/forks/ReactFeatureFlags.test-renderer.www.js index 8ebb4d0dbf8db..1badea3abf309 100644 --- a/packages/shared/forks/ReactFeatureFlags.test-renderer.www.js +++ b/packages/shared/forks/ReactFeatureFlags.test-renderer.www.js @@ -29,7 +29,6 @@ export const enablePostpone = false; export const enableHalt = false; export const disableCommentsAsDOMContainers = true; export const disableInputAttributeSyncing = false; -export const disableIEWorkarounds = true; export const enableScopeAPI = true; export const enableCreateEventHandleAPI = false; export const enableSuspenseCallback = true; diff --git a/packages/shared/forks/ReactFeatureFlags.www.js b/packages/shared/forks/ReactFeatureFlags.www.js index 7a5761932c820..2a432305635d3 100644 --- a/packages/shared/forks/ReactFeatureFlags.www.js +++ b/packages/shared/forks/ReactFeatureFlags.www.js @@ -53,7 +53,6 @@ export const enableFabricCompleteRootInCommitPhase = false; export const enableSuspenseAvoidThisFallback = true; export const enableSuspenseAvoidThisFallbackFizz = false; -export const disableIEWorkarounds = true; export const enableCPUSuspense = true; export const enableUseMemoCacheHook = true; export const enableUseEffectEventHook = true; diff --git a/scripts/jest/setupTests.www.js b/scripts/jest/setupTests.www.js index ba0ee287fd4bd..efee213861ca6 100644 --- a/scripts/jest/setupTests.www.js +++ b/scripts/jest/setupTests.www.js @@ -15,7 +15,6 @@ jest.mock('shared/ReactFeatureFlags', () => { // These are hardcoded to true for the next release, // but still run the tests against both variants until // we remove the flag. - actual.disableIEWorkarounds = __VARIANT__; actual.disableClientCache = __VARIANT__; return actual;