From e327e4afcf928e84c548b9e56db9640e7615cba2 Mon Sep 17 00:00:00 2001 From: Dominic Gannaway Date: Tue, 21 Jan 2020 13:41:35 +0000 Subject: [PATCH 1/2] Add unstable_renderSubtreeIntoContainer and unstable_createPortal feature flags Add flags to other files address feedback Fix The ReactDOM.unstable_createPortal() Trigger build Make message same as before --- .../src/__tests__/ReactDOMFiber-test.js | 48 +- .../renderSubtreeIntoContainer-test.js | 564 +++++++++--------- packages/react-dom/src/client/ReactDOM.js | 60 +- packages/shared/ReactFeatureFlags.js | 6 + .../forks/ReactFeatureFlags.native-fb.js | 2 + .../forks/ReactFeatureFlags.native-oss.js | 2 + .../forks/ReactFeatureFlags.persistent.js | 2 + .../forks/ReactFeatureFlags.test-renderer.js | 2 + .../ReactFeatureFlags.test-renderer.www.js | 2 + .../shared/forks/ReactFeatureFlags.www.js | 4 + 10 files changed, 376 insertions(+), 316 deletions(-) diff --git a/packages/react-dom/src/__tests__/ReactDOMFiber-test.js b/packages/react-dom/src/__tests__/ReactDOMFiber-test.js index a9856cc3f0c8..efc605a9edb3 100644 --- a/packages/react-dom/src/__tests__/ReactDOMFiber-test.js +++ b/packages/react-dom/src/__tests__/ReactDOMFiber-test.js @@ -13,6 +13,8 @@ const React = require('react'); const ReactDOM = require('react-dom'); const PropTypes = require('prop-types'); +const ReactFeatureFlags = require('shared/ReactFeatureFlags'); + describe('ReactDOMFiber', () => { let container; @@ -247,30 +249,32 @@ describe('ReactDOMFiber', () => { }); // TODO: remove in React 17 - it('should support unstable_createPortal alias', () => { - const portalContainer = document.createElement('div'); + if (!ReactFeatureFlags.disableUnstableCreatePortal) { + it('should support unstable_createPortal alias', () => { + const portalContainer = document.createElement('div'); - expect(() => - ReactDOM.render( -
- {ReactDOM.unstable_createPortal(
portal
, portalContainer)} -
, - container, - ), - ).toWarnDev( - 'The ReactDOM.unstable_createPortal() alias has been deprecated, ' + - 'and will be removed in React 17+. Update your code to use ' + - 'ReactDOM.createPortal() instead. It has the exact same API, ' + - 'but without the "unstable_" prefix.', - {withoutStack: true}, - ); - expect(portalContainer.innerHTML).toBe('
portal
'); - expect(container.innerHTML).toBe('
'); + expect(() => + ReactDOM.render( +
+ {ReactDOM.unstable_createPortal(
portal
, portalContainer)} +
, + container, + ), + ).toWarnDev( + 'The ReactDOM.unstable_createPortal() alias has been deprecated, ' + + 'and will be removed in React 17+. Update your code to use ' + + 'ReactDOM.createPortal() instead. It has the exact same API, ' + + 'but without the "unstable_" prefix.', + {withoutStack: true}, + ); + expect(portalContainer.innerHTML).toBe('
portal
'); + expect(container.innerHTML).toBe('
'); - ReactDOM.unmountComponentAtNode(container); - expect(portalContainer.innerHTML).toBe(''); - expect(container.innerHTML).toBe(''); - }); + ReactDOM.unmountComponentAtNode(container); + expect(portalContainer.innerHTML).toBe(''); + expect(container.innerHTML).toBe(''); + }); + } it('should render many portals', () => { const portalContainer1 = document.createElement('div'); diff --git a/packages/react-dom/src/__tests__/renderSubtreeIntoContainer-test.js b/packages/react-dom/src/__tests__/renderSubtreeIntoContainer-test.js index c5ad79e2eba8..a7ae3807da41 100644 --- a/packages/react-dom/src/__tests__/renderSubtreeIntoContainer-test.js +++ b/packages/react-dom/src/__tests__/renderSubtreeIntoContainer-test.js @@ -16,308 +16,322 @@ const ReactTestUtils = require('react-dom/test-utils'); const renderSubtreeIntoContainer = require('react-dom') .unstable_renderSubtreeIntoContainer; -describe('renderSubtreeIntoContainer', () => { - it('should pass context when rendering subtree elsewhere', () => { - const portal = document.createElement('div'); - - class Component extends React.Component { - static contextTypes = { - foo: PropTypes.string.isRequired, - }; - - render() { - return
{this.context.foo}
; - } - } - - class Parent extends React.Component { - static childContextTypes = { - foo: PropTypes.string.isRequired, - }; - - getChildContext() { - return { - foo: 'bar', +const ReactFeatureFlags = require('shared/ReactFeatureFlags'); + +// Once this flag is always true, we should delete this test file +if (ReactFeatureFlags.disableUnstableRenderSubtreeIntoContainer) { + describe('renderSubtreeIntoContainer', () => { + it('empty test', () => { + // Empty test to prevent "Your test suite must contain at least one test." error. + }); + }); +} else { + describe('renderSubtreeIntoContainer', () => { + it('should pass context when rendering subtree elsewhere', () => { + const portal = document.createElement('div'); + + class Component extends React.Component { + static contextTypes = { + foo: PropTypes.string.isRequired, }; - } - - render() { - return null; - } - componentDidMount() { - expect( - function() { - renderSubtreeIntoContainer(this, , portal); - }.bind(this), - ).not.toThrow(); + render() { + return
{this.context.foo}
; + } } - } - - ReactTestUtils.renderIntoDocument(); - expect(portal.firstChild.innerHTML).toBe('bar'); - }); - - it('should throw if parentComponent is invalid', () => { - const portal = document.createElement('div'); - class Component extends React.Component { - static contextTypes = { - foo: PropTypes.string.isRequired, - }; - - render() { - return
{this.context.foo}
; - } - } - - // ESLint is confused here and thinks Parent is unused, presumably because - // it is only used inside of the class body? - // eslint-disable-next-line no-unused-vars - class Parent extends React.Component { - static childContextTypes = { - foo: PropTypes.string.isRequired, - }; - - getChildContext() { - return { - foo: 'bar', + class Parent extends React.Component { + static childContextTypes = { + foo: PropTypes.string.isRequired, }; - } - render() { - return null; - } + getChildContext() { + return { + foo: 'bar', + }; + } + + render() { + return null; + } + + componentDidMount() { + expect( + function() { + renderSubtreeIntoContainer(this, , portal); + }.bind(this), + ).toWarnDev( + 'ReactDOM.unstable_renderSubtreeIntoContainer() is deprecated and ' + + 'will be removed in a future major release. Consider using React Portals instead.', + ); + } + } + + ReactTestUtils.renderIntoDocument(); + expect(portal.firstChild.innerHTML).toBe('bar'); + }); + + it('should throw if parentComponent is invalid', () => { + const portal = document.createElement('div'); + + class Component extends React.Component { + static contextTypes = { + foo: PropTypes.string.isRequired, + }; - componentDidMount() { - expect(function() { - renderSubtreeIntoContainer(, , portal); - }).toThrowError('parentComponentmust be a valid React Component'); + render() { + return
{this.context.foo}
; + } } - } - }); - - it('should update context if it changes due to setState', () => { - const container = document.createElement('div'); - document.body.appendChild(container); - const portal = document.createElement('div'); - - class Component extends React.Component { - static contextTypes = { - foo: PropTypes.string.isRequired, - getFoo: PropTypes.func.isRequired, - }; - render() { - return
{this.context.foo + '-' + this.context.getFoo()}
; - } - } - - class Parent extends React.Component { - static childContextTypes = { - foo: PropTypes.string.isRequired, - getFoo: PropTypes.func.isRequired, - }; - - state = { - bar: 'initial', - }; - - getChildContext() { - return { - foo: this.state.bar, - getFoo: () => this.state.bar, + // ESLint is confused here and thinks Parent is unused, presumably because + // it is only used inside of the class body? + // eslint-disable-next-line no-unused-vars + class Parent extends React.Component { + static childContextTypes = { + foo: PropTypes.string.isRequired, }; - } - - render() { - return null; - } - componentDidMount() { - renderSubtreeIntoContainer(this, , portal); - } + getChildContext() { + return { + foo: 'bar', + }; + } + + render() { + return null; + } + + componentDidMount() { + expect(function() { + renderSubtreeIntoContainer(, , portal); + }).toThrowError('parentComponentmust be a valid React Component'); + } + } + }); + + it('should update context if it changes due to setState', () => { + const container = document.createElement('div'); + document.body.appendChild(container); + const portal = document.createElement('div'); + + class Component extends React.Component { + static contextTypes = { + foo: PropTypes.string.isRequired, + getFoo: PropTypes.func.isRequired, + }; - componentDidUpdate() { - renderSubtreeIntoContainer(this, , portal); + render() { + return
{this.context.foo + '-' + this.context.getFoo()}
; + } } - } - - const instance = ReactDOM.render(, container); - expect(portal.firstChild.innerHTML).toBe('initial-initial'); - instance.setState({bar: 'changed'}); - expect(portal.firstChild.innerHTML).toBe('changed-changed'); - }); - it('should update context if it changes due to re-render', () => { - const container = document.createElement('div'); - document.body.appendChild(container); - const portal = document.createElement('div'); - - class Component extends React.Component { - static contextTypes = { - foo: PropTypes.string.isRequired, - getFoo: PropTypes.func.isRequired, - }; - - render() { - return
{this.context.foo + '-' + this.context.getFoo()}
; - } - } - - class Parent extends React.Component { - static childContextTypes = { - foo: PropTypes.string.isRequired, - getFoo: PropTypes.func.isRequired, - }; - - getChildContext() { - return { - foo: this.props.bar, - getFoo: () => this.props.bar, + class Parent extends React.Component { + static childContextTypes = { + foo: PropTypes.string.isRequired, + getFoo: PropTypes.func.isRequired, }; - } - render() { - return null; - } + state = { + bar: 'initial', + }; - componentDidMount() { - renderSubtreeIntoContainer(this, , portal); - } + getChildContext() { + return { + foo: this.state.bar, + getFoo: () => this.state.bar, + }; + } + + render() { + return null; + } + + componentDidMount() { + renderSubtreeIntoContainer(this, , portal); + } + + componentDidUpdate() { + renderSubtreeIntoContainer(this, , portal); + } + } + + const instance = ReactDOM.render(, container); + expect(portal.firstChild.innerHTML).toBe('initial-initial'); + instance.setState({bar: 'changed'}); + expect(portal.firstChild.innerHTML).toBe('changed-changed'); + }); + + it('should update context if it changes due to re-render', () => { + const container = document.createElement('div'); + document.body.appendChild(container); + const portal = document.createElement('div'); + + class Component extends React.Component { + static contextTypes = { + foo: PropTypes.string.isRequired, + getFoo: PropTypes.func.isRequired, + }; - componentDidUpdate() { - renderSubtreeIntoContainer(this, , portal); + render() { + return
{this.context.foo + '-' + this.context.getFoo()}
; + } } - } - - ReactDOM.render(, container); - expect(portal.firstChild.innerHTML).toBe('initial-initial'); - ReactDOM.render(, container); - expect(portal.firstChild.innerHTML).toBe('changed-changed'); - }); - it('should render portal with non-context-provider parent', () => { - const container = document.createElement('div'); - document.body.appendChild(container); - const portal = document.createElement('div'); + class Parent extends React.Component { + static childContextTypes = { + foo: PropTypes.string.isRequired, + getFoo: PropTypes.func.isRequired, + }; - class Parent extends React.Component { - render() { - return null; + getChildContext() { + return { + foo: this.props.bar, + getFoo: () => this.props.bar, + }; + } + + render() { + return null; + } + + componentDidMount() { + renderSubtreeIntoContainer(this, , portal); + } + + componentDidUpdate() { + renderSubtreeIntoContainer(this, , portal); + } + } + + ReactDOM.render(, container); + expect(portal.firstChild.innerHTML).toBe('initial-initial'); + ReactDOM.render(, container); + expect(portal.firstChild.innerHTML).toBe('changed-changed'); + }); + + it('should render portal with non-context-provider parent', () => { + const container = document.createElement('div'); + document.body.appendChild(container); + const portal = document.createElement('div'); + + class Parent extends React.Component { + render() { + return null; + } + + componentDidMount() { + renderSubtreeIntoContainer(this,
hello
, portal); + } + } + + ReactDOM.render(, container); + expect(portal.firstChild.innerHTML).toBe('hello'); + }); + + it('should get context through non-context-provider parent', () => { + const container = document.createElement('div'); + document.body.appendChild(container); + const portal = document.createElement('div'); + + class Parent extends React.Component { + render() { + return ; + } + getChildContext() { + return {value: this.props.value}; + } + static childContextTypes = { + value: PropTypes.string.isRequired, + }; } - componentDidMount() { - renderSubtreeIntoContainer(this,
hello
, portal); + class Middle extends React.Component { + render() { + return null; + } + componentDidMount() { + renderSubtreeIntoContainer(this, , portal); + } } - } - - ReactDOM.render(, container); - expect(portal.firstChild.innerHTML).toBe('hello'); - }); - - it('should get context through non-context-provider parent', () => { - const container = document.createElement('div'); - document.body.appendChild(container); - const portal = document.createElement('div'); - class Parent extends React.Component { - render() { - return ; - } - getChildContext() { - return {value: this.props.value}; - } - static childContextTypes = { - value: PropTypes.string.isRequired, - }; - } - - class Middle extends React.Component { - render() { - return null; - } - componentDidMount() { - renderSubtreeIntoContainer(this, , portal); - } - } - - class Child extends React.Component { - static contextTypes = { - value: PropTypes.string.isRequired, - }; - render() { - return
{this.context.value}
; + class Child extends React.Component { + static contextTypes = { + value: PropTypes.string.isRequired, + }; + render() { + return
{this.context.value}
; + } + } + + ReactDOM.render(, container); + expect(portal.textContent).toBe('foo'); + }); + + it('should get context through middle non-context-provider layer', () => { + const container = document.createElement('div'); + document.body.appendChild(container); + const portal1 = document.createElement('div'); + const portal2 = document.createElement('div'); + + class Parent extends React.Component { + render() { + return null; + } + getChildContext() { + return {value: this.props.value}; + } + componentDidMount() { + renderSubtreeIntoContainer(this, , portal1); + } + static childContextTypes = { + value: PropTypes.string.isRequired, + }; } - } - - ReactDOM.render(, container); - expect(portal.textContent).toBe('foo'); - }); - - it('should get context through middle non-context-provider layer', () => { - const container = document.createElement('div'); - document.body.appendChild(container); - const portal1 = document.createElement('div'); - const portal2 = document.createElement('div'); - class Parent extends React.Component { - render() { - return null; - } - getChildContext() { - return {value: this.props.value}; - } - componentDidMount() { - renderSubtreeIntoContainer(this, , portal1); + class Middle extends React.Component { + render() { + return null; + } + componentDidMount() { + renderSubtreeIntoContainer(this, , portal2); + } } - static childContextTypes = { - value: PropTypes.string.isRequired, - }; - } - - class Middle extends React.Component { - render() { - return null; - } - componentDidMount() { - renderSubtreeIntoContainer(this, , portal2); - } - } - - class Child extends React.Component { - static contextTypes = { - value: PropTypes.string.isRequired, - }; - render() { - return
{this.context.value}
; - } - } - ReactDOM.render(, container); - expect(portal2.textContent).toBe('foo'); - }); - - it('fails gracefully when mixing React 15 and 16', () => { - class C extends React.Component { - render() { - return
; - } - } - const c = ReactDOM.render(, document.createElement('div')); - // React 15 calls this: - // https://github.com/facebook/react/blob/77b71fc3c4/src/renderers/dom/client/ReactMount.js#L478-L479 - expect(() => { - c._reactInternalInstance._processChildContext({}); - }).toThrow( - __DEV__ - ? '_processChildContext is not available in React 16+. This likely ' + - 'means you have multiple copies of React and are attempting to nest ' + - 'a React 15 tree inside a React 16 tree using ' + - "unstable_renderSubtreeIntoContainer, which isn't supported. Try to " + - 'make sure you have only one copy of React (and ideally, switch to ' + - 'ReactDOM.createPortal).' - : "Cannot read property '_processChildContext' of undefined", - ); + class Child extends React.Component { + static contextTypes = { + value: PropTypes.string.isRequired, + }; + render() { + return
{this.context.value}
; + } + } + + ReactDOM.render(, container); + expect(portal2.textContent).toBe('foo'); + }); + + it('fails gracefully when mixing React 15 and 16', () => { + class C extends React.Component { + render() { + return
; + } + } + const c = ReactDOM.render(, document.createElement('div')); + // React 15 calls this: + // https://github.com/facebook/react/blob/77b71fc3c4/src/renderers/dom/client/ReactMount.js#L478-L479 + expect(() => { + c._reactInternalInstance._processChildContext({}); + }).toThrow( + __DEV__ + ? '_processChildContext is not available in React 16+. This likely ' + + 'means you have multiple copies of React and are attempting to nest ' + + 'a React 15 tree inside a React 16 tree using ' + + "unstable_renderSubtreeIntoContainer, which isn't supported. Try to " + + 'make sure you have only one copy of React (and ideally, switch to ' + + 'ReactDOM.createPortal).' + : "Cannot read property '_processChildContext' of undefined", + ); + }); }); -}); +} diff --git a/packages/react-dom/src/client/ReactDOM.js b/packages/react-dom/src/client/ReactDOM.js index 079d3432ef2b..19a3db22387c 100644 --- a/packages/react-dom/src/client/ReactDOM.js +++ b/packages/react-dom/src/client/ReactDOM.js @@ -53,7 +53,11 @@ import { } from 'legacy-events/EventPropagators'; import ReactVersion from 'shared/ReactVersion'; import invariant from 'shared/invariant'; -import {exposeConcurrentModeAPIs} from 'shared/ReactFeatureFlags'; +import { + exposeConcurrentModeAPIs, + disableUnstableCreatePortal, + disableUnstableRenderSubtreeIntoContainer, +} from 'shared/ReactFeatureFlags'; import { getInstanceFromNode, @@ -77,6 +81,7 @@ setAttemptContinuousHydration(attemptContinuousHydration); setAttemptHydrationAtCurrentPriority(attemptHydrationAtCurrentPriority); let didWarnAboutUnstableCreatePortal = false; +let didWarnAboutUnstableRenderSubtreeIntoContainer = false; if (__DEV__) { if ( @@ -129,26 +134,8 @@ const ReactDOM: Object = { findDOMNode, hydrate, render, - unstable_renderSubtreeIntoContainer, unmountComponentAtNode, - // Temporary alias since we already shipped React 16 RC with it. - // TODO: remove in React 17. - unstable_createPortal(...args) { - if (__DEV__) { - if (!didWarnAboutUnstableCreatePortal) { - didWarnAboutUnstableCreatePortal = true; - console.warn( - 'The ReactDOM.unstable_createPortal() alias has been deprecated, ' + - 'and will be removed in React 17+. Update your code to use ' + - 'ReactDOM.createPortal() instead. It has the exact same API, ' + - 'but without the "unstable_" prefix.', - ); - } - } - return createPortal(...args); - }, - unstable_batchedUpdates: batchedUpdates, flushSync: flushSync, @@ -189,6 +176,41 @@ if (exposeConcurrentModeAPIs) { }; } +if (!disableUnstableRenderSubtreeIntoContainer) { + ReactDOM.unstable_renderSubtreeIntoContainer = function(...args) { + if (__DEV__) { + if (!didWarnAboutUnstableRenderSubtreeIntoContainer) { + didWarnAboutUnstableRenderSubtreeIntoContainer = true; + console.warn( + 'ReactDOM.unstable_renderSubtreeIntoContainer() is deprecated ' + + 'and will be removed in a future major release. Consider using ' + + 'React Portals instead.', + ); + } + } + return unstable_renderSubtreeIntoContainer(...args); + }; +} + +if (!disableUnstableCreatePortal) { + // Temporary alias since we already shipped React 16 RC with it. + // TODO: remove in React 17. + ReactDOM.unstable_createPortal = function(...args) { + if (__DEV__) { + if (!didWarnAboutUnstableCreatePortal) { + didWarnAboutUnstableCreatePortal = true; + console.warn( + 'The ReactDOM.unstable_createPortal() alias has been deprecated, ' + + 'and will be removed in React 17+. Update your code to use ' + + 'ReactDOM.createPortal() instead. It has the exact same API, ' + + 'but without the "unstable_" prefix.', + ); + } + } + return createPortal(...args); + }; +} + const foundDevTools = injectIntoDevTools({ findFiberByHostInstance: getClosestInstanceFromNode, bundleType: __DEV__ ? 1 : 0, diff --git a/packages/shared/ReactFeatureFlags.js b/packages/shared/ReactFeatureFlags.js index 7a19c29ea8ae..92de043bde50 100644 --- a/packages/shared/ReactFeatureFlags.js +++ b/packages/shared/ReactFeatureFlags.js @@ -108,3 +108,9 @@ export const disableCreateFactory = false; // Disables children for