From afc95621b0a4975f9515444108d5196e542cfc01 Mon Sep 17 00:00:00 2001 From: Ricky Hanlon Date: Mon, 29 Jan 2024 15:16:40 -0500 Subject: [PATCH 1/3] Move flags to experimental --- .../__tests__/ReactComponentLifeCycle-test.js | 111 +++++++++--------- ...eactLegacyErrorBoundaries-test.internal.js | 72 ++++++------ ...tIncrementalErrorHandling-test.internal.js | 2 +- packages/shared/ReactFeatureFlags.js | 4 +- 4 files changed, 93 insertions(+), 96 deletions(-) diff --git a/packages/react-dom/src/__tests__/ReactComponentLifeCycle-test.js b/packages/react-dom/src/__tests__/ReactComponentLifeCycle-test.js index a05e7bd16fc28..3843dd3f8d04a 100644 --- a/packages/react-dom/src/__tests__/ReactComponentLifeCycle-test.js +++ b/packages/react-dom/src/__tests__/ReactComponentLifeCycle-test.js @@ -1098,70 +1098,69 @@ describe('ReactComponentLifeCycle', () => { }); }); - if (!require('shared/ReactFeatureFlags').disableModulePatternComponents) { - it('calls effects on module-pattern component', async () => { - const log = []; + // @gate !disableModulePatternComponents && !disableLegacyContext + it('calls effects on module-pattern component', async () => { + const log = []; - function Parent() { - return { - render() { - expect(typeof this.props).toBe('object'); - log.push('render'); - return ; - }, - UNSAFE_componentWillMount() { - log.push('will mount'); - }, - componentDidMount() { - log.push('did mount'); - }, - componentDidUpdate() { - log.push('did update'); - }, - getChildContext() { - return {x: 2}; - }, - }; - } - Parent.childContextTypes = { - x: PropTypes.number, - }; - function Child(props, context) { - expect(context.x).toBe(2); - return
; - } - Child.contextTypes = { - x: PropTypes.number, + function Parent() { + return { + render() { + expect(typeof this.props).toBe('object'); + log.push('render'); + return ; + }, + UNSAFE_componentWillMount() { + log.push('will mount'); + }, + componentDidMount() { + log.push('did mount'); + }, + componentDidUpdate() { + log.push('did update'); + }, + getChildContext() { + return {x: 2}; + }, }; + } + Parent.childContextTypes = { + x: PropTypes.number, + }; + function Child(props, context) { + expect(context.x).toBe(2); + return
; + } + Child.contextTypes = { + x: PropTypes.number, + }; - const root = ReactDOMClient.createRoot(document.createElement('div')); - await expect(async () => { - await act(() => { - root.render( c && log.push('ref')} />); - }); - }).toErrorDev( - 'Warning: The component appears to be a function component that returns a class instance. ' + - 'Change Parent to a class that extends React.Component instead. ' + - "If you can't use a class try assigning the prototype on the function as a workaround. " + - '`Parent.prototype = React.Component.prototype`. ' + - "Don't use an arrow function since it cannot be called with `new` by React.", - ); + const root = ReactDOMClient.createRoot(document.createElement('div')); + await expect(async () => { await act(() => { root.render( c && log.push('ref')} />); }); + }).toErrorDev( + 'Warning: The component appears to be a function component that returns a class instance. ' + + 'Change Parent to a class that extends React.Component instead. ' + + "If you can't use a class try assigning the prototype on the function as a workaround. " + + '`Parent.prototype = React.Component.prototype`. ' + + "Don't use an arrow function since it cannot be called with `new` by React.", + ); + await act(() => { + root.render( c && log.push('ref')} />); + }); - expect(log).toEqual([ - 'will mount', - 'render', - 'did mount', - 'ref', + expect(log).toEqual([ + 'will mount', + 'render', + 'did mount', + 'ref', - 'render', - 'did update', - 'ref', - ]); - }); - } + 'render', + 'did update', + 'ref', + ]); + }); it('should warn if getDerivedStateFromProps returns undefined', async () => { class MyComponent extends React.Component { diff --git a/packages/react-dom/src/__tests__/ReactLegacyErrorBoundaries-test.internal.js b/packages/react-dom/src/__tests__/ReactLegacyErrorBoundaries-test.internal.js index bcdf2b796134e..ba1d7d02fda91 100644 --- a/packages/react-dom/src/__tests__/ReactLegacyErrorBoundaries-test.internal.js +++ b/packages/react-dom/src/__tests__/ReactLegacyErrorBoundaries-test.internal.js @@ -824,45 +824,43 @@ describe('ReactLegacyErrorBoundaries', () => { ); expect(container.firstChild.textContent).toBe('Caught an error: Hello.'); }); - - if (!require('shared/ReactFeatureFlags').disableModulePatternComponents) { - it('renders an error state if module-style context provider throws in componentWillMount', () => { - function BrokenComponentWillMountWithContext() { - return { - getChildContext() { - return {foo: 42}; - }, - render() { - return
{this.props.children}
; - }, - UNSAFE_componentWillMount() { - throw new Error('Hello'); - }, - }; - } - BrokenComponentWillMountWithContext.childContextTypes = { - foo: PropTypes.number, + // @gate !disableModulePatternComponents && !disableLegacyContext + it('renders an error state if module-style context provider throws in componentWillMount', () => { + function BrokenComponentWillMountWithContext() { + return { + getChildContext() { + return {foo: 42}; + }, + render() { + return
{this.props.children}
; + }, + UNSAFE_componentWillMount() { + throw new Error('Hello'); + }, }; + } + BrokenComponentWillMountWithContext.childContextTypes = { + foo: PropTypes.number, + }; - const container = document.createElement('div'); - expect(() => - ReactDOM.render( - - - , - container, - ), - ).toErrorDev( - 'Warning: The component appears to be a function component that ' + - 'returns a class instance. ' + - 'Change BrokenComponentWillMountWithContext to a class that extends React.Component instead. ' + - "If you can't use a class try assigning the prototype on the function as a workaround. " + - '`BrokenComponentWillMountWithContext.prototype = React.Component.prototype`. ' + - "Don't use an arrow function since it cannot be called with `new` by React.", - ); - expect(container.firstChild.textContent).toBe('Caught an error: Hello.'); - }); - } + const container = document.createElement('div'); + expect(() => + ReactDOM.render( + + + , + container, + ), + ).toErrorDev( + 'Warning: The component appears to be a function component that ' + + 'returns a class instance. ' + + 'Change BrokenComponentWillMountWithContext to a class that extends React.Component instead. ' + + "If you can't use a class try assigning the prototype on the function as a workaround. " + + '`BrokenComponentWillMountWithContext.prototype = React.Component.prototype`. ' + + "Don't use an arrow function since it cannot be called with `new` by React.", + ); + expect(container.firstChild.textContent).toBe('Caught an error: Hello.'); + }); it('mounts the error message if mounting fails', () => { function renderError(error) { diff --git a/packages/react-reconciler/src/__tests__/ReactIncrementalErrorHandling-test.internal.js b/packages/react-reconciler/src/__tests__/ReactIncrementalErrorHandling-test.internal.js index d60e0b5426401..5dea7f6d448c3 100644 --- a/packages/react-reconciler/src/__tests__/ReactIncrementalErrorHandling-test.internal.js +++ b/packages/react-reconciler/src/__tests__/ReactIncrementalErrorHandling-test.internal.js @@ -1756,7 +1756,7 @@ describe('ReactIncrementalErrorHandling', () => { ); }); - // @gate !disableModulePatternComponents + // @gate !disableModulePatternComponents && !disableLegacyContext it('handles error thrown inside getDerivedStateFromProps of a module-style context provider', async () => { function Provider() { return { diff --git a/packages/shared/ReactFeatureFlags.js b/packages/shared/ReactFeatureFlags.js index 67862a4f14288..ba4dd1f6b82c6 100644 --- a/packages/shared/ReactFeatureFlags.js +++ b/packages/shared/ReactFeatureFlags.js @@ -137,11 +137,11 @@ export const transitionLaneExpirationMs = 5000; const __NEXT_MAJOR__ = __EXPERIMENTAL__; // Not ready to break experimental yet. -export const disableLegacyContext = false; +export const disableLegacyContext = __NEXT_MAJOR__; // Not ready to break experimental yet. // Disable javascript: URL strings in href for XSS protection. -export const disableJavaScriptURLs = false; +export const disableJavaScriptURLs = __NEXT_MAJOR__; // Not ready to break experimental yet. // Modern behaviour aligns more with what components From c1b9f9cdf4164dcc6fb83eaa6d3e2a1526323006 Mon Sep 17 00:00:00 2001 From: Ricky Hanlon Date: Tue, 6 Feb 2024 22:05:43 -0500 Subject: [PATCH 2/3] Fix gates --- .../src/__tests__/ReactErrorBoundaries-test.internal.js | 5 ++--- .../__tests__/ReactIncrementalErrorHandling-test.internal.js | 2 +- 2 files changed, 3 insertions(+), 4 deletions(-) diff --git a/packages/react-dom/src/__tests__/ReactErrorBoundaries-test.internal.js b/packages/react-dom/src/__tests__/ReactErrorBoundaries-test.internal.js index 403a3de7b6b17..dd970a4222769 100644 --- a/packages/react-dom/src/__tests__/ReactErrorBoundaries-test.internal.js +++ b/packages/react-dom/src/__tests__/ReactErrorBoundaries-test.internal.js @@ -882,9 +882,8 @@ describe('ReactErrorBoundaries', () => { expect(container.firstChild.textContent).toBe('Caught an error: Hello.'); }); - // @gate !disableModulePatternComponents - // @gate !disableLegacyContext - it('renders an error state if module-style context provider throws in componentWillMount', async () => { + // @gate !(disableModulePatternComponents && disableLegacyContext) + it('renders an error state if module-style context provider throws in componentWillMount', () => { function BrokenComponentWillMountWithContext() { return { getChildContext() { diff --git a/packages/react-reconciler/src/__tests__/ReactIncrementalErrorHandling-test.internal.js b/packages/react-reconciler/src/__tests__/ReactIncrementalErrorHandling-test.internal.js index 5dea7f6d448c3..31f62cc7c9d9d 100644 --- a/packages/react-reconciler/src/__tests__/ReactIncrementalErrorHandling-test.internal.js +++ b/packages/react-reconciler/src/__tests__/ReactIncrementalErrorHandling-test.internal.js @@ -1756,7 +1756,7 @@ describe('ReactIncrementalErrorHandling', () => { ); }); - // @gate !disableModulePatternComponents && !disableLegacyContext + // @gate !(disableModulePatternComponents && disableLegacyContext) it('handles error thrown inside getDerivedStateFromProps of a module-style context provider', async () => { function Provider() { return { From 5405fae919212d50d25dfcdb87d1a8990d48bfe4 Mon Sep 17 00:00:00 2001 From: Ricky Hanlon Date: Fri, 9 Feb 2024 15:13:23 -0500 Subject: [PATCH 3/3] Fix gates for real this time --- .../ReactErrorBoundaries-test.internal.js | 16 ++++++++++++++-- .../ReactLegacyErrorBoundaries-test.internal.js | 3 ++- ...eactIncrementalErrorHandling-test.internal.js | 3 ++- 3 files changed, 18 insertions(+), 4 deletions(-) diff --git a/packages/react-dom/src/__tests__/ReactErrorBoundaries-test.internal.js b/packages/react-dom/src/__tests__/ReactErrorBoundaries-test.internal.js index dd970a4222769..8ffc6d1da57a3 100644 --- a/packages/react-dom/src/__tests__/ReactErrorBoundaries-test.internal.js +++ b/packages/react-dom/src/__tests__/ReactErrorBoundaries-test.internal.js @@ -882,8 +882,20 @@ describe('ReactErrorBoundaries', () => { expect(container.firstChild.textContent).toBe('Caught an error: Hello.'); }); - // @gate !(disableModulePatternComponents && disableLegacyContext) - it('renders an error state if module-style context provider throws in componentWillMount', () => { + // @gate !disableModulePatternComponents + // @gate !disableLegacyContext || !__DEV__ + it('renders an error state if module-style context provider throws in componentWillMount', async () => { + console.log( + gate(flags => { + return { + disableModulePatternComponents: flags.disableModulePatternComponents, + disableLegacyContext: flags.disableLegacyContext, + val: !( + flags.disableModulePatternComponents && flags.disableLegacyContext + ), + }; + }), + ); function BrokenComponentWillMountWithContext() { return { getChildContext() { diff --git a/packages/react-dom/src/__tests__/ReactLegacyErrorBoundaries-test.internal.js b/packages/react-dom/src/__tests__/ReactLegacyErrorBoundaries-test.internal.js index ba1d7d02fda91..0858482fa8f62 100644 --- a/packages/react-dom/src/__tests__/ReactLegacyErrorBoundaries-test.internal.js +++ b/packages/react-dom/src/__tests__/ReactLegacyErrorBoundaries-test.internal.js @@ -824,7 +824,8 @@ describe('ReactLegacyErrorBoundaries', () => { ); expect(container.firstChild.textContent).toBe('Caught an error: Hello.'); }); - // @gate !disableModulePatternComponents && !disableLegacyContext + // @gate !disableModulePatternComponents + // @gate !disableLegacyContext || !__DEV__ it('renders an error state if module-style context provider throws in componentWillMount', () => { function BrokenComponentWillMountWithContext() { return { diff --git a/packages/react-reconciler/src/__tests__/ReactIncrementalErrorHandling-test.internal.js b/packages/react-reconciler/src/__tests__/ReactIncrementalErrorHandling-test.internal.js index 31f62cc7c9d9d..0f5bbb4cea38b 100644 --- a/packages/react-reconciler/src/__tests__/ReactIncrementalErrorHandling-test.internal.js +++ b/packages/react-reconciler/src/__tests__/ReactIncrementalErrorHandling-test.internal.js @@ -1756,7 +1756,8 @@ describe('ReactIncrementalErrorHandling', () => { ); }); - // @gate !(disableModulePatternComponents && disableLegacyContext) + // @gate !disableModulePatternComponents + // @gate !disableLegacyContext || !__DEV__ it('handles error thrown inside getDerivedStateFromProps of a module-style context provider', async () => { function Provider() { return {