From bc70441c8b3fa85338283af3eeb47b5d15e9dbfe Mon Sep 17 00:00:00 2001 From: Brian Vaughn Date: Wed, 14 Mar 2018 13:07:58 -0700 Subject: [PATCH] RFC #30: React.forwardRef implementation (#12346) Added React.forwardRef support to react-reconciler based renders and the SSR partial renderer. --- .../ReactDOMServerIntegrationRefs-test.js | 21 ++ .../src/server/ReactPartialRenderer.js | 20 ++ packages/react-is/src/ReactIs.js | 6 + .../react-is/src/__tests__/ReactIs-test.js | 9 + packages/react-reconciler/src/ReactFiber.js | 5 + .../src/ReactFiberBeginWork.js | 14 + .../src/ReactFiberCompleteWork.js | 3 + packages/react/src/React.js | 2 + packages/react/src/ReactElementValidator.js | 4 +- .../src/__tests__/forwardRef-test.internal.js | 252 ++++++++++++++++++ packages/react/src/forwardRef.js | 27 ++ packages/shared/ReactSymbols.js | 3 + packages/shared/ReactTypeOfWork.js | 4 +- 13 files changed, 368 insertions(+), 2 deletions(-) create mode 100644 packages/react/src/__tests__/forwardRef-test.internal.js create mode 100644 packages/react/src/forwardRef.js diff --git a/packages/react-dom/src/__tests__/ReactDOMServerIntegrationRefs-test.js b/packages/react-dom/src/__tests__/ReactDOMServerIntegrationRefs-test.js index 974d69497d8c0..37a83b2ff2581 100644 --- a/packages/react-dom/src/__tests__/ReactDOMServerIntegrationRefs-test.js +++ b/packages/react-dom/src/__tests__/ReactDOMServerIntegrationRefs-test.js @@ -96,4 +96,25 @@ describe('ReactDOMServerIntegration', () => { expect(component.refs.myDiv).toBe(root.firstChild); }); }); + + it('should forward refs', async () => { + const divRef = React.createRef(); + + class InnerComponent extends React.Component { + render() { + return
{this.props.value}
; + } + } + + const OuterComponent = React.forwardRef((props, ref) => ( + + )); + + await clientRenderOnServerString( + , + ); + + expect(divRef.current).not.toBe(null); + expect(divRef.current.textContent).toBe('hello'); + }); }); diff --git a/packages/react-dom/src/server/ReactPartialRenderer.js b/packages/react-dom/src/server/ReactPartialRenderer.js index b7742df2f6b49..1cede6e411785 100644 --- a/packages/react-dom/src/server/ReactPartialRenderer.js +++ b/packages/react-dom/src/server/ReactPartialRenderer.js @@ -27,6 +27,7 @@ import describeComponentFrame from 'shared/describeComponentFrame'; import {ReactDebugCurrentFrame} from 'shared/ReactGlobalSharedState'; import {warnAboutDeprecatedLifecycles} from 'shared/ReactFeatureFlags'; import { + REACT_FORWARD_REF_TYPE, REACT_FRAGMENT_TYPE, REACT_STRICT_MODE_TYPE, REACT_ASYNC_MODE_TYPE, @@ -841,6 +842,25 @@ class ReactDOMServerRenderer { } if (typeof elementType === 'object' && elementType !== null) { switch (elementType.$$typeof) { + case REACT_FORWARD_REF_TYPE: { + const element: ReactElement = ((nextChild: any): ReactElement); + const nextChildren = toArray( + elementType.render(element.props, element.ref), + ); + const frame: Frame = { + type: null, + domNamespace: parentNamespace, + children: nextChildren, + childIndex: 0, + context: context, + footer: '', + }; + if (__DEV__) { + ((frame: any): FrameDev).debugElementStack = []; + } + this.stack.push(frame); + return ''; + } case REACT_PROVIDER_TYPE: { const provider: ReactProvider = (nextChild: any); const nextProps = provider.props; diff --git a/packages/react-is/src/ReactIs.js b/packages/react-is/src/ReactIs.js index d26bc82f5e8c1..0265055e020de 100644 --- a/packages/react-is/src/ReactIs.js +++ b/packages/react-is/src/ReactIs.js @@ -13,6 +13,7 @@ import { REACT_ASYNC_MODE_TYPE, REACT_CONTEXT_TYPE, REACT_ELEMENT_TYPE, + REACT_FORWARD_REF_TYPE, REACT_FRAGMENT_TYPE, REACT_PORTAL_TYPE, REACT_PROVIDER_TYPE, @@ -37,6 +38,7 @@ export function typeOf(object: any) { switch ($$typeofType) { case REACT_CONTEXT_TYPE: + case REACT_FORWARD_REF_TYPE: case REACT_PROVIDER_TYPE: return $$typeofType; default: @@ -55,6 +57,7 @@ export const AsyncMode = REACT_ASYNC_MODE_TYPE; export const ContextConsumer = REACT_CONTEXT_TYPE; export const ContextProvider = REACT_PROVIDER_TYPE; export const Element = REACT_ELEMENT_TYPE; +export const ForwardRef = REACT_FORWARD_REF_TYPE; export const Fragment = REACT_FRAGMENT_TYPE; export const Portal = REACT_PORTAL_TYPE; export const StrictMode = REACT_STRICT_MODE_TYPE; @@ -75,6 +78,9 @@ export function isElement(object: any) { object.$$typeof === REACT_ELEMENT_TYPE ); } +export function isForwardRef(object: any) { + return typeOf(object) === REACT_FORWARD_REF_TYPE; +} export function isFragment(object: any) { return typeOf(object) === REACT_FRAGMENT_TYPE; } diff --git a/packages/react-is/src/__tests__/ReactIs-test.js b/packages/react-is/src/__tests__/ReactIs-test.js index 6200388fd525d..ab7f6e0d4c24f 100644 --- a/packages/react-is/src/__tests__/ReactIs-test.js +++ b/packages/react-is/src/__tests__/ReactIs-test.js @@ -76,6 +76,15 @@ describe('ReactIs', () => { expect(ReactIs.isElement()).toBe(true); }); + it('should identify ref forwarding component', () => { + const RefForwardingComponent = React.forwardRef((props, ref) => null); + expect(ReactIs.typeOf()).toBe(ReactIs.ForwardRef); + expect(ReactIs.isForwardRef()).toBe(true); + expect(ReactIs.isForwardRef({type: ReactIs.StrictMode})).toBe(false); + expect(ReactIs.isForwardRef()).toBe(false); + expect(ReactIs.isForwardRef(
)).toBe(false); + }); + it('should identify fragments', () => { expect(ReactIs.typeOf()).toBe(ReactIs.Fragment); expect(ReactIs.isFragment()).toBe(true); diff --git a/packages/react-reconciler/src/ReactFiber.js b/packages/react-reconciler/src/ReactFiber.js index eea42369de7ac..6c88c0aa18b02 100644 --- a/packages/react-reconciler/src/ReactFiber.js +++ b/packages/react-reconciler/src/ReactFiber.js @@ -25,6 +25,7 @@ import { HostPortal, CallComponent, ReturnComponent, + ForwardRef, Fragment, Mode, ContextProvider, @@ -35,6 +36,7 @@ import getComponentName from 'shared/getComponentName'; import {NoWork} from './ReactFiberExpirationTime'; import {NoContext, AsyncMode, StrictMode} from './ReactTypeOfMode'; import { + REACT_FORWARD_REF_TYPE, REACT_FRAGMENT_TYPE, REACT_RETURN_TYPE, REACT_CALL_TYPE, @@ -357,6 +359,9 @@ export function createFiberFromElement( // This is a consumer fiberTag = ContextConsumer; break; + case REACT_FORWARD_REF_TYPE: + fiberTag = ForwardRef; + break; default: if (typeof type.tag === 'number') { // Currently assumed to be a continuation and therefore is a diff --git a/packages/react-reconciler/src/ReactFiberBeginWork.js b/packages/react-reconciler/src/ReactFiberBeginWork.js index 95914814bc1e0..039fe24032e76 100644 --- a/packages/react-reconciler/src/ReactFiberBeginWork.js +++ b/packages/react-reconciler/src/ReactFiberBeginWork.js @@ -26,6 +26,7 @@ import { CallComponent, CallHandlerPhase, ReturnComponent, + ForwardRef, Fragment, Mode, ContextProvider, @@ -153,6 +154,17 @@ export default function( } } + function updateForwardRef(current, workInProgress) { + const render = workInProgress.type.render; + const nextChildren = render( + workInProgress.pendingProps, + workInProgress.ref, + ); + reconcileChildren(current, workInProgress, nextChildren); + memoizeProps(workInProgress, nextChildren); + return workInProgress.child; + } + function updateFragment(current, workInProgress) { const nextChildren = workInProgress.pendingProps; if (hasLegacyContextChanged()) { @@ -1130,6 +1142,8 @@ export default function( workInProgress, renderExpirationTime, ); + case ForwardRef: + return updateForwardRef(current, workInProgress); case Fragment: return updateFragment(current, workInProgress); case Mode: diff --git a/packages/react-reconciler/src/ReactFiberCompleteWork.js b/packages/react-reconciler/src/ReactFiberCompleteWork.js index 9e796dd53c152..3fb99ef2c2767 100644 --- a/packages/react-reconciler/src/ReactFiberCompleteWork.js +++ b/packages/react-reconciler/src/ReactFiberCompleteWork.js @@ -32,6 +32,7 @@ import { ReturnComponent, ContextProvider, ContextConsumer, + ForwardRef, Fragment, Mode, } from 'shared/ReactTypeOfWork'; @@ -603,6 +604,8 @@ export default function( case ReturnComponent: // Does nothing. return null; + case ForwardRef: + return null; case Fragment: return null; case Mode: diff --git a/packages/react/src/React.js b/packages/react/src/React.js index a2feefed097f8..d3a52cf61c406 100644 --- a/packages/react/src/React.js +++ b/packages/react/src/React.js @@ -24,6 +24,7 @@ import { isValidElement, } from './ReactElement'; import {createContext} from './ReactContext'; +import forwardRef from './forwardRef'; import { createElementWithValidation, createFactoryWithValidation, @@ -45,6 +46,7 @@ const React = { PureComponent, createContext, + forwardRef, Fragment: REACT_FRAGMENT_TYPE, StrictMode: REACT_STRICT_MODE_TYPE, diff --git a/packages/react/src/ReactElementValidator.js b/packages/react/src/ReactElementValidator.js index 471e6d6f8b677..fff09423d3e04 100644 --- a/packages/react/src/ReactElementValidator.js +++ b/packages/react/src/ReactElementValidator.js @@ -22,6 +22,7 @@ import { REACT_ASYNC_MODE_TYPE, REACT_PROVIDER_TYPE, REACT_CONTEXT_TYPE, + REACT_FORWARD_REF_TYPE, } from 'shared/ReactSymbols'; import checkPropTypes from 'prop-types/checkPropTypes'; import warning from 'fbjs/lib/warning'; @@ -297,7 +298,8 @@ export function createElementWithValidation(type, props, children) { (typeof type === 'object' && type !== null && (type.$$typeof === REACT_PROVIDER_TYPE || - type.$$typeof === REACT_CONTEXT_TYPE)); + type.$$typeof === REACT_CONTEXT_TYPE || + type.$$typeof === REACT_FORWARD_REF_TYPE)); // We warn in this case but don't throw. We expect the element creation to // succeed and there will likely be errors in render. diff --git a/packages/react/src/__tests__/forwardRef-test.internal.js b/packages/react/src/__tests__/forwardRef-test.internal.js new file mode 100644 index 0000000000000..068448c437400 --- /dev/null +++ b/packages/react/src/__tests__/forwardRef-test.internal.js @@ -0,0 +1,252 @@ +/** + * Copyright (c) 2013-present, Facebook, Inc. + * + * This source code is licensed under the MIT license found in the + * LICENSE file in the root directory of this source tree. + * + * @emails react-core + */ + +'use strict'; + +describe('forwardRef', () => { + let React; + let ReactFeatureFlags; + let ReactNoop; + + beforeEach(() => { + jest.resetModules(); + ReactFeatureFlags = require('shared/ReactFeatureFlags'); + ReactFeatureFlags.debugRenderPhaseSideEffectsForStrictMode = false; + React = require('react'); + ReactNoop = require('react-noop-renderer'); + }); + + it('should work without a ref to be forwarded', () => { + class Child extends React.Component { + render() { + ReactNoop.yield(this.props.value); + return null; + } + } + + function Wrapper(props) { + return ; + } + + const RefForwardingComponent = React.forwardRef((props, ref) => ( + + )); + + ReactNoop.render(); + expect(ReactNoop.flush()).toEqual([123]); + }); + + it('should forward a ref for a single child', () => { + class Child extends React.Component { + render() { + ReactNoop.yield(this.props.value); + return null; + } + } + + function Wrapper(props) { + return ; + } + + const RefForwardingComponent = React.forwardRef((props, ref) => ( + + )); + + const ref = React.createRef(); + + ReactNoop.render(); + expect(ReactNoop.flush()).toEqual([123]); + expect(ref.current instanceof Child).toBe(true); + }); + + it('should forward a ref for multiple children', () => { + class Child extends React.Component { + render() { + ReactNoop.yield(this.props.value); + return null; + } + } + + function Wrapper(props) { + return ; + } + + const RefForwardingComponent = React.forwardRef((props, ref) => ( + + )); + + const ref = React.createRef(); + + ReactNoop.render( +
+
+ +
+
, + ); + expect(ReactNoop.flush()).toEqual([123]); + expect(ref.current instanceof Child).toBe(true); + }); + + it('should update refs when switching between children', () => { + function FunctionalComponent({forwardedRef, setRefOnDiv}) { + return ( +
+
First
+ Second +
+ ); + } + + const RefForwardingComponent = React.forwardRef((props, ref) => ( + + )); + + const ref = React.createRef(); + + ReactNoop.render(); + ReactNoop.flush(); + expect(ref.current.type).toBe('div'); + + ReactNoop.render(); + ReactNoop.flush(); + expect(ref.current.type).toBe('span'); + }); + + it('should maintain child instance and ref through updates', () => { + class Child extends React.Component { + constructor(props) { + super(props); + } + render() { + ReactNoop.yield(this.props.value); + return null; + } + } + + function Wrapper(props) { + return ; + } + + const RefForwardingComponent = React.forwardRef((props, ref) => ( + + )); + + let setRefCount = 0; + let ref; + + const setRef = r => { + setRefCount++; + ref = r; + }; + + ReactNoop.render(); + expect(ReactNoop.flush()).toEqual([123]); + expect(ref instanceof Child).toBe(true); + expect(setRefCount).toBe(1); + ReactNoop.render(); + expect(ReactNoop.flush()).toEqual([456]); + expect(ref instanceof Child).toBe(true); + expect(setRefCount).toBe(1); + }); + + it('should not break lifecycle error handling', () => { + class ErrorBoundary extends React.Component { + state = {error: null}; + componentDidCatch(error) { + ReactNoop.yield('ErrorBoundary.componentDidCatch'); + this.setState({error}); + } + render() { + if (this.state.error) { + ReactNoop.yield('ErrorBoundary.render: catch'); + return null; + } + ReactNoop.yield('ErrorBoundary.render: try'); + return this.props.children; + } + } + + class BadRender extends React.Component { + render() { + ReactNoop.yield('BadRender throw'); + throw new Error('oops!'); + } + } + + function Wrapper(props) { + ReactNoop.yield('Wrapper'); + return ; + } + + const RefForwardingComponent = React.forwardRef((props, ref) => ( + + )); + + const ref = React.createRef(); + + ReactNoop.render( + + + , + ); + expect(ReactNoop.flush()).toEqual([ + 'ErrorBoundary.render: try', + 'Wrapper', + 'BadRender throw', + 'ErrorBoundary.componentDidCatch', + 'ErrorBoundary.render: catch', + ]); + expect(ref.current).toBe(null); + }); + + it('should support rendering null', () => { + const RefForwardingComponent = React.forwardRef((props, ref) => null); + + const ref = React.createRef(); + + ReactNoop.render(); + ReactNoop.flush(); + expect(ref.current).toBe(null); + }); + + it('should support rendering null for multiple children', () => { + const RefForwardingComponent = React.forwardRef((props, ref) => null); + + const ref = React.createRef(); + + ReactNoop.render( +
+
+ +
+
, + ); + ReactNoop.flush(); + expect(ref.current).toBe(null); + }); + + it('should warn if not provided a callback during creation', () => { + expect(() => React.forwardRef(undefined)).toWarnDev( + 'forwardRef requires a render function but was given undefined.', + ); + expect(() => React.forwardRef(null)).toWarnDev( + 'forwardRef requires a render function but was given null.', + ); + expect(() => React.forwardRef('foo')).toWarnDev( + 'forwardRef requires a render function but was given string.', + ); + }); + + it('should warn if no render function is provided', () => { + expect(React.forwardRef).toWarnDev( + 'forwardRef requires a render function but was given undefined.', + ); + }); +}); diff --git a/packages/react/src/forwardRef.js b/packages/react/src/forwardRef.js new file mode 100644 index 0000000000000..6a923be12795d --- /dev/null +++ b/packages/react/src/forwardRef.js @@ -0,0 +1,27 @@ +/** + * Copyright (c) 2014-present, Facebook, Inc. + * + * This source code is licensed under the MIT license found in the + * LICENSE file in the root directory of this source tree. + */ + +import {REACT_FORWARD_REF_TYPE} from 'shared/ReactSymbols'; + +import warning from 'fbjs/lib/warning'; + +export default function forwardRef( + render: (props: Props, ref: React$ElementRef) => React$Node, +) { + if (__DEV__) { + warning( + typeof render === 'function', + 'forwardRef requires a render function but was given %s.', + render === null ? 'null' : typeof render, + ); + } + + return { + $$typeof: REACT_FORWARD_REF_TYPE, + render, + }; +} diff --git a/packages/shared/ReactSymbols.js b/packages/shared/ReactSymbols.js index cc60e34e4c5b1..12e0fdddad9b9 100644 --- a/packages/shared/ReactSymbols.js +++ b/packages/shared/ReactSymbols.js @@ -36,6 +36,9 @@ export const REACT_CONTEXT_TYPE = hasSymbol export const REACT_ASYNC_MODE_TYPE = hasSymbol ? Symbol.for('react.async_mode') : 0xeacf; +export const REACT_FORWARD_REF_TYPE = hasSymbol + ? Symbol.for('react.forward_ref') + : 0xead0; const MAYBE_ITERATOR_SYMBOL = typeof Symbol === 'function' && Symbol.iterator; const FAUX_ITERATOR_SYMBOL = '@@iterator'; diff --git a/packages/shared/ReactTypeOfWork.js b/packages/shared/ReactTypeOfWork.js index 3d1bfc3f37584..573b75aabc0f0 100644 --- a/packages/shared/ReactTypeOfWork.js +++ b/packages/shared/ReactTypeOfWork.js @@ -21,7 +21,8 @@ export type TypeOfWork = | 10 | 11 | 12 - | 13; + | 13 + | 14; export const IndeterminateComponent = 0; // Before we know whether it is functional or class export const FunctionalComponent = 1; @@ -37,3 +38,4 @@ export const Fragment = 10; export const Mode = 11; export const ContextConsumer = 12; export const ContextProvider = 13; +export const ForwardRef = 14;