From d8d2b6e89cdff27a1ac246c6e9e030c2cc8760e3 Mon Sep 17 00:00:00 2001 From: Dan Abramov Date: Wed, 1 Apr 2020 18:31:59 +0100 Subject: [PATCH] Disable module components dynamically for WWW (#18446) * Make disableModulePatternComponents dynamic for WWW * Run both flags and tests and respect the flag in SSR --- .../__tests__/ReactComponentLifeCycle-test.js | 108 +++++++++--------- .../__tests__/ReactCompositeComponent-test.js | 63 +++++++--- .../ReactCompositeComponentState-test.js | 96 ++++++++-------- .../ReactDOMServerIntegrationElements-test.js | 35 ++++-- .../ReactErrorBoundaries-test.internal.js | 70 ++++++------ ...eactLegacyErrorBoundaries-test.internal.js | 70 ++++++------ packages/react-dom/src/__tests__/refs-test.js | 44 +++---- .../src/server/ReactPartialRenderer.js | 47 +++++--- .../src/ReactFiberBeginWork.js | 28 +++++ .../src/__tests__/ReactHooks-test.internal.js | 56 ++++----- ...eactHooksWithNoopRenderer-test.internal.js | 70 ++++++------ .../ReactIncremental-test.internal.js | 82 ++++++------- ...tIncrementalErrorHandling-test.internal.js | 64 ++++++----- .../__tests__/ReactFreshIntegration-test.js | 68 +++++------ .../forks/ReactFeatureFlags.testing.www.js | 2 +- .../forks/ReactFeatureFlags.www-dynamic.js | 1 + .../shared/forks/ReactFeatureFlags.www.js | 3 +- scripts/jest/setupTests.www.js | 2 - 18 files changed, 506 insertions(+), 403 deletions(-) diff --git a/packages/react-dom/src/__tests__/ReactComponentLifeCycle-test.js b/packages/react-dom/src/__tests__/ReactComponentLifeCycle-test.js index 6b51eb6a12..bfe41c540b 100644 --- a/packages/react-dom/src/__tests__/ReactComponentLifeCycle-test.js +++ b/packages/react-dom/src/__tests__/ReactComponentLifeCycle-test.js @@ -983,64 +983,66 @@ describe('ReactComponentLifeCycle', () => { }); }); - it('calls effects on module-pattern component', function() { - const log = []; + if (!require('shared/ReactFeatureFlags').disableModulePatternComponents) { + it('calls effects on module-pattern component', function() { + 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}; - }, + 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, }; - } - Parent.childContextTypes = { - x: PropTypes.number, - }; - function Child(props, context) { - expect(context.x).toBe(2); - return
; - } - Child.contextTypes = { - x: PropTypes.number, - }; - const div = document.createElement('div'); - expect(() => - ReactDOM.render( c && log.push('ref')} />, div), - ).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.", - ); - ReactDOM.render( c && log.push('ref')} />, div); + const div = document.createElement('div'); + expect(() => + ReactDOM.render( c && log.push('ref')} />, div), + ).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.", + ); + ReactDOM.render( c && log.push('ref')} />, div); - 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', () => { class MyComponent extends React.Component { diff --git a/packages/react-dom/src/__tests__/ReactCompositeComponent-test.js b/packages/react-dom/src/__tests__/ReactCompositeComponent-test.js index 98396fb6eb..73984da6c4 100644 --- a/packages/react-dom/src/__tests__/ReactCompositeComponent-test.js +++ b/packages/react-dom/src/__tests__/ReactCompositeComponent-test.js @@ -108,26 +108,53 @@ describe('ReactCompositeComponent', () => { }; }); - it('should support module pattern components', () => { - function Child({test}) { - return { - render() { - return
{test}
; - }, - }; - } + if (require('shared/ReactFeatureFlags').disableModulePatternComponents) { + it('should not support module pattern components', () => { + function Child({test}) { + return { + render() { + return
{test}
; + }, + }; + } - const el = document.createElement('div'); - expect(() => ReactDOM.render(, el)).toErrorDev( - 'Warning: The component appears to be a function component that returns a class instance. ' + - 'Change Child 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. " + - '`Child.prototype = React.Component.prototype`. ' + - "Don't use an arrow function since it cannot be called with `new` by React.", - ); + const el = document.createElement('div'); + expect(() => { + expect(() => ReactDOM.render(, el)).toThrow( + 'Objects are not valid as a React child (found: object with keys {render}).', + ); + }).toErrorDev( + 'Warning: The component appears to be a function component that returns a class instance. ' + + 'Change Child 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. " + + '`Child.prototype = React.Component.prototype`. ' + + "Don't use an arrow function since it cannot be called with `new` by React.", + ); - expect(el.textContent).toBe('test'); - }); + expect(el.textContent).toBe(''); + }); + } else { + it('should support module pattern components', () => { + function Child({test}) { + return { + render() { + return
{test}
; + }, + }; + } + + const el = document.createElement('div'); + expect(() => ReactDOM.render(, el)).toErrorDev( + 'Warning: The component appears to be a function component that returns a class instance. ' + + 'Change Child 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. " + + '`Child.prototype = React.Component.prototype`. ' + + "Don't use an arrow function since it cannot be called with `new` by React.", + ); + + expect(el.textContent).toBe('test'); + }); + } it('should support rendering to different child types over time', () => { const instance = ReactTestUtils.renderIntoDocument(); diff --git a/packages/react-dom/src/__tests__/ReactCompositeComponentState-test.js b/packages/react-dom/src/__tests__/ReactCompositeComponentState-test.js index 70e15b769e..5708d67edb 100644 --- a/packages/react-dom/src/__tests__/ReactCompositeComponentState-test.js +++ b/packages/react-dom/src/__tests__/ReactCompositeComponentState-test.js @@ -459,55 +459,57 @@ describe('ReactCompositeComponent-state', () => { ]); }); - it('should support stateful module pattern components', () => { - function Child() { - return { - state: { - count: 123, - }, - render() { - return
{`count:${this.state.count}`}
; - }, + if (!require('shared/ReactFeatureFlags').disableModulePatternComponents) { + it('should support stateful module pattern components', () => { + function Child() { + return { + state: { + count: 123, + }, + render() { + return
{`count:${this.state.count}`}
; + }, + }; + } + + const el = document.createElement('div'); + expect(() => ReactDOM.render(, el)).toErrorDev( + 'Warning: The component appears to be a function component that returns a class instance. ' + + 'Change Child 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. " + + '`Child.prototype = React.Component.prototype`. ' + + "Don't use an arrow function since it cannot be called with `new` by React.", + ); + + expect(el.textContent).toBe('count:123'); + }); + + it('should support getDerivedStateFromProps for module pattern components', () => { + function Child() { + return { + state: { + count: 1, + }, + render() { + return
{`count:${this.state.count}`}
; + }, + }; + } + Child.getDerivedStateFromProps = (props, prevState) => { + return { + count: prevState.count + props.incrementBy, + }; }; - } - const el = document.createElement('div'); - expect(() => ReactDOM.render(, el)).toErrorDev( - 'Warning: The component appears to be a function component that returns a class instance. ' + - 'Change Child 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. " + - '`Child.prototype = React.Component.prototype`. ' + - "Don't use an arrow function since it cannot be called with `new` by React.", - ); + const el = document.createElement('div'); + ReactDOM.render(, el); + expect(el.textContent).toBe('count:1'); - expect(el.textContent).toBe('count:123'); - }); + ReactDOM.render(, el); + expect(el.textContent).toBe('count:3'); - it('should support getDerivedStateFromProps for module pattern components', () => { - function Child() { - return { - state: { - count: 1, - }, - render() { - return
{`count:${this.state.count}`}
; - }, - }; - } - Child.getDerivedStateFromProps = (props, prevState) => { - return { - count: prevState.count + props.incrementBy, - }; - }; - - const el = document.createElement('div'); - ReactDOM.render(, el); - expect(el.textContent).toBe('count:1'); - - ReactDOM.render(, el); - expect(el.textContent).toBe('count:3'); - - ReactDOM.render(, el); - expect(el.textContent).toBe('count:4'); - }); + ReactDOM.render(, el); + expect(el.textContent).toBe('count:4'); + }); + } }); diff --git a/packages/react-dom/src/__tests__/ReactDOMServerIntegrationElements-test.js b/packages/react-dom/src/__tests__/ReactDOMServerIntegrationElements-test.js index 5d56419a1a..383bcd03e5 100644 --- a/packages/react-dom/src/__tests__/ReactDOMServerIntegrationElements-test.js +++ b/packages/react-dom/src/__tests__/ReactDOMServerIntegrationElements-test.js @@ -644,16 +644,33 @@ describe('ReactDOMServerIntegration', () => { checkFooDiv(await render()); }); - itRenders('factory components', async render => { - const FactoryComponent = () => { - return { - render: function() { - return
foo
; - }, + if (require('shared/ReactFeatureFlags').disableModulePatternComponents) { + itThrowsWhenRendering( + 'factory components', + async render => { + const FactoryComponent = () => { + return { + render: function() { + return
foo
; + }, + }; + }; + await render(, 1); + }, + 'Objects are not valid as a React child (found: object with keys {render})', + ); + } else { + itRenders('factory components', async render => { + const FactoryComponent = () => { + return { + render: function() { + return
foo
; + }, + }; }; - }; - checkFooDiv(await render(, 1)); - }); + checkFooDiv(await render(, 1)); + }); + } }); describe('component hierarchies', function() { diff --git a/packages/react-dom/src/__tests__/ReactErrorBoundaries-test.internal.js b/packages/react-dom/src/__tests__/ReactErrorBoundaries-test.internal.js index 057a0e4ae8..7f5ff9efb2 100644 --- a/packages/react-dom/src/__tests__/ReactErrorBoundaries-test.internal.js +++ b/packages/react-dom/src/__tests__/ReactErrorBoundaries-test.internal.js @@ -782,43 +782,45 @@ describe('ReactErrorBoundaries', () => { expect(container.firstChild.textContent).toBe('Caught an error: Hello.'); }); - 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'); - }, + 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, }; - } - 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.", - ); + 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.'); - }); + 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-dom/src/__tests__/ReactLegacyErrorBoundaries-test.internal.js b/packages/react-dom/src/__tests__/ReactLegacyErrorBoundaries-test.internal.js index 1dff2312b4..25a7ab2452 100644 --- a/packages/react-dom/src/__tests__/ReactLegacyErrorBoundaries-test.internal.js +++ b/packages/react-dom/src/__tests__/ReactLegacyErrorBoundaries-test.internal.js @@ -814,42 +814,44 @@ describe('ReactLegacyErrorBoundaries', () => { expect(container.firstChild.textContent).toBe('Caught an error: Hello.'); }); - 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'); - }, + 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, }; - } - 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-dom/src/__tests__/refs-test.js b/packages/react-dom/src/__tests__/refs-test.js index 6d4bd943d4..972e454e39 100644 --- a/packages/react-dom/src/__tests__/refs-test.js +++ b/packages/react-dom/src/__tests__/refs-test.js @@ -156,29 +156,31 @@ describe('reactiverefs', () => { }); }); -describe('factory components', () => { - it('Should correctly get the ref', () => { - function Comp() { - return { - render() { - return
; - }, - }; - } +if (!require('shared/ReactFeatureFlags').disableModulePatternComponents) { + describe('factory components', () => { + it('Should correctly get the ref', () => { + function Comp() { + return { + render() { + return
; + }, + }; + } - let inst; - expect( - () => (inst = ReactTestUtils.renderIntoDocument()), - ).toErrorDev( - 'Warning: The component appears to be a function component that returns a class instance. ' + - 'Change Comp 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. " + - '`Comp.prototype = React.Component.prototype`. ' + - "Don't use an arrow function since it cannot be called with `new` by React.", - ); - expect(inst.refs.elemRef.tagName).toBe('DIV'); + let inst; + expect( + () => (inst = ReactTestUtils.renderIntoDocument()), + ).toErrorDev( + 'Warning: The component appears to be a function component that returns a class instance. ' + + 'Change Comp 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. " + + '`Comp.prototype = React.Component.prototype`. ' + + "Don't use an arrow function since it cannot be called with `new` by React.", + ); + expect(inst.refs.elemRef.tagName).toBe('DIV'); + }); }); -}); +} /** * Tests that when a ref hops around children, we can track that correctly. diff --git a/packages/react-dom/src/server/ReactPartialRenderer.js b/packages/react-dom/src/server/ReactPartialRenderer.js index 919d58a589..94b0ca3f96 100644 --- a/packages/react-dom/src/server/ReactPartialRenderer.js +++ b/packages/react-dom/src/server/ReactPartialRenderer.js @@ -20,6 +20,7 @@ import ReactSharedInternals from 'shared/ReactSharedInternals'; import { warnAboutDeprecatedLifecycles, disableLegacyContext, + disableModulePatternComponents, enableSuspenseServerRenderer, enableFundamentalAPI, enableDeprecatedFlareAPI, @@ -527,28 +528,38 @@ function resolve( inst = Component(element.props, publicContext, updater); inst = finishHooks(Component, element.props, inst, publicContext); - if (inst == null || inst.render == null) { + if (__DEV__) { + // Support for module components is deprecated and is removed behind a flag. + // Whether or not it would crash later, we want to show a good message in DEV first. + if (inst != null && inst.render != null) { + const componentName = getComponentName(Component) || 'Unknown'; + if (!didWarnAboutModulePatternComponent[componentName]) { + console.error( + 'The <%s /> component appears to be a function component that returns a class instance. ' + + 'Change %s 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. " + + "`%s.prototype = React.Component.prototype`. Don't use an arrow function since it " + + 'cannot be called with `new` by React.', + componentName, + componentName, + componentName, + ); + didWarnAboutModulePatternComponent[componentName] = true; + } + } + } + + // If the flag is on, everything is assumed to be a function component. + // Otherwise, we also do the unfortunate dynamic checks. + if ( + disableModulePatternComponents || + inst == null || + inst.render == null + ) { child = inst; validateRenderResult(child, Component); return; } - - if (__DEV__) { - const componentName = getComponentName(Component) || 'Unknown'; - if (!didWarnAboutModulePatternComponent[componentName]) { - console.error( - 'The <%s /> component appears to be a function component that returns a class instance. ' + - 'Change %s 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. " + - "`%s.prototype = React.Component.prototype`. Don't use an arrow function since it " + - 'cannot be called with `new` by React.', - componentName, - componentName, - componentName, - ); - didWarnAboutModulePatternComponent[componentName] = true; - } - } } inst.props = element.props; diff --git a/packages/react-reconciler/src/ReactFiberBeginWork.js b/packages/react-reconciler/src/ReactFiberBeginWork.js index 2c6472c60f..97d088d143 100644 --- a/packages/react-reconciler/src/ReactFiberBeginWork.js +++ b/packages/react-reconciler/src/ReactFiberBeginWork.js @@ -1374,7 +1374,35 @@ function mountIndeterminateComponent( // React DevTools reads this flag. workInProgress.effectTag |= PerformedWork; + if (__DEV__) { + // Support for module components is deprecated and is removed behind a flag. + // Whether or not it would crash later, we want to show a good message in DEV first. + if ( + typeof value === 'object' && + value !== null && + typeof value.render === 'function' && + value.$$typeof === undefined + ) { + const componentName = getComponentName(Component) || 'Unknown'; + if (!didWarnAboutModulePatternComponent[componentName]) { + console.error( + 'The <%s /> component appears to be a function component that returns a class instance. ' + + 'Change %s 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. " + + "`%s.prototype = React.Component.prototype`. Don't use an arrow function since it " + + 'cannot be called with `new` by React.', + componentName, + componentName, + componentName, + ); + didWarnAboutModulePatternComponent[componentName] = true; + } + } + } + if ( + // Run these checks in production only if the flag is off. + // Eventually we'll delete this branch altogether. !disableModulePatternComponents && typeof value === 'object' && value !== null && diff --git a/packages/react-reconciler/src/__tests__/ReactHooks-test.internal.js b/packages/react-reconciler/src/__tests__/ReactHooks-test.internal.js index 9c527fed58..dfd2bb48c8 100644 --- a/packages/react-reconciler/src/__tests__/ReactHooks-test.internal.js +++ b/packages/react-reconciler/src/__tests__/ReactHooks-test.internal.js @@ -1237,6 +1237,7 @@ describe('ReactHooks', () => { 'Context can only be read while React is rendering', ); }); + it('double-invokes components with Hooks in Strict Mode', () => { ReactFeatureFlags.debugRenderPhaseSideEffectsForStrictMode = true; @@ -1351,32 +1352,35 @@ describe('ReactHooks', () => { ); expect(renderCount).toBe(__DEV__ ? 2 : 1); - renderCount = 0; - expect(() => renderer.update()).toErrorDev( - 'Warning: The component appears to be a function component that returns a class instance. ' + - 'Change Factory 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. " + - '`Factory.prototype = React.Component.prototype`. ' + - "Don't use an arrow function since it cannot be called with `new` by React.", - ); - expect(renderCount).toBe(1); - renderCount = 0; - renderer.update(); - expect(renderCount).toBe(1); - renderCount = 0; - renderer.update( - - - , - ); - expect(renderCount).toBe(__DEV__ ? 2 : 1); // Treated like a class - renderCount = 0; - renderer.update( - - - , - ); - expect(renderCount).toBe(__DEV__ ? 2 : 1); // Treated like a class + if (!require('shared/ReactFeatureFlags').disableModulePatternComponents) { + renderCount = 0; + expect(() => renderer.update()).toErrorDev( + 'Warning: The component appears to be a function component that returns a class instance. ' + + 'Change Factory 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. " + + '`Factory.prototype = React.Component.prototype`. ' + + "Don't use an arrow function since it cannot be called with `new` by React.", + ); + expect(renderCount).toBe(1); + renderCount = 0; + renderer.update(); + expect(renderCount).toBe(1); + + renderCount = 0; + renderer.update( + + + , + ); + expect(renderCount).toBe(__DEV__ ? 2 : 1); // Treated like a class + renderCount = 0; + renderer.update( + + + , + ); + expect(renderCount).toBe(__DEV__ ? 2 : 1); // Treated like a class + } renderCount = 0; renderer.update(); diff --git a/packages/react-reconciler/src/__tests__/ReactHooksWithNoopRenderer-test.internal.js b/packages/react-reconciler/src/__tests__/ReactHooksWithNoopRenderer-test.internal.js index dec9258996..a04578381d 100644 --- a/packages/react-reconciler/src/__tests__/ReactHooksWithNoopRenderer-test.internal.js +++ b/packages/react-reconciler/src/__tests__/ReactHooksWithNoopRenderer-test.internal.js @@ -187,41 +187,43 @@ describe('ReactHooksWithNoopRenderer', () => { expect(Scheduler).toFlushAndYield([10]); }); - it('throws inside module-style components', () => { - function Counter() { - return { - render() { - const [count] = useState(0); - return ; - }, - }; - } - ReactNoop.render(); - expect(() => - expect(Scheduler).toFlushAndThrow( - 'Invalid hook call. Hooks can only be called inside of the body of a function component. This could happen ' + - 'for one of the following reasons:\n' + - '1. You might have mismatching versions of React and the renderer (such as React DOM)\n' + - '2. You might be breaking the Rules of Hooks\n' + - '3. You might have more than one copy of React in the same app\n' + - 'See https://fb.me/react-invalid-hook-call for tips about how to debug and fix this problem.', - ), - ).toErrorDev( - 'Warning: The component appears to be a function component that returns a class instance. ' + - 'Change Counter 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. " + - '`Counter.prototype = React.Component.prototype`. ' + - "Don't use an arrow function since it cannot be called with `new` by React.", - ); + if (!require('shared/ReactFeatureFlags').disableModulePatternComponents) { + it('throws inside module-style components', () => { + function Counter() { + return { + render() { + const [count] = useState(0); + return ; + }, + }; + } + ReactNoop.render(); + expect(() => + expect(Scheduler).toFlushAndThrow( + 'Invalid hook call. Hooks can only be called inside of the body of a function component. This could happen ' + + 'for one of the following reasons:\n' + + '1. You might have mismatching versions of React and the renderer (such as React DOM)\n' + + '2. You might be breaking the Rules of Hooks\n' + + '3. You might have more than one copy of React in the same app\n' + + 'See https://fb.me/react-invalid-hook-call for tips about how to debug and fix this problem.', + ), + ).toErrorDev( + 'Warning: The component appears to be a function component that returns a class instance. ' + + 'Change Counter 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. " + + '`Counter.prototype = React.Component.prototype`. ' + + "Don't use an arrow function since it cannot be called with `new` by React.", + ); - // Confirm that a subsequent hook works properly. - function GoodCounter(props) { - const [count] = useState(props.initialCount); - return ; - } - ReactNoop.render(); - expect(Scheduler).toFlushAndYield([10]); - }); + // Confirm that a subsequent hook works properly. + function GoodCounter(props) { + const [count] = useState(props.initialCount); + return ; + } + ReactNoop.render(); + expect(Scheduler).toFlushAndYield([10]); + }); + } it('throws when called outside the render phase', () => { expect(() => useState(0)).toThrow( diff --git a/packages/react-reconciler/src/__tests__/ReactIncremental-test.internal.js b/packages/react-reconciler/src/__tests__/ReactIncremental-test.internal.js index a5e1a0ace5..6da7c63472 100644 --- a/packages/react-reconciler/src/__tests__/ReactIncremental-test.internal.js +++ b/packages/react-reconciler/src/__tests__/ReactIncremental-test.internal.js @@ -2012,48 +2012,50 @@ describe('ReactIncremental', () => { ]); }); - it('does not leak own context into context provider (factory components)', () => { - const ops = []; - function Recurse(props, context) { - return { - getChildContext() { - return {n: (context.n || 3) - 1}; - }, - render() { - ops.push('Recurse ' + JSON.stringify(context)); - if (context.n === 0) { - return null; - } - return ; - }, + if (!require('shared/ReactFeatureFlags').disableModulePatternComponents) { + it('does not leak own context into context provider (factory components)', () => { + const ops = []; + function Recurse(props, context) { + return { + getChildContext() { + return {n: (context.n || 3) - 1}; + }, + render() { + ops.push('Recurse ' + JSON.stringify(context)); + if (context.n === 0) { + return null; + } + return ; + }, + }; + } + Recurse.contextTypes = { + n: PropTypes.number, + }; + Recurse.childContextTypes = { + n: PropTypes.number, }; - } - Recurse.contextTypes = { - n: PropTypes.number, - }; - Recurse.childContextTypes = { - n: PropTypes.number, - }; - ReactNoop.render(); - expect(() => expect(Scheduler).toFlushWithoutYielding()).toErrorDev([ - 'Warning: The component appears to be a function component that returns a class instance. ' + - 'Change Recurse 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. " + - '`Recurse.prototype = React.Component.prototype`. ' + - "Don't use an arrow function since it cannot be called with `new` by React.", - 'Legacy context API has been detected within a strict-mode tree.\n\n' + - 'The old API will be supported in all 16.x releases, but applications ' + - 'using it should migrate to the new version.\n\n' + - 'Please update the following components: Recurse', - ]); - expect(ops).toEqual([ - 'Recurse {}', - 'Recurse {"n":2}', - 'Recurse {"n":1}', - 'Recurse {"n":0}', - ]); - }); + ReactNoop.render(); + expect(() => expect(Scheduler).toFlushWithoutYielding()).toErrorDev([ + 'Warning: The component appears to be a function component that returns a class instance. ' + + 'Change Recurse 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. " + + '`Recurse.prototype = React.Component.prototype`. ' + + "Don't use an arrow function since it cannot be called with `new` by React.", + 'Legacy context API has been detected within a strict-mode tree.\n\n' + + 'The old API will be supported in all 16.x releases, but applications ' + + 'using it should migrate to the new version.\n\n' + + 'Please update the following components: Recurse', + ]); + expect(ops).toEqual([ + 'Recurse {}', + 'Recurse {"n":2}', + 'Recurse {"n":1}', + 'Recurse {"n":0}', + ]); + }); + } it('provides context when reusing work', () => { class Intl extends React.Component { diff --git a/packages/react-reconciler/src/__tests__/ReactIncrementalErrorHandling-test.internal.js b/packages/react-reconciler/src/__tests__/ReactIncrementalErrorHandling-test.internal.js index e064861a03..0ed45b631d 100644 --- a/packages/react-reconciler/src/__tests__/ReactIncrementalErrorHandling-test.internal.js +++ b/packages/react-reconciler/src/__tests__/ReactIncrementalErrorHandling-test.internal.js @@ -1695,39 +1695,41 @@ describe('ReactIncrementalErrorHandling', () => { expect(ReactNoop.getChildren()).toEqual([span('Caught an error: Hello')]); }); - it('handles error thrown inside getDerivedStateFromProps of a module-style context provider', () => { - function Provider() { - return { - getChildContext() { - return {foo: 'bar'}; - }, - render() { - return 'Hi'; - }, + if (!require('shared/ReactFeatureFlags').disableModulePatternComponents) { + it('handles error thrown inside getDerivedStateFromProps of a module-style context provider', () => { + function Provider() { + return { + getChildContext() { + return {foo: 'bar'}; + }, + render() { + return 'Hi'; + }, + }; + } + Provider.childContextTypes = { + x: () => {}, + }; + Provider.getDerivedStateFromProps = () => { + throw new Error('Oops!'); }; - } - Provider.childContextTypes = { - x: () => {}, - }; - Provider.getDerivedStateFromProps = () => { - throw new Error('Oops!'); - }; - ReactNoop.render(); - expect(() => { - expect(Scheduler).toFlushAndThrow('Oops!'); - }).toErrorDev([ - 'Warning: The component appears to be a function component that returns a class instance. ' + - 'Change Provider 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. " + - '`Provider.prototype = React.Component.prototype`. ' + - "Don't use an arrow function since it cannot be called with `new` by React.", - 'Legacy context API has been detected within a strict-mode tree.\n\n' + - 'The old API will be supported in all 16.x releases, but ' + - 'applications using it should migrate to the new version.\n\n' + - 'Please update the following components: Provider', - ]); - }); + ReactNoop.render(); + expect(() => { + expect(Scheduler).toFlushAndThrow('Oops!'); + }).toErrorDev([ + 'Warning: The component appears to be a function component that returns a class instance. ' + + 'Change Provider 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. " + + '`Provider.prototype = React.Component.prototype`. ' + + "Don't use an arrow function since it cannot be called with `new` by React.", + 'Legacy context API has been detected within a strict-mode tree.\n\n' + + 'The old API will be supported in all 16.x releases, but ' + + 'applications using it should migrate to the new version.\n\n' + + 'Please update the following components: Provider', + ]); + }); + } it('uncaught errors should be discarded if the render is aborted', async () => { const root = ReactNoop.createRoot(); diff --git a/packages/react-refresh/src/__tests__/ReactFreshIntegration-test.js b/packages/react-refresh/src/__tests__/ReactFreshIntegration-test.js index a86ca14d98..2f7a8dbd06 100644 --- a/packages/react-refresh/src/__tests__/ReactFreshIntegration-test.js +++ b/packages/react-refresh/src/__tests__/ReactFreshIntegration-test.js @@ -1379,51 +1379,53 @@ describe('ReactFreshIntegration', () => { } }); - it('remounts deprecated factory components', () => { - if (__DEV__) { - expect(() => { - render(` + if (!require('shared/ReactFeatureFlags').disableModulePatternComponents) { + it('remounts deprecated factory components', () => { + if (__DEV__) { + expect(() => { + render(` + function Parent() { + return { + render() { + return ; + } + }; + }; + + function Child({prop}) { + return

{prop}1

; + }; + + export default Parent; + `); + }).toErrorDev( + 'The component appears to be a function component ' + + 'that returns a class instance.', + ); + const el = container.firstChild; + expect(el.textContent).toBe('A1'); + patch(` function Parent() { return { render() { - return ; + return ; } }; }; function Child({prop}) { - return

{prop}1

; + return

{prop}2

; }; export default Parent; `); - }).toErrorDev( - 'The component appears to be a function component ' + - 'that returns a class instance.', - ); - const el = container.firstChild; - expect(el.textContent).toBe('A1'); - patch(` - function Parent() { - return { - render() { - return ; - } - }; - }; - - function Child({prop}) { - return

{prop}2

; - }; - - export default Parent; - `); - // Like classes, factory components always remount. - expect(container.firstChild).not.toBe(el); - const newEl = container.firstChild; - expect(newEl.textContent).toBe('B2'); - } - }); + // Like classes, factory components always remount. + expect(container.firstChild).not.toBe(el); + const newEl = container.firstChild; + expect(newEl.textContent).toBe('B2'); + } + }); + } describe('with inline requires', () => { beforeEach(() => { diff --git a/packages/shared/forks/ReactFeatureFlags.testing.www.js b/packages/shared/forks/ReactFeatureFlags.testing.www.js index 20ca0f2951..7a18e2d910 100644 --- a/packages/shared/forks/ReactFeatureFlags.testing.www.js +++ b/packages/shared/forks/ReactFeatureFlags.testing.www.js @@ -36,7 +36,7 @@ export const disableLegacyContext = __EXPERIMENTAL__; export const disableSchedulerTimeoutBasedOnReactExpirationTime = false; export const enableTrustedTypesIntegration = false; export const disableTextareaChildren = __EXPERIMENTAL__; -export const disableModulePatternComponents = false; +export const disableModulePatternComponents = true; export const warnUnstableRenderSubtreeIntoContainer = false; export const deferPassiveEffectCleanupDuringUnmount = true; export const runAllPassiveEffectDestroysBeforeCreates = true; diff --git a/packages/shared/forks/ReactFeatureFlags.www-dynamic.js b/packages/shared/forks/ReactFeatureFlags.www-dynamic.js index a4b2d9d128..923372abb7 100644 --- a/packages/shared/forks/ReactFeatureFlags.www-dynamic.js +++ b/packages/shared/forks/ReactFeatureFlags.www-dynamic.js @@ -14,6 +14,7 @@ // with the __VARIANT__ set to `true`, and once set to `false`. export const warnAboutSpreadingKeyToJSX = __VARIANT__; +export const disableModulePatternComponents = __VARIANT__; // These are already tested in both modes using the build type dimension, // so we don't need to use __VARIANT__ to get extra coverage. diff --git a/packages/shared/forks/ReactFeatureFlags.www.js b/packages/shared/forks/ReactFeatureFlags.www.js index 07ced9fce3..b281c14d25 100644 --- a/packages/shared/forks/ReactFeatureFlags.www.js +++ b/packages/shared/forks/ReactFeatureFlags.www.js @@ -16,6 +16,7 @@ const dynamicFeatureFlags: DynamicFeatureFlags = require('ReactFeatureFlags'); export const { debugRenderPhaseSideEffectsForStrictMode, + disableModulePatternComponents, disableInputAttributeSyncing, enableTrustedTypesIntegration, warnAboutShorthandPropertyCollision, @@ -65,8 +66,6 @@ export const flushSuspenseFallbacksInTests = true; export const disableTextareaChildren = __EXPERIMENTAL__; -export const disableModulePatternComponents = __EXPERIMENTAL__; - export const warnUnstableRenderSubtreeIntoContainer = false; export const enableLegacyFBSupport = !__EXPERIMENTAL__; diff --git a/scripts/jest/setupTests.www.js b/scripts/jest/setupTests.www.js index 73a3f87694..16aca49d6f 100644 --- a/scripts/jest/setupTests.www.js +++ b/scripts/jest/setupTests.www.js @@ -17,8 +17,6 @@ jest.mock('shared/ReactFeatureFlags', () => { wwwFlags.warnAboutUnmockedScheduler = defaultFlags.warnAboutUnmockedScheduler; wwwFlags.disableJavaScriptURLs = defaultFlags.disableJavaScriptURLs; wwwFlags.enableDeprecatedFlareAPI = defaultFlags.enableDeprecatedFlareAPI; - wwwFlags.disableModulePatternComponents = - defaultFlags.disableModulePatternComponents; return wwwFlags; });