From 9bd0fdb40264f7117d98ac200b6cf04a4d0ac0dd Mon Sep 17 00:00:00 2001 From: Brian Vaughn Date: Thu, 8 Mar 2018 15:40:24 -0800 Subject: [PATCH 01/21] RFC 30: React.useRef --- 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 | 13 ++ .../src/ReactFiberCompleteWork.js | 3 + packages/react/src/React.js | 2 + packages/react/src/ReactElementValidator.js | 4 +- .../src/__tests__/useRef-test.internal.js | 151 ++++++++++++++++++ packages/react/src/useRef.js | 17 ++ packages/shared/ReactSymbols.js | 3 + packages/shared/ReactTypeOfWork.js | 4 +- 11 files changed, 215 insertions(+), 2 deletions(-) create mode 100644 packages/react/src/__tests__/useRef-test.internal.js create mode 100644 packages/react/src/useRef.js diff --git a/packages/react-is/src/ReactIs.js b/packages/react-is/src/ReactIs.js index d26bc82f5e8c1..c61164b17f52a 100644 --- a/packages/react-is/src/ReactIs.js +++ b/packages/react-is/src/ReactIs.js @@ -17,6 +17,7 @@ import { REACT_PORTAL_TYPE, REACT_PROVIDER_TYPE, REACT_STRICT_MODE_TYPE, + REACT_USE_REF_TYPE, } from 'shared/ReactSymbols'; export function typeOf(object: any) { @@ -38,6 +39,7 @@ export function typeOf(object: any) { switch ($$typeofType) { case REACT_CONTEXT_TYPE: case REACT_PROVIDER_TYPE: + case REACT_USE_REF_TYPE: return $$typeofType; default: return $$typeof; @@ -58,6 +60,7 @@ export const Element = REACT_ELEMENT_TYPE; export const Fragment = REACT_FRAGMENT_TYPE; export const Portal = REACT_PORTAL_TYPE; export const StrictMode = REACT_STRICT_MODE_TYPE; +export const UseRef = REACT_USE_REF_TYPE; export function isAsyncMode(object: any) { return typeOf(object) === REACT_ASYNC_MODE_TYPE; @@ -84,3 +87,6 @@ export function isPortal(object: any) { export function isStrictMode(object: any) { return typeOf(object) === REACT_STRICT_MODE_TYPE; } +export function isUseRef(object: any) { + return typeOf(object) === REACT_USE_REF_TYPE; +} diff --git a/packages/react-is/src/__tests__/ReactIs-test.js b/packages/react-is/src/__tests__/ReactIs-test.js index 6200388fd525d..17ce965b53d00 100644 --- a/packages/react-is/src/__tests__/ReactIs-test.js +++ b/packages/react-is/src/__tests__/ReactIs-test.js @@ -100,4 +100,13 @@ describe('ReactIs', () => { expect(ReactIs.isStrictMode()).toBe(false); expect(ReactIs.isStrictMode(
)).toBe(false); }); + + it('should identify ref forwarding component', () => { + const RefForwardingComponent = React.useRef((props, ref) => null); + expect(ReactIs.typeOf()).toBe(ReactIs.UseRef); + expect(ReactIs.isUseRef()).toBe(true); + expect(ReactIs.isUseRef({type: ReactIs.StrictMode})).toBe(false); + expect(ReactIs.isUseRef()).toBe(false); + expect(ReactIs.isUseRef(
)).toBe(false); + }); }); diff --git a/packages/react-reconciler/src/ReactFiber.js b/packages/react-reconciler/src/ReactFiber.js index eea42369de7ac..9a3711ab2e0d5 100644 --- a/packages/react-reconciler/src/ReactFiber.js +++ b/packages/react-reconciler/src/ReactFiber.js @@ -29,6 +29,7 @@ import { Mode, ContextProvider, ContextConsumer, + UseRef, } from 'shared/ReactTypeOfWork'; import getComponentName from 'shared/getComponentName'; @@ -42,6 +43,7 @@ import { REACT_PROVIDER_TYPE, REACT_CONTEXT_TYPE, REACT_ASYNC_MODE_TYPE, + REACT_USE_REF_TYPE, } from 'shared/ReactSymbols'; let hasBadMapPolyfill; @@ -357,6 +359,9 @@ export function createFiberFromElement( // This is a consumer fiberTag = ContextConsumer; break; + case REACT_USE_REF_TYPE: + fiberTag = UseRef; + 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 a5197b32025fd..f28a725265051 100644 --- a/packages/react-reconciler/src/ReactFiberBeginWork.js +++ b/packages/react-reconciler/src/ReactFiberBeginWork.js @@ -30,6 +30,7 @@ import { Mode, ContextProvider, ContextConsumer, + UseRef, } from 'shared/ReactTypeOfWork'; import { PerformedWork, @@ -166,6 +167,16 @@ export default function( return workInProgress.child; } + function updateUseRef(current, workInProgress) { + const nextChildren = workInProgress.type.callback( + workInProgress.pendingProps, + workInProgress.ref, + ); + reconcileChildren(current, workInProgress, nextChildren); + memoizeProps(workInProgress, nextChildren); + return workInProgress.child; + } + function updateMode(current, workInProgress) { const nextChildren = workInProgress.pendingProps.children; if (hasLegacyContextChanged()) { @@ -1102,6 +1113,8 @@ export default function( return updateFragment(current, workInProgress); case Mode: return updateMode(current, workInProgress); + case UseRef: + return updateUseRef(current, workInProgress); case ContextProvider: return updateContextProvider( current, diff --git a/packages/react-reconciler/src/ReactFiberCompleteWork.js b/packages/react-reconciler/src/ReactFiberCompleteWork.js index 9e796dd53c152..dd4e8dd6d3b11 100644 --- a/packages/react-reconciler/src/ReactFiberCompleteWork.js +++ b/packages/react-reconciler/src/ReactFiberCompleteWork.js @@ -34,6 +34,7 @@ import { ContextConsumer, Fragment, Mode, + UseRef, } from 'shared/ReactTypeOfWork'; import { Placement, @@ -617,6 +618,8 @@ export default function( return null; case ContextConsumer: return null; + case UseRef: + return null; // Error cases case IndeterminateComponent: invariant( diff --git a/packages/react/src/React.js b/packages/react/src/React.js index a2feefed097f8..0d3c6c6edf7a7 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 useRef from './useRef'; import { createElementWithValidation, createFactoryWithValidation, @@ -45,6 +46,7 @@ const React = { PureComponent, createContext, + useRef, 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..6ce0a734bc5ea 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_USE_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_USE_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__/useRef-test.internal.js b/packages/react/src/__tests__/useRef-test.internal.js new file mode 100644 index 0000000000000..2c83baaf2c58d --- /dev/null +++ b/packages/react/src/__tests__/useRef-test.internal.js @@ -0,0 +1,151 @@ +/** + * 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('useRef', () => { + 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.useRef((props, ref) => ( + + )); + + ReactNoop.render(); + expect(ReactNoop.flush()).toEqual([123]); + }); + + it('should forward a ref to a chosen component', () => { + class Child extends React.Component { + render() { + ReactNoop.yield(this.props.value); + return null; + } + } + + function Wrapper(props) { + return ; + } + + const RefForwardingComponent = React.useRef((props, ref) => ( + + )); + + const ref = React.createRef(); + + ReactNoop.render(); + expect(ReactNoop.flush()).toEqual([123]); + expect(ref.value instanceof Child).toBe(true); + }); + + it('should maintain ref through updates', () => { + class Child extends React.Component { + render() { + ReactNoop.yield(this.props.value); + return null; + } + } + + function Wrapper(props) { + return ; + } + + const RefForwardingComponent = React.useRef((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.useRef((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.value).toBe(null); + }); +}); diff --git a/packages/react/src/useRef.js b/packages/react/src/useRef.js new file mode 100644 index 0000000000000..8b9e3565180b6 --- /dev/null +++ b/packages/react/src/useRef.js @@ -0,0 +1,17 @@ +/** + * 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_USE_REF_TYPE} from 'shared/ReactSymbols'; + +export default function useRef( + callback: (props: Props, ref: React$ElementRef) => React$Node, +) { + return { + $$typeof: REACT_USE_REF_TYPE, + callback, + }; +} diff --git a/packages/shared/ReactSymbols.js b/packages/shared/ReactSymbols.js index cc60e34e4c5b1..9c7308af7a302 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_USE_REF_TYPE = hasSymbol + ? Symbol.for('react.use_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..598a1a2f30150 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 UseRef = 14; From 753dba7687c16ec7aeb535a7f6e7cd31d914e3c3 Mon Sep 17 00:00:00 2001 From: Brian Vaughn Date: Thu, 8 Mar 2018 15:45:47 -0800 Subject: [PATCH 02/21] Added error handling tests --- .../src/ReactFiberBeginWork.js | 2 +- .../src/__tests__/useRef-test.internal.js | 22 +++++++++++++++++++ packages/react/src/useRef.js | 12 ++++++++-- 3 files changed, 33 insertions(+), 3 deletions(-) diff --git a/packages/react-reconciler/src/ReactFiberBeginWork.js b/packages/react-reconciler/src/ReactFiberBeginWork.js index f28a725265051..0ccf6243d5564 100644 --- a/packages/react-reconciler/src/ReactFiberBeginWork.js +++ b/packages/react-reconciler/src/ReactFiberBeginWork.js @@ -168,7 +168,7 @@ export default function( } function updateUseRef(current, workInProgress) { - const nextChildren = workInProgress.type.callback( + const nextChildren = workInProgress.type.renderProp( workInProgress.pendingProps, workInProgress.ref, ); diff --git a/packages/react/src/__tests__/useRef-test.internal.js b/packages/react/src/__tests__/useRef-test.internal.js index 2c83baaf2c58d..224b1b41c6c2f 100644 --- a/packages/react/src/__tests__/useRef-test.internal.js +++ b/packages/react/src/__tests__/useRef-test.internal.js @@ -148,4 +148,26 @@ describe('useRef', () => { ]); expect(ref.value).toBe(null); }); + + it('should support rendering null', () => { + const RefForwardingComponent = React.useRef((props, ref) => null); + + const ref = React.createRef(); + + ReactNoop.render(); + ReactNoop.flush(); + expect(ref.value).toBe(null); + }); + + it('should error if not provided a callback', () => { + expect(() => React.useRef(undefined)).toThrow( + 'useRef requires a render prop but was given undefined.', + ); + expect(() => React.useRef(null)).toThrow( + 'useRef requires a render prop but was given null.', + ); + expect(() => React.useRef('foo')).toThrow( + 'useRef requires a render prop but was given string.', + ); + }); }); diff --git a/packages/react/src/useRef.js b/packages/react/src/useRef.js index 8b9e3565180b6..42386d855cfca 100644 --- a/packages/react/src/useRef.js +++ b/packages/react/src/useRef.js @@ -7,11 +7,19 @@ import {REACT_USE_REF_TYPE} from 'shared/ReactSymbols'; +import invariant from 'fbjs/lib/invariant'; + export default function useRef( - callback: (props: Props, ref: React$ElementRef) => React$Node, + renderProp: (props: Props, ref: React$ElementRef) => React$Node, ) { + invariant( + typeof renderProp === 'function', + 'useRef requires a render prop but was given %s.', + renderProp === null ? 'null' : typeof renderProp, + ); + return { $$typeof: REACT_USE_REF_TYPE, - callback, + renderProp, }; } From 56138d2e02a8ca3b2b4bfa3e908891bcb13a4fac Mon Sep 17 00:00:00 2001 From: Brian Vaughn Date: Thu, 8 Mar 2018 16:06:54 -0800 Subject: [PATCH 03/21] invariant() -> warning() --- packages/react/src/__tests__/useRef-test.internal.js | 6 +++--- packages/react/src/useRef.js | 4 ++-- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/packages/react/src/__tests__/useRef-test.internal.js b/packages/react/src/__tests__/useRef-test.internal.js index 224b1b41c6c2f..38698afa7e4ef 100644 --- a/packages/react/src/__tests__/useRef-test.internal.js +++ b/packages/react/src/__tests__/useRef-test.internal.js @@ -160,13 +160,13 @@ describe('useRef', () => { }); it('should error if not provided a callback', () => { - expect(() => React.useRef(undefined)).toThrow( + expect(() => React.useRef(undefined)).toWarnDev( 'useRef requires a render prop but was given undefined.', ); - expect(() => React.useRef(null)).toThrow( + expect(() => React.useRef(null)).toWarnDev( 'useRef requires a render prop but was given null.', ); - expect(() => React.useRef('foo')).toThrow( + expect(() => React.useRef('foo')).toWarnDev( 'useRef requires a render prop but was given string.', ); }); diff --git a/packages/react/src/useRef.js b/packages/react/src/useRef.js index 42386d855cfca..8b49810716f25 100644 --- a/packages/react/src/useRef.js +++ b/packages/react/src/useRef.js @@ -7,12 +7,12 @@ import {REACT_USE_REF_TYPE} from 'shared/ReactSymbols'; -import invariant from 'fbjs/lib/invariant'; +import warning from 'fbjs/lib/warning'; export default function useRef( renderProp: (props: Props, ref: React$ElementRef) => React$Node, ) { - invariant( + warning( typeof renderProp === 'function', 'useRef requires a render prop but was given %s.', renderProp === null ? 'null' : typeof renderProp, From 60efe327e8331c5c77032f98eeb1338a21c83752 Mon Sep 17 00:00:00 2001 From: Brian Vaughn Date: Fri, 9 Mar 2018 07:45:57 -0800 Subject: [PATCH 04/21] Changed warning wording slightly --- packages/react/src/__tests__/useRef-test.internal.js | 6 +++--- packages/react/src/useRef.js | 2 +- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/packages/react/src/__tests__/useRef-test.internal.js b/packages/react/src/__tests__/useRef-test.internal.js index 38698afa7e4ef..4e3999fbde659 100644 --- a/packages/react/src/__tests__/useRef-test.internal.js +++ b/packages/react/src/__tests__/useRef-test.internal.js @@ -161,13 +161,13 @@ describe('useRef', () => { it('should error if not provided a callback', () => { expect(() => React.useRef(undefined)).toWarnDev( - 'useRef requires a render prop but was given undefined.', + 'useRef requires a render function but was given undefined.', ); expect(() => React.useRef(null)).toWarnDev( - 'useRef requires a render prop but was given null.', + 'useRef requires a render function but was given null.', ); expect(() => React.useRef('foo')).toWarnDev( - 'useRef requires a render prop but was given string.', + 'useRef requires a render function but was given string.', ); }); }); diff --git a/packages/react/src/useRef.js b/packages/react/src/useRef.js index 8b49810716f25..72b428cd7f8d0 100644 --- a/packages/react/src/useRef.js +++ b/packages/react/src/useRef.js @@ -14,7 +14,7 @@ export default function useRef( ) { warning( typeof renderProp === 'function', - 'useRef requires a render prop but was given %s.', + 'useRef requires a render function but was given %s.', renderProp === null ? 'null' : typeof renderProp, ); From 166d23696cf21dd9bcf6e300cb82b178e584ebc3 Mon Sep 17 00:00:00 2001 From: Brian Vaughn Date: Sat, 10 Mar 2018 09:17:02 -0800 Subject: [PATCH 05/21] Added invariant with component stack (in dev) for useRef without function --- .../src/ReactFiberBeginWork.js | 9 +++++- .../src/__tests__/useRef-test.internal.js | 31 ++++++++++++++++++- 2 files changed, 38 insertions(+), 2 deletions(-) diff --git a/packages/react-reconciler/src/ReactFiberBeginWork.js b/packages/react-reconciler/src/ReactFiberBeginWork.js index 0ccf6243d5564..d535940d06afd 100644 --- a/packages/react-reconciler/src/ReactFiberBeginWork.js +++ b/packages/react-reconciler/src/ReactFiberBeginWork.js @@ -168,7 +168,14 @@ export default function( } function updateUseRef(current, workInProgress) { - const nextChildren = workInProgress.type.renderProp( + const renderProp = workInProgress.type.renderProp; + invariant( + typeof renderProp === 'function', + 'useRef requires a render function but was given %s.%s', + renderProp === null ? 'null' : typeof renderProp, + ReactDebugCurrentFiber.getCurrentFiberStackAddendum() || '', + ); + const nextChildren = renderProp( workInProgress.pendingProps, workInProgress.ref, ); diff --git a/packages/react/src/__tests__/useRef-test.internal.js b/packages/react/src/__tests__/useRef-test.internal.js index 4e3999fbde659..8c55cc8f57a7d 100644 --- a/packages/react/src/__tests__/useRef-test.internal.js +++ b/packages/react/src/__tests__/useRef-test.internal.js @@ -14,6 +14,10 @@ describe('useRef', () => { let ReactFeatureFlags; let ReactNoop; + function normalizeCodeLocInfo(str) { + return str && str.replace(/\(at .+?:\d+\)/g, '(at **)'); + } + beforeEach(() => { jest.resetModules(); ReactFeatureFlags = require('shared/ReactFeatureFlags'); @@ -159,7 +163,7 @@ describe('useRef', () => { expect(ref.value).toBe(null); }); - it('should error if not provided a callback', () => { + it('should warn if not provided a callback during creation', () => { expect(() => React.useRef(undefined)).toWarnDev( 'useRef requires a render function but was given undefined.', ); @@ -170,4 +174,29 @@ describe('useRef', () => { 'useRef requires a render function but was given string.', ); }); + + it('should error with a callstack if rendered without a function', () => { + let RefForwardingComponent; + expect(() => { + RefForwardingComponent = React.useRef(); + }).toWarnDev('useRef requires a render function but was given undefined.'); + + ReactNoop.render( +
+ +
, + ); + + let caughtError; + try { + ReactNoop.flush(); + } catch (error) { + caughtError = error; + } + expect(caughtError).toBeDefined(); + expect(normalizeCodeLocInfo(caughtError.message)).toBe( + 'useRef requires a render function but was given undefined.' + + (__DEV__ ? '\n in div (at **)' : ''), + ); + }); }); From 0d565b5d540020de40d5f465d7268758c88d5f16 Mon Sep 17 00:00:00 2001 From: Brian Vaughn Date: Mon, 12 Mar 2018 12:43:19 -0700 Subject: [PATCH 06/21] Renamed files useRef -> forwradRef --- .../{useRef-test.internal.js => forwardRef-test.internal.js} | 0 packages/react/src/{useRef.js => forwardRef.js} | 0 2 files changed, 0 insertions(+), 0 deletions(-) rename packages/react/src/__tests__/{useRef-test.internal.js => forwardRef-test.internal.js} (100%) rename packages/react/src/{useRef.js => forwardRef.js} (100%) diff --git a/packages/react/src/__tests__/useRef-test.internal.js b/packages/react/src/__tests__/forwardRef-test.internal.js similarity index 100% rename from packages/react/src/__tests__/useRef-test.internal.js rename to packages/react/src/__tests__/forwardRef-test.internal.js diff --git a/packages/react/src/useRef.js b/packages/react/src/forwardRef.js similarity index 100% rename from packages/react/src/useRef.js rename to packages/react/src/forwardRef.js From 508d52ed98bb3963e2dfcf54294bfa29282e08a9 Mon Sep 17 00:00:00 2001 From: Brian Vaughn Date: Mon, 12 Mar 2018 12:45:20 -0700 Subject: [PATCH 07/21] Find and replace useRef => forwardRef --- .../react-is/src/__tests__/ReactIs-test.js | 2 +- .../src/ReactFiberBeginWork.js | 2 +- packages/react/src/React.js | 4 +-- .../src/__tests__/forwardRef-test.internal.js | 32 ++++++++++--------- packages/react/src/forwardRef.js | 4 +-- 5 files changed, 23 insertions(+), 21 deletions(-) diff --git a/packages/react-is/src/__tests__/ReactIs-test.js b/packages/react-is/src/__tests__/ReactIs-test.js index 17ce965b53d00..3d32536f21fd7 100644 --- a/packages/react-is/src/__tests__/ReactIs-test.js +++ b/packages/react-is/src/__tests__/ReactIs-test.js @@ -102,7 +102,7 @@ describe('ReactIs', () => { }); it('should identify ref forwarding component', () => { - const RefForwardingComponent = React.useRef((props, ref) => null); + const RefForwardingComponent = React.forwardRef((props, ref) => null); expect(ReactIs.typeOf()).toBe(ReactIs.UseRef); expect(ReactIs.isUseRef()).toBe(true); expect(ReactIs.isUseRef({type: ReactIs.StrictMode})).toBe(false); diff --git a/packages/react-reconciler/src/ReactFiberBeginWork.js b/packages/react-reconciler/src/ReactFiberBeginWork.js index d535940d06afd..5027cd6635fba 100644 --- a/packages/react-reconciler/src/ReactFiberBeginWork.js +++ b/packages/react-reconciler/src/ReactFiberBeginWork.js @@ -171,7 +171,7 @@ export default function( const renderProp = workInProgress.type.renderProp; invariant( typeof renderProp === 'function', - 'useRef requires a render function but was given %s.%s', + 'forwardRef requires a render function but was given %s.%s', renderProp === null ? 'null' : typeof renderProp, ReactDebugCurrentFiber.getCurrentFiberStackAddendum() || '', ); diff --git a/packages/react/src/React.js b/packages/react/src/React.js index 0d3c6c6edf7a7..d3a52cf61c406 100644 --- a/packages/react/src/React.js +++ b/packages/react/src/React.js @@ -24,7 +24,7 @@ import { isValidElement, } from './ReactElement'; import {createContext} from './ReactContext'; -import useRef from './useRef'; +import forwardRef from './forwardRef'; import { createElementWithValidation, createFactoryWithValidation, @@ -46,7 +46,7 @@ const React = { PureComponent, createContext, - useRef, + forwardRef, Fragment: REACT_FRAGMENT_TYPE, StrictMode: REACT_STRICT_MODE_TYPE, diff --git a/packages/react/src/__tests__/forwardRef-test.internal.js b/packages/react/src/__tests__/forwardRef-test.internal.js index 8c55cc8f57a7d..35b97f8f49642 100644 --- a/packages/react/src/__tests__/forwardRef-test.internal.js +++ b/packages/react/src/__tests__/forwardRef-test.internal.js @@ -9,7 +9,7 @@ 'use strict'; -describe('useRef', () => { +describe('forwardRef', () => { let React; let ReactFeatureFlags; let ReactNoop; @@ -38,7 +38,7 @@ describe('useRef', () => { return ; } - const RefForwardingComponent = React.useRef((props, ref) => ( + const RefForwardingComponent = React.forwardRef((props, ref) => ( )); @@ -58,7 +58,7 @@ describe('useRef', () => { return ; } - const RefForwardingComponent = React.useRef((props, ref) => ( + const RefForwardingComponent = React.forwardRef((props, ref) => ( )); @@ -81,7 +81,7 @@ describe('useRef', () => { return ; } - const RefForwardingComponent = React.useRef((props, ref) => ( + const RefForwardingComponent = React.forwardRef((props, ref) => ( )); @@ -132,7 +132,7 @@ describe('useRef', () => { return ; } - const RefForwardingComponent = React.useRef((props, ref) => ( + const RefForwardingComponent = React.forwardRef((props, ref) => ( )); @@ -154,7 +154,7 @@ describe('useRef', () => { }); it('should support rendering null', () => { - const RefForwardingComponent = React.useRef((props, ref) => null); + const RefForwardingComponent = React.forwardRef((props, ref) => null); const ref = React.createRef(); @@ -164,22 +164,24 @@ describe('useRef', () => { }); it('should warn if not provided a callback during creation', () => { - expect(() => React.useRef(undefined)).toWarnDev( - 'useRef requires a render function but was given undefined.', + expect(() => React.forwardRef(undefined)).toWarnDev( + 'forwardRef requires a render function but was given undefined.', ); - expect(() => React.useRef(null)).toWarnDev( - 'useRef requires a render function but was given null.', + expect(() => React.forwardRef(null)).toWarnDev( + 'forwardRef requires a render function but was given null.', ); - expect(() => React.useRef('foo')).toWarnDev( - 'useRef requires a render function but was given string.', + expect(() => React.forwardRef('foo')).toWarnDev( + 'forwardRef requires a render function but was given string.', ); }); it('should error with a callstack if rendered without a function', () => { let RefForwardingComponent; expect(() => { - RefForwardingComponent = React.useRef(); - }).toWarnDev('useRef requires a render function but was given undefined.'); + RefForwardingComponent = React.forwardRef(); + }).toWarnDev( + 'forwardRef requires a render function but was given undefined.', + ); ReactNoop.render(
@@ -195,7 +197,7 @@ describe('useRef', () => { } expect(caughtError).toBeDefined(); expect(normalizeCodeLocInfo(caughtError.message)).toBe( - 'useRef requires a render function but was given undefined.' + + 'forwardRef requires a render function but was given undefined.' + (__DEV__ ? '\n in div (at **)' : ''), ); }); diff --git a/packages/react/src/forwardRef.js b/packages/react/src/forwardRef.js index 72b428cd7f8d0..cb05805ec1b08 100644 --- a/packages/react/src/forwardRef.js +++ b/packages/react/src/forwardRef.js @@ -9,12 +9,12 @@ import {REACT_USE_REF_TYPE} from 'shared/ReactSymbols'; import warning from 'fbjs/lib/warning'; -export default function useRef( +export default function forwardRef( renderProp: (props: Props, ref: React$ElementRef) => React$Node, ) { warning( typeof renderProp === 'function', - 'useRef requires a render function but was given %s.', + 'forwardRef requires a render function but was given %s.', renderProp === null ? 'null' : typeof renderProp, ); From d5a196d4bd96c2bc3fa5eee259269af246a7e057 Mon Sep 17 00:00:00 2001 From: Brian Vaughn Date: Mon, 12 Mar 2018 15:12:58 -0700 Subject: [PATCH 08/21] Changed implementation to avoid creating a fiber --- .../react-reconciler/src/ReactChildFiber.js | 65 ++++++++++++++++++- packages/react-reconciler/src/ReactFiber.js | 5 -- .../src/ReactFiberBeginWork.js | 20 ------ .../src/ReactFiberCompleteWork.js | 3 - .../src/__tests__/forwardRef-test.internal.js | 47 +++++++++++++- packages/react/src/forwardRef.js | 16 +++-- packages/shared/ReactTypeOfWork.js | 4 +- 7 files changed, 119 insertions(+), 41 deletions(-) diff --git a/packages/react-reconciler/src/ReactChildFiber.js b/packages/react-reconciler/src/ReactChildFiber.js index d3140340c261b..2f885eee6e915 100644 --- a/packages/react-reconciler/src/ReactChildFiber.js +++ b/packages/react-reconciler/src/ReactChildFiber.js @@ -19,6 +19,7 @@ import { REACT_ELEMENT_TYPE, REACT_FRAGMENT_TYPE, REACT_PORTAL_TYPE, + REACT_USE_REF_TYPE, } from 'shared/ReactSymbols'; import { FunctionalComponent, @@ -466,6 +467,26 @@ function ChildReconciler(shouldTrackSideEffects) { } if (typeof newChild === 'object' && newChild !== null) { + if ( + typeof newChild.type === 'object' && + newChild.type !== null && + newChild.type.$$typeof === REACT_USE_REF_TYPE + ) { + const renderFn = newChild.type.renderFn; + invariant( + typeof renderFn === 'function', + 'forwardRef requires a render function but was given %s.%s', + renderFn === null ? 'null' : typeof renderFn, + ReactDebugCurrentFiber.getCurrentFiberStackAddendum() || '', + ); + + return createChild( + returnFiber, + renderFn(newChild.props, newChild.ref), + expirationTime, + ); + } + switch (newChild.$$typeof) { case REACT_ELEMENT_TYPE: { const created = createFiberFromElement( @@ -1128,6 +1149,32 @@ function ChildReconciler(shouldTrackSideEffects) { child = child.sibling; } + if ( + typeof element.type === 'object' && + element.type !== null && + element.type.$$typeof === REACT_USE_REF_TYPE + ) { + const renderFn = element.type.renderFn; + invariant( + typeof renderFn === 'function', + 'forwardRef requires a render function but was given %s.%s', + renderFn === null ? 'null' : typeof renderFn, + ReactDebugCurrentFiber.getCurrentFiberStackAddendum() || '', + ); + + const newElement = renderFn(element.props, element.ref); + if (newElement == null) { + return null; + } + + return reconcileSingleElement( + returnFiber, + currentFirstChild, + newElement, + expirationTime, + ); + } + if (element.type === REACT_FRAGMENT_TYPE) { const created = createFiberFromFragment( element.props.children, @@ -1213,10 +1260,24 @@ function ChildReconciler(shouldTrackSideEffects) { if ( typeof newChild === 'object' && newChild !== null && - newChild.type === REACT_FRAGMENT_TYPE && newChild.key === null ) { - newChild = newChild.props.children; + if (newChild.type === REACT_FRAGMENT_TYPE) { + newChild = newChild.props.children; + } else if ( + typeof newChild.type === 'object' && + newChild.type !== null && + newChild.type.$$typeof === REACT_USE_REF_TYPE + ) { + const renderFn = newChild.type.renderFn; + invariant( + typeof renderFn === 'function', + 'forwardRef requires a render function but was given %s.%s', + renderFn === null ? 'null' : typeof renderFn, + ReactDebugCurrentFiber.getCurrentFiberStackAddendum() || '', + ); + newChild = renderFn(newChild.props, newChild.ref); + } } // Handle object types diff --git a/packages/react-reconciler/src/ReactFiber.js b/packages/react-reconciler/src/ReactFiber.js index 9a3711ab2e0d5..eea42369de7ac 100644 --- a/packages/react-reconciler/src/ReactFiber.js +++ b/packages/react-reconciler/src/ReactFiber.js @@ -29,7 +29,6 @@ import { Mode, ContextProvider, ContextConsumer, - UseRef, } from 'shared/ReactTypeOfWork'; import getComponentName from 'shared/getComponentName'; @@ -43,7 +42,6 @@ import { REACT_PROVIDER_TYPE, REACT_CONTEXT_TYPE, REACT_ASYNC_MODE_TYPE, - REACT_USE_REF_TYPE, } from 'shared/ReactSymbols'; let hasBadMapPolyfill; @@ -359,9 +357,6 @@ export function createFiberFromElement( // This is a consumer fiberTag = ContextConsumer; break; - case REACT_USE_REF_TYPE: - fiberTag = UseRef; - 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 5027cd6635fba..a5197b32025fd 100644 --- a/packages/react-reconciler/src/ReactFiberBeginWork.js +++ b/packages/react-reconciler/src/ReactFiberBeginWork.js @@ -30,7 +30,6 @@ import { Mode, ContextProvider, ContextConsumer, - UseRef, } from 'shared/ReactTypeOfWork'; import { PerformedWork, @@ -167,23 +166,6 @@ export default function( return workInProgress.child; } - function updateUseRef(current, workInProgress) { - const renderProp = workInProgress.type.renderProp; - invariant( - typeof renderProp === 'function', - 'forwardRef requires a render function but was given %s.%s', - renderProp === null ? 'null' : typeof renderProp, - ReactDebugCurrentFiber.getCurrentFiberStackAddendum() || '', - ); - const nextChildren = renderProp( - workInProgress.pendingProps, - workInProgress.ref, - ); - reconcileChildren(current, workInProgress, nextChildren); - memoizeProps(workInProgress, nextChildren); - return workInProgress.child; - } - function updateMode(current, workInProgress) { const nextChildren = workInProgress.pendingProps.children; if (hasLegacyContextChanged()) { @@ -1120,8 +1102,6 @@ export default function( return updateFragment(current, workInProgress); case Mode: return updateMode(current, workInProgress); - case UseRef: - return updateUseRef(current, workInProgress); case ContextProvider: return updateContextProvider( current, diff --git a/packages/react-reconciler/src/ReactFiberCompleteWork.js b/packages/react-reconciler/src/ReactFiberCompleteWork.js index dd4e8dd6d3b11..9e796dd53c152 100644 --- a/packages/react-reconciler/src/ReactFiberCompleteWork.js +++ b/packages/react-reconciler/src/ReactFiberCompleteWork.js @@ -34,7 +34,6 @@ import { ContextConsumer, Fragment, Mode, - UseRef, } from 'shared/ReactTypeOfWork'; import { Placement, @@ -618,8 +617,6 @@ export default function( return null; case ContextConsumer: return null; - case UseRef: - return null; // Error cases case IndeterminateComponent: invariant( diff --git a/packages/react/src/__tests__/forwardRef-test.internal.js b/packages/react/src/__tests__/forwardRef-test.internal.js index 35b97f8f49642..b8bb903949bc5 100644 --- a/packages/react/src/__tests__/forwardRef-test.internal.js +++ b/packages/react/src/__tests__/forwardRef-test.internal.js @@ -46,7 +46,7 @@ describe('forwardRef', () => { expect(ReactNoop.flush()).toEqual([123]); }); - it('should forward a ref to a chosen component', () => { + it('should forward a ref for a single child', () => { class Child extends React.Component { render() { ReactNoop.yield(this.props.value); @@ -69,6 +69,35 @@ describe('forwardRef', () => { expect(ref.value 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.value instanceof Child).toBe(true); + }); + it('should maintain ref through updates', () => { class Child extends React.Component { render() { @@ -163,6 +192,22 @@ describe('forwardRef', () => { expect(ref.value).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.value).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.', diff --git a/packages/react/src/forwardRef.js b/packages/react/src/forwardRef.js index cb05805ec1b08..1545f7c9e29ef 100644 --- a/packages/react/src/forwardRef.js +++ b/packages/react/src/forwardRef.js @@ -10,16 +10,18 @@ import {REACT_USE_REF_TYPE} from 'shared/ReactSymbols'; import warning from 'fbjs/lib/warning'; export default function forwardRef( - renderProp: (props: Props, ref: React$ElementRef) => React$Node, + renderFn: (props: Props, ref: React$ElementRef) => React$Node, ) { - warning( - typeof renderProp === 'function', - 'forwardRef requires a render function but was given %s.', - renderProp === null ? 'null' : typeof renderProp, - ); + if (__DEV__) { + warning( + typeof renderFn === 'function', + 'forwardRef requires a render function but was given %s.', + renderFn === null ? 'null' : typeof renderFn, + ); + } return { $$typeof: REACT_USE_REF_TYPE, - renderProp, + renderFn, }; } diff --git a/packages/shared/ReactTypeOfWork.js b/packages/shared/ReactTypeOfWork.js index 598a1a2f30150..3d1bfc3f37584 100644 --- a/packages/shared/ReactTypeOfWork.js +++ b/packages/shared/ReactTypeOfWork.js @@ -21,8 +21,7 @@ export type TypeOfWork = | 10 | 11 | 12 - | 13 - | 14; + | 13; export const IndeterminateComponent = 0; // Before we know whether it is functional or class export const FunctionalComponent = 1; @@ -38,4 +37,3 @@ export const Fragment = 10; export const Mode = 11; export const ContextConsumer = 12; export const ContextProvider = 13; -export const UseRef = 14; From 3583bf64e3f10cd2fbfc69c762bc6aadc6518d15 Mon Sep 17 00:00:00 2001 From: Brian Vaughn Date: Mon, 12 Mar 2018 15:14:02 -0700 Subject: [PATCH 09/21] USE_REF => FORWARD_REF --- packages/react-is/src/ReactIs.js | 10 +++++----- packages/react-is/src/__tests__/ReactIs-test.js | 10 +++++----- packages/react-reconciler/src/ReactChildFiber.js | 8 ++++---- packages/react/src/ReactElementValidator.js | 4 ++-- packages/react/src/forwardRef.js | 4 ++-- packages/shared/ReactSymbols.js | 2 +- 6 files changed, 19 insertions(+), 19 deletions(-) diff --git a/packages/react-is/src/ReactIs.js b/packages/react-is/src/ReactIs.js index c61164b17f52a..2c71e4c871df1 100644 --- a/packages/react-is/src/ReactIs.js +++ b/packages/react-is/src/ReactIs.js @@ -17,7 +17,7 @@ import { REACT_PORTAL_TYPE, REACT_PROVIDER_TYPE, REACT_STRICT_MODE_TYPE, - REACT_USE_REF_TYPE, + REACT_FORWARD_REF_TYPE, } from 'shared/ReactSymbols'; export function typeOf(object: any) { @@ -39,7 +39,7 @@ export function typeOf(object: any) { switch ($$typeofType) { case REACT_CONTEXT_TYPE: case REACT_PROVIDER_TYPE: - case REACT_USE_REF_TYPE: + case REACT_FORWARD_REF_TYPE: return $$typeofType; default: return $$typeof; @@ -60,7 +60,7 @@ export const Element = REACT_ELEMENT_TYPE; export const Fragment = REACT_FRAGMENT_TYPE; export const Portal = REACT_PORTAL_TYPE; export const StrictMode = REACT_STRICT_MODE_TYPE; -export const UseRef = REACT_USE_REF_TYPE; +export const ForwardRef = REACT_FORWARD_REF_TYPE; export function isAsyncMode(object: any) { return typeOf(object) === REACT_ASYNC_MODE_TYPE; @@ -87,6 +87,6 @@ export function isPortal(object: any) { export function isStrictMode(object: any) { return typeOf(object) === REACT_STRICT_MODE_TYPE; } -export function isUseRef(object: any) { - return typeOf(object) === REACT_USE_REF_TYPE; +export function isForwardRef(object: any) { + return typeOf(object) === REACT_FORWARD_REF_TYPE; } diff --git a/packages/react-is/src/__tests__/ReactIs-test.js b/packages/react-is/src/__tests__/ReactIs-test.js index 3d32536f21fd7..c5a529e4d1fe1 100644 --- a/packages/react-is/src/__tests__/ReactIs-test.js +++ b/packages/react-is/src/__tests__/ReactIs-test.js @@ -103,10 +103,10 @@ describe('ReactIs', () => { it('should identify ref forwarding component', () => { const RefForwardingComponent = React.forwardRef((props, ref) => null); - expect(ReactIs.typeOf()).toBe(ReactIs.UseRef); - expect(ReactIs.isUseRef()).toBe(true); - expect(ReactIs.isUseRef({type: ReactIs.StrictMode})).toBe(false); - expect(ReactIs.isUseRef()).toBe(false); - expect(ReactIs.isUseRef(
)).toBe(false); + 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); }); }); diff --git a/packages/react-reconciler/src/ReactChildFiber.js b/packages/react-reconciler/src/ReactChildFiber.js index 2f885eee6e915..9e5d79f946928 100644 --- a/packages/react-reconciler/src/ReactChildFiber.js +++ b/packages/react-reconciler/src/ReactChildFiber.js @@ -19,7 +19,7 @@ import { REACT_ELEMENT_TYPE, REACT_FRAGMENT_TYPE, REACT_PORTAL_TYPE, - REACT_USE_REF_TYPE, + REACT_FORWARD_REF_TYPE, } from 'shared/ReactSymbols'; import { FunctionalComponent, @@ -470,7 +470,7 @@ function ChildReconciler(shouldTrackSideEffects) { if ( typeof newChild.type === 'object' && newChild.type !== null && - newChild.type.$$typeof === REACT_USE_REF_TYPE + newChild.type.$$typeof === REACT_FORWARD_REF_TYPE ) { const renderFn = newChild.type.renderFn; invariant( @@ -1152,7 +1152,7 @@ function ChildReconciler(shouldTrackSideEffects) { if ( typeof element.type === 'object' && element.type !== null && - element.type.$$typeof === REACT_USE_REF_TYPE + element.type.$$typeof === REACT_FORWARD_REF_TYPE ) { const renderFn = element.type.renderFn; invariant( @@ -1267,7 +1267,7 @@ function ChildReconciler(shouldTrackSideEffects) { } else if ( typeof newChild.type === 'object' && newChild.type !== null && - newChild.type.$$typeof === REACT_USE_REF_TYPE + newChild.type.$$typeof === REACT_FORWARD_REF_TYPE ) { const renderFn = newChild.type.renderFn; invariant( diff --git a/packages/react/src/ReactElementValidator.js b/packages/react/src/ReactElementValidator.js index 6ce0a734bc5ea..fff09423d3e04 100644 --- a/packages/react/src/ReactElementValidator.js +++ b/packages/react/src/ReactElementValidator.js @@ -22,7 +22,7 @@ import { REACT_ASYNC_MODE_TYPE, REACT_PROVIDER_TYPE, REACT_CONTEXT_TYPE, - REACT_USE_REF_TYPE, + REACT_FORWARD_REF_TYPE, } from 'shared/ReactSymbols'; import checkPropTypes from 'prop-types/checkPropTypes'; import warning from 'fbjs/lib/warning'; @@ -299,7 +299,7 @@ export function createElementWithValidation(type, props, children) { type !== null && (type.$$typeof === REACT_PROVIDER_TYPE || type.$$typeof === REACT_CONTEXT_TYPE || - type.$$typeof === REACT_USE_REF_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/forwardRef.js b/packages/react/src/forwardRef.js index 1545f7c9e29ef..c3246fcd76da5 100644 --- a/packages/react/src/forwardRef.js +++ b/packages/react/src/forwardRef.js @@ -5,7 +5,7 @@ * LICENSE file in the root directory of this source tree. */ -import {REACT_USE_REF_TYPE} from 'shared/ReactSymbols'; +import {REACT_FORWARD_REF_TYPE} from 'shared/ReactSymbols'; import warning from 'fbjs/lib/warning'; @@ -21,7 +21,7 @@ export default function forwardRef( } return { - $$typeof: REACT_USE_REF_TYPE, + $$typeof: REACT_FORWARD_REF_TYPE, renderFn, }; } diff --git a/packages/shared/ReactSymbols.js b/packages/shared/ReactSymbols.js index 9c7308af7a302..5d2e2e9ffb2c3 100644 --- a/packages/shared/ReactSymbols.js +++ b/packages/shared/ReactSymbols.js @@ -36,7 +36,7 @@ export const REACT_CONTEXT_TYPE = hasSymbol export const REACT_ASYNC_MODE_TYPE = hasSymbol ? Symbol.for('react.async_mode') : 0xeacf; -export const REACT_USE_REF_TYPE = hasSymbol +export const REACT_FORWARD_REF_TYPE = hasSymbol ? Symbol.for('react.use_ref') : 0xead0; From 7c3d82302610f61717a6f4d9ed4b7f1dc261bf35 Mon Sep 17 00:00:00 2001 From: Brian Vaughn Date: Mon, 12 Mar 2018 15:40:33 -0700 Subject: [PATCH 10/21] Removed recursion from ReactChildFiber for forwardRef --- .../react-reconciler/src/ReactChildFiber.js | 122 +++++++++--------- .../src/__tests__/forwardRef-test.internal.js | 14 +- 2 files changed, 72 insertions(+), 64 deletions(-) diff --git a/packages/react-reconciler/src/ReactChildFiber.js b/packages/react-reconciler/src/ReactChildFiber.js index 9e5d79f946928..a0e242362282f 100644 --- a/packages/react-reconciler/src/ReactChildFiber.js +++ b/packages/react-reconciler/src/ReactChildFiber.js @@ -453,6 +453,28 @@ function ChildReconciler(shouldTrackSideEffects) { newChild: any, expirationTime: ExpirationTime, ): Fiber | null { + if ( + typeof newChild === 'object' && + newChild !== null && + typeof newChild.type === 'object' && + newChild.type !== null && + newChild.type.$$typeof === REACT_FORWARD_REF_TYPE + ) { + const renderFn = newChild.type.renderFn; + invariant( + typeof renderFn === 'function', + 'forwardRef requires a render function but was given %s.%s', + renderFn === null ? 'null' : typeof renderFn, + ReactDebugCurrentFiber.getCurrentFiberStackAddendum() || '', + ); + + return createChild( + returnFiber, + renderFn(newChild.props, newChild.ref), + expirationTime, + ); + } + if (typeof newChild === 'string' || typeof newChild === 'number') { // Text nodes don't have keys. If the previous node is implicitly keyed // we can continue to replace it without aborting even if it is not a text @@ -467,26 +489,6 @@ function ChildReconciler(shouldTrackSideEffects) { } if (typeof newChild === 'object' && newChild !== null) { - if ( - typeof newChild.type === 'object' && - newChild.type !== null && - newChild.type.$$typeof === REACT_FORWARD_REF_TYPE - ) { - const renderFn = newChild.type.renderFn; - invariant( - typeof renderFn === 'function', - 'forwardRef requires a render function but was given %s.%s', - renderFn === null ? 'null' : typeof renderFn, - ReactDebugCurrentFiber.getCurrentFiberStackAddendum() || '', - ); - - return createChild( - returnFiber, - renderFn(newChild.props, newChild.ref), - expirationTime, - ); - } - switch (newChild.$$typeof) { case REACT_ELEMENT_TYPE: { const created = createFiberFromElement( @@ -1115,6 +1117,23 @@ function ChildReconciler(shouldTrackSideEffects) { ): Fiber { const key = element.key; let child = currentFirstChild; + + if ( + typeof element.type === 'object' && + element.type !== null && + element.type.$$typeof === REACT_FORWARD_REF_TYPE + ) { + const renderFn = element.type.renderFn; + invariant( + typeof renderFn === 'function', + 'forwardRef requires a render function but was given %s.%s', + renderFn === null ? 'null' : typeof renderFn, + ReactDebugCurrentFiber.getCurrentFiberStackAddendum() || '', + ); + + element = renderFn(element.props, element.ref); + } + while (child !== null) { // TODO: If key === null and child.key === null, then this only applies to // the first item in the list. @@ -1149,32 +1168,6 @@ function ChildReconciler(shouldTrackSideEffects) { child = child.sibling; } - if ( - typeof element.type === 'object' && - element.type !== null && - element.type.$$typeof === REACT_FORWARD_REF_TYPE - ) { - const renderFn = element.type.renderFn; - invariant( - typeof renderFn === 'function', - 'forwardRef requires a render function but was given %s.%s', - renderFn === null ? 'null' : typeof renderFn, - ReactDebugCurrentFiber.getCurrentFiberStackAddendum() || '', - ); - - const newElement = renderFn(element.props, element.ref); - if (newElement == null) { - return null; - } - - return reconcileSingleElement( - returnFiber, - currentFirstChild, - newElement, - expirationTime, - ); - } - if (element.type === REACT_FRAGMENT_TYPE) { const created = createFiberFromFragment( element.props.children, @@ -1254,30 +1247,33 @@ function ChildReconciler(shouldTrackSideEffects) { // not as a fragment. Nested arrays on the other hand will be treated as // fragment nodes. Recursion happens at the normal flow. + if ( + typeof newChild === 'object' && + newChild !== null && + typeof newChild.type === 'object' && + newChild.type !== null && + newChild.type.$$typeof === REACT_FORWARD_REF_TYPE + ) { + const renderFn = newChild.type.renderFn; + invariant( + typeof renderFn === 'function', + 'forwardRef requires a render function but was given %s.%s', + renderFn === null ? 'null' : typeof renderFn, + ReactDebugCurrentFiber.getCurrentFiberStackAddendum() || '', + ); + newChild = renderFn(newChild.props, newChild.ref); + } + // Handle top level unkeyed fragments as if they were arrays. // This leads to an ambiguity between <>{[...]} and <>.... // We treat the ambiguous cases above the same. if ( typeof newChild === 'object' && newChild !== null && - newChild.key === null + newChild.key === null && + newChild.type === REACT_FRAGMENT_TYPE ) { - if (newChild.type === REACT_FRAGMENT_TYPE) { - newChild = newChild.props.children; - } else if ( - typeof newChild.type === 'object' && - newChild.type !== null && - newChild.type.$$typeof === REACT_FORWARD_REF_TYPE - ) { - const renderFn = newChild.type.renderFn; - invariant( - typeof renderFn === 'function', - 'forwardRef requires a render function but was given %s.%s', - renderFn === null ? 'null' : typeof renderFn, - ReactDebugCurrentFiber.getCurrentFiberStackAddendum() || '', - ); - newChild = renderFn(newChild.props, newChild.ref); - } + newChild = newChild.props.children; } // Handle object types diff --git a/packages/react/src/__tests__/forwardRef-test.internal.js b/packages/react/src/__tests__/forwardRef-test.internal.js index b8bb903949bc5..181792efe6c5f 100644 --- a/packages/react/src/__tests__/forwardRef-test.internal.js +++ b/packages/react/src/__tests__/forwardRef-test.internal.js @@ -98,8 +98,13 @@ describe('forwardRef', () => { expect(ref.value instanceof Child).toBe(true); }); - it('should maintain ref through updates', () => { + it('should maintain child instance and ref through updates', () => { + let childInstantiatedCount = 0; class Child extends React.Component { + constructor(props) { + super(props); + childInstantiatedCount++; + } render() { ReactNoop.yield(this.props.value); return null; @@ -124,10 +129,17 @@ describe('forwardRef', () => { ReactNoop.render(); expect(ReactNoop.flush()).toEqual([123]); + expect(childInstantiatedCount).toBe(1); expect(ref instanceof Child).toBe(true); expect(setRefCount).toBe(1); ReactNoop.render(); expect(ReactNoop.flush()).toEqual([456]); + expect(childInstantiatedCount).toBe(1); + expect(ref instanceof Child).toBe(true); + expect(setRefCount).toBe(1); + ReactNoop.render(); + expect(ReactNoop.flush()).toEqual([789]); + expect(childInstantiatedCount).toBe(1); expect(ref instanceof Child).toBe(true); expect(setRefCount).toBe(1); }); From 6185838fbf9902ac0ae832331367ec1c06a4fdb2 Mon Sep 17 00:00:00 2001 From: Brian Vaughn Date: Mon, 12 Mar 2018 15:42:45 -0700 Subject: [PATCH 11/21] Reverted accidental line reordering --- packages/react-reconciler/src/ReactChildFiber.js | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/packages/react-reconciler/src/ReactChildFiber.js b/packages/react-reconciler/src/ReactChildFiber.js index a0e242362282f..283e4080bd757 100644 --- a/packages/react-reconciler/src/ReactChildFiber.js +++ b/packages/react-reconciler/src/ReactChildFiber.js @@ -1270,8 +1270,8 @@ function ChildReconciler(shouldTrackSideEffects) { if ( typeof newChild === 'object' && newChild !== null && - newChild.key === null && - newChild.type === REACT_FRAGMENT_TYPE + newChild.type === REACT_FRAGMENT_TYPE && + newChild.key === null ) { newChild = newChild.props.children; } From 6d7d1ef2c358f55973cad0f21b051fa07bc2c89d Mon Sep 17 00:00:00 2001 From: Brian Vaughn Date: Mon, 12 Mar 2018 16:06:55 -0700 Subject: [PATCH 12/21] forwardRef is now a new fiber/type again --- packages/react-is/src/ReactIs.js | 12 ++-- .../react-is/src/__tests__/ReactIs-test.js | 18 +++--- .../react-reconciler/src/ReactChildFiber.js | 57 ------------------- packages/react-reconciler/src/ReactFiber.js | 5 ++ .../src/ReactFiberBeginWork.js | 20 +++++++ .../src/ReactFiberCompleteWork.js | 3 + .../src/__tests__/forwardRef-test.internal.js | 9 --- packages/shared/ReactTypeOfWork.js | 4 +- 8 files changed, 46 insertions(+), 82 deletions(-) diff --git a/packages/react-is/src/ReactIs.js b/packages/react-is/src/ReactIs.js index 2c71e4c871df1..0265055e020de 100644 --- a/packages/react-is/src/ReactIs.js +++ b/packages/react-is/src/ReactIs.js @@ -13,11 +13,11 @@ import { REACT_ASYNC_MODE_TYPE, REACT_CONTEXT_TYPE, REACT_ELEMENT_TYPE, + REACT_FORWARD_REF_TYPE, REACT_FRAGMENT_TYPE, REACT_PORTAL_TYPE, REACT_PROVIDER_TYPE, REACT_STRICT_MODE_TYPE, - REACT_FORWARD_REF_TYPE, } from 'shared/ReactSymbols'; export function typeOf(object: any) { @@ -38,8 +38,8 @@ export function typeOf(object: any) { switch ($$typeofType) { case REACT_CONTEXT_TYPE: - case REACT_PROVIDER_TYPE: case REACT_FORWARD_REF_TYPE: + case REACT_PROVIDER_TYPE: return $$typeofType; default: return $$typeof; @@ -57,10 +57,10 @@ 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; -export const ForwardRef = REACT_FORWARD_REF_TYPE; export function isAsyncMode(object: any) { return typeOf(object) === REACT_ASYNC_MODE_TYPE; @@ -78,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; } @@ -87,6 +90,3 @@ export function isPortal(object: any) { export function isStrictMode(object: any) { return typeOf(object) === REACT_STRICT_MODE_TYPE; } -export function isForwardRef(object: any) { - return typeOf(object) === REACT_FORWARD_REF_TYPE; -} diff --git a/packages/react-is/src/__tests__/ReactIs-test.js b/packages/react-is/src/__tests__/ReactIs-test.js index c5a529e4d1fe1..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); @@ -100,13 +109,4 @@ describe('ReactIs', () => { expect(ReactIs.isStrictMode()).toBe(false); expect(ReactIs.isStrictMode(
)).toBe(false); }); - - 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); - }); }); diff --git a/packages/react-reconciler/src/ReactChildFiber.js b/packages/react-reconciler/src/ReactChildFiber.js index 283e4080bd757..d3140340c261b 100644 --- a/packages/react-reconciler/src/ReactChildFiber.js +++ b/packages/react-reconciler/src/ReactChildFiber.js @@ -19,7 +19,6 @@ import { REACT_ELEMENT_TYPE, REACT_FRAGMENT_TYPE, REACT_PORTAL_TYPE, - REACT_FORWARD_REF_TYPE, } from 'shared/ReactSymbols'; import { FunctionalComponent, @@ -453,28 +452,6 @@ function ChildReconciler(shouldTrackSideEffects) { newChild: any, expirationTime: ExpirationTime, ): Fiber | null { - if ( - typeof newChild === 'object' && - newChild !== null && - typeof newChild.type === 'object' && - newChild.type !== null && - newChild.type.$$typeof === REACT_FORWARD_REF_TYPE - ) { - const renderFn = newChild.type.renderFn; - invariant( - typeof renderFn === 'function', - 'forwardRef requires a render function but was given %s.%s', - renderFn === null ? 'null' : typeof renderFn, - ReactDebugCurrentFiber.getCurrentFiberStackAddendum() || '', - ); - - return createChild( - returnFiber, - renderFn(newChild.props, newChild.ref), - expirationTime, - ); - } - if (typeof newChild === 'string' || typeof newChild === 'number') { // Text nodes don't have keys. If the previous node is implicitly keyed // we can continue to replace it without aborting even if it is not a text @@ -1117,23 +1094,6 @@ function ChildReconciler(shouldTrackSideEffects) { ): Fiber { const key = element.key; let child = currentFirstChild; - - if ( - typeof element.type === 'object' && - element.type !== null && - element.type.$$typeof === REACT_FORWARD_REF_TYPE - ) { - const renderFn = element.type.renderFn; - invariant( - typeof renderFn === 'function', - 'forwardRef requires a render function but was given %s.%s', - renderFn === null ? 'null' : typeof renderFn, - ReactDebugCurrentFiber.getCurrentFiberStackAddendum() || '', - ); - - element = renderFn(element.props, element.ref); - } - while (child !== null) { // TODO: If key === null and child.key === null, then this only applies to // the first item in the list. @@ -1247,23 +1207,6 @@ function ChildReconciler(shouldTrackSideEffects) { // not as a fragment. Nested arrays on the other hand will be treated as // fragment nodes. Recursion happens at the normal flow. - if ( - typeof newChild === 'object' && - newChild !== null && - typeof newChild.type === 'object' && - newChild.type !== null && - newChild.type.$$typeof === REACT_FORWARD_REF_TYPE - ) { - const renderFn = newChild.type.renderFn; - invariant( - typeof renderFn === 'function', - 'forwardRef requires a render function but was given %s.%s', - renderFn === null ? 'null' : typeof renderFn, - ReactDebugCurrentFiber.getCurrentFiberStackAddendum() || '', - ); - newChild = renderFn(newChild.props, newChild.ref); - } - // Handle top level unkeyed fragments as if they were arrays. // This leads to an ambiguity between <>{[...]} and <>.... // We treat the ambiguous cases above the same. 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..cd181f27394d0 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,23 @@ export default function( } } + function updateForwardRef(current, workInProgress) { + const renderFn = workInProgress.type.renderFn; + invariant( + typeof renderFn === 'function', + 'forwardRef requires a render function but was given %s.%s', + renderFn === null ? 'null' : typeof renderFn, + ReactDebugCurrentFiber.getCurrentFiberStackAddendum() || '', + ); + const nextChildren = renderFn( + 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 +1148,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/__tests__/forwardRef-test.internal.js b/packages/react/src/__tests__/forwardRef-test.internal.js index 181792efe6c5f..f5f469f3d4489 100644 --- a/packages/react/src/__tests__/forwardRef-test.internal.js +++ b/packages/react/src/__tests__/forwardRef-test.internal.js @@ -99,11 +99,9 @@ describe('forwardRef', () => { }); it('should maintain child instance and ref through updates', () => { - let childInstantiatedCount = 0; class Child extends React.Component { constructor(props) { super(props); - childInstantiatedCount++; } render() { ReactNoop.yield(this.props.value); @@ -129,17 +127,10 @@ describe('forwardRef', () => { ReactNoop.render(); expect(ReactNoop.flush()).toEqual([123]); - expect(childInstantiatedCount).toBe(1); expect(ref instanceof Child).toBe(true); expect(setRefCount).toBe(1); ReactNoop.render(); expect(ReactNoop.flush()).toEqual([456]); - expect(childInstantiatedCount).toBe(1); - expect(ref instanceof Child).toBe(true); - expect(setRefCount).toBe(1); - ReactNoop.render(); - expect(ReactNoop.flush()).toEqual([789]); - expect(childInstantiatedCount).toBe(1); expect(ref instanceof Child).toBe(true); expect(setRefCount).toBe(1); }); 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; From 532136504cdceff2249968806387ba77cdf36eba Mon Sep 17 00:00:00 2001 From: Brian Vaughn Date: Wed, 14 Mar 2018 10:36:47 -0700 Subject: [PATCH 13/21] Renamed renderFn to render --- packages/react-reconciler/src/ReactFiberBeginWork.js | 8 ++++---- packages/react/src/forwardRef.js | 8 ++++---- 2 files changed, 8 insertions(+), 8 deletions(-) diff --git a/packages/react-reconciler/src/ReactFiberBeginWork.js b/packages/react-reconciler/src/ReactFiberBeginWork.js index cd181f27394d0..552319c5bc71c 100644 --- a/packages/react-reconciler/src/ReactFiberBeginWork.js +++ b/packages/react-reconciler/src/ReactFiberBeginWork.js @@ -155,14 +155,14 @@ export default function( } function updateForwardRef(current, workInProgress) { - const renderFn = workInProgress.type.renderFn; + const render = workInProgress.type.render; invariant( - typeof renderFn === 'function', + typeof render === 'function', 'forwardRef requires a render function but was given %s.%s', - renderFn === null ? 'null' : typeof renderFn, + render === null ? 'null' : typeof render, ReactDebugCurrentFiber.getCurrentFiberStackAddendum() || '', ); - const nextChildren = renderFn( + const nextChildren = render( workInProgress.pendingProps, workInProgress.ref, ); diff --git a/packages/react/src/forwardRef.js b/packages/react/src/forwardRef.js index c3246fcd76da5..6a923be12795d 100644 --- a/packages/react/src/forwardRef.js +++ b/packages/react/src/forwardRef.js @@ -10,18 +10,18 @@ import {REACT_FORWARD_REF_TYPE} from 'shared/ReactSymbols'; import warning from 'fbjs/lib/warning'; export default function forwardRef( - renderFn: (props: Props, ref: React$ElementRef) => React$Node, + render: (props: Props, ref: React$ElementRef) => React$Node, ) { if (__DEV__) { warning( - typeof renderFn === 'function', + typeof render === 'function', 'forwardRef requires a render function but was given %s.', - renderFn === null ? 'null' : typeof renderFn, + render === null ? 'null' : typeof render, ); } return { $$typeof: REACT_FORWARD_REF_TYPE, - renderFn, + render, }; } From 557cd79ed7a021bd61c6cd76942fbfd4ba682332 Mon Sep 17 00:00:00 2001 From: Brian Vaughn Date: Wed, 14 Mar 2018 10:38:23 -0700 Subject: [PATCH 14/21] Removed second invariant --- .../src/ReactFiberBeginWork.js | 9 +----- .../src/__tests__/forwardRef-test.internal.js | 29 ++----------------- 2 files changed, 3 insertions(+), 35 deletions(-) diff --git a/packages/react-reconciler/src/ReactFiberBeginWork.js b/packages/react-reconciler/src/ReactFiberBeginWork.js index 552319c5bc71c..25de8a7daf386 100644 --- a/packages/react-reconciler/src/ReactFiberBeginWork.js +++ b/packages/react-reconciler/src/ReactFiberBeginWork.js @@ -155,14 +155,7 @@ export default function( } function updateForwardRef(current, workInProgress) { - const render = workInProgress.type.render; - invariant( - typeof render === 'function', - 'forwardRef requires a render function but was given %s.%s', - render === null ? 'null' : typeof render, - ReactDebugCurrentFiber.getCurrentFiberStackAddendum() || '', - ); - const nextChildren = render( + const nextChildren = workInProgress.type.render( workInProgress.pendingProps, workInProgress.ref, ); diff --git a/packages/react/src/__tests__/forwardRef-test.internal.js b/packages/react/src/__tests__/forwardRef-test.internal.js index f5f469f3d4489..72e88017fa1f9 100644 --- a/packages/react/src/__tests__/forwardRef-test.internal.js +++ b/packages/react/src/__tests__/forwardRef-test.internal.js @@ -14,10 +14,6 @@ describe('forwardRef', () => { let ReactFeatureFlags; let ReactNoop; - function normalizeCodeLocInfo(str) { - return str && str.replace(/\(at .+?:\d+\)/g, '(at **)'); - } - beforeEach(() => { jest.resetModules(); ReactFeatureFlags = require('shared/ReactFeatureFlags'); @@ -223,30 +219,9 @@ describe('forwardRef', () => { ); }); - it('should error with a callstack if rendered without a function', () => { - let RefForwardingComponent; - expect(() => { - RefForwardingComponent = React.forwardRef(); - }).toWarnDev( + it('should warn if no render function is provided', () => { + expect(React.forwardRef).toWarnDev( 'forwardRef requires a render function but was given undefined.', ); - - ReactNoop.render( -
- -
, - ); - - let caughtError; - try { - ReactNoop.flush(); - } catch (error) { - caughtError = error; - } - expect(caughtError).toBeDefined(); - expect(normalizeCodeLocInfo(caughtError.message)).toBe( - 'forwardRef requires a render function but was given undefined.' + - (__DEV__ ? '\n in div (at **)' : ''), - ); }); }); From 04ec7207a6dffc4fd851031e69b5819293a003e9 Mon Sep 17 00:00:00 2001 From: Brian Vaughn Date: Wed, 14 Mar 2018 11:11:14 -0700 Subject: [PATCH 15/21] Added forwardRef type to server renderer --- .../ReactDOMServerIntegrationRefs-test.js | 19 ++++++++++++++++++ .../src/server/ReactPartialRenderer.js | 20 +++++++++++++++++++ 2 files changed, 39 insertions(+) diff --git a/packages/react-dom/src/__tests__/ReactDOMServerIntegrationRefs-test.js b/packages/react-dom/src/__tests__/ReactDOMServerIntegrationRefs-test.js index 974d69497d8c0..058ca9cfd6a53 100644 --- a/packages/react-dom/src/__tests__/ReactDOMServerIntegrationRefs-test.js +++ b/packages/react-dom/src/__tests__/ReactDOMServerIntegrationRefs-test.js @@ -96,4 +96,23 @@ 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
hello
; + } + } + + const OuterComponent = React.forwardRef((props, ref) => ( + + )); + + await clientRenderOnServerString(); + + expect(divRef.value).not.toBe(null); + expect(divRef.value.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; From 1b1deda762fffc56c12a1e48f60ac137e20a7d4a Mon Sep 17 00:00:00 2001 From: Brian Vaughn Date: Wed, 14 Mar 2018 11:19:59 -0700 Subject: [PATCH 16/21] Slightly beefed up SSR forwardRef test case --- .../src/__tests__/ReactDOMServerIntegrationRefs-test.js | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/packages/react-dom/src/__tests__/ReactDOMServerIntegrationRefs-test.js b/packages/react-dom/src/__tests__/ReactDOMServerIntegrationRefs-test.js index 058ca9cfd6a53..ad3dd46792bd6 100644 --- a/packages/react-dom/src/__tests__/ReactDOMServerIntegrationRefs-test.js +++ b/packages/react-dom/src/__tests__/ReactDOMServerIntegrationRefs-test.js @@ -102,7 +102,7 @@ describe('ReactDOMServerIntegration', () => { class InnerComponent extends React.Component { render() { - return
hello
; + return
{this.props.value}
; } } @@ -110,7 +110,9 @@ describe('ReactDOMServerIntegration', () => { )); - await clientRenderOnServerString(); + await clientRenderOnServerString( + , + ); expect(divRef.value).not.toBe(null); expect(divRef.value.textContent).toBe('hello'); From b91b3a35a3f35c65d0808b3ff13f6644dc386b4e Mon Sep 17 00:00:00 2001 From: Brian Vaughn Date: Wed, 14 Mar 2018 11:29:06 -0700 Subject: [PATCH 17/21] Symbol.for react.use_ref => react.forward_ref --- packages/shared/ReactSymbols.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/shared/ReactSymbols.js b/packages/shared/ReactSymbols.js index 5d2e2e9ffb2c3..12e0fdddad9b9 100644 --- a/packages/shared/ReactSymbols.js +++ b/packages/shared/ReactSymbols.js @@ -37,7 +37,7 @@ export const REACT_ASYNC_MODE_TYPE = hasSymbol ? Symbol.for('react.async_mode') : 0xeacf; export const REACT_FORWARD_REF_TYPE = hasSymbol - ? Symbol.for('react.use_ref') + ? Symbol.for('react.forward_ref') : 0xead0; const MAYBE_ITERATOR_SYMBOL = typeof Symbol === 'function' && Symbol.iterator; From c038d8973ec099f28768634ba138b539a037ba35 Mon Sep 17 00:00:00 2001 From: Brian Vaughn Date: Wed, 14 Mar 2018 11:33:40 -0700 Subject: [PATCH 18/21] Added test to ensure refs can be forwarded between components --- .../src/__tests__/forwardRef-test.internal.js | 25 +++++++++++++++++++ 1 file changed, 25 insertions(+) diff --git a/packages/react/src/__tests__/forwardRef-test.internal.js b/packages/react/src/__tests__/forwardRef-test.internal.js index 72e88017fa1f9..2f91e5f64d6eb 100644 --- a/packages/react/src/__tests__/forwardRef-test.internal.js +++ b/packages/react/src/__tests__/forwardRef-test.internal.js @@ -94,6 +94,31 @@ describe('forwardRef', () => { expect(ref.value 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.value.type).toBe('div'); + + ReactNoop.render(); + ReactNoop.flush(); + expect(ref.value.type).toBe('span'); + }); + it('should maintain child instance and ref through updates', () => { class Child extends React.Component { constructor(props) { From 5b5b625a055a86a96198babf90f47034497204f4 Mon Sep 17 00:00:00 2001 From: Brian Vaughn Date: Wed, 14 Mar 2018 11:34:35 -0700 Subject: [PATCH 19/21] Read render to prop before calling --- packages/react-reconciler/src/ReactFiberBeginWork.js | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/packages/react-reconciler/src/ReactFiberBeginWork.js b/packages/react-reconciler/src/ReactFiberBeginWork.js index 25de8a7daf386..039fe24032e76 100644 --- a/packages/react-reconciler/src/ReactFiberBeginWork.js +++ b/packages/react-reconciler/src/ReactFiberBeginWork.js @@ -155,7 +155,8 @@ export default function( } function updateForwardRef(current, workInProgress) { - const nextChildren = workInProgress.type.render( + const render = workInProgress.type.render; + const nextChildren = render( workInProgress.pendingProps, workInProgress.ref, ); From 2a7b3c87c34ba92a290855854bb5eebced84dbf8 Mon Sep 17 00:00:00 2001 From: Brian Vaughn Date: Wed, 14 Mar 2018 12:48:54 -0700 Subject: [PATCH 20/21] Merged master --- packages/create-subscription/README.md | 184 +++++++ packages/create-subscription/index.js | 12 + packages/create-subscription/npm/index.js | 7 + packages/create-subscription/package.json | 21 + .../createSubscription-test.internal.js | 457 ++++++++++++++++++ .../src/createSubscription.js | 159 ++++++ .../src/__tests__/ReactComponent-test.js | 4 +- .../ReactErrorBoundaries-test.internal.js | 6 +- .../src/ReactFiberClassComponent.js | 16 +- .../src/ReactFiberCommitWork.js | 19 +- ...tIncrementalErrorHandling-test.internal.js | 43 ++ packages/react/src/ReactCreateRef.js | 2 +- .../src/__tests__/ReactCreateRef-test.js | 60 +++ packages/shared/ReactTypes.js | 2 +- scripts/rollup/bundles.js | 10 + scripts/rollup/results.json | 204 ++++---- yarn.lock | 10 + 17 files changed, 1111 insertions(+), 105 deletions(-) create mode 100644 packages/create-subscription/README.md create mode 100644 packages/create-subscription/index.js create mode 100644 packages/create-subscription/npm/index.js create mode 100644 packages/create-subscription/package.json create mode 100644 packages/create-subscription/src/__tests__/createSubscription-test.internal.js create mode 100644 packages/create-subscription/src/createSubscription.js create mode 100644 packages/react/src/__tests__/ReactCreateRef-test.js diff --git a/packages/create-subscription/README.md b/packages/create-subscription/README.md new file mode 100644 index 0000000000000..a55210d1d4b89 --- /dev/null +++ b/packages/create-subscription/README.md @@ -0,0 +1,184 @@ +# create-subscription + +`create-subscription` provides an async-safe interface to manage a subscription. + +## When should you NOT use this? + +This utility should be used for subscriptions to a single value that are typically only read in one place and may update frequently (e.g. a component that subscribes to a geolocation API to show a dot on a map). + +Other cases have **better long-term solutions**: +* Redux/Flux stores should use the [context API](https://reactjs.org/docs/context.html) instead. +* I/O subscriptions (e.g. notifications) that update infrequently should use [`simple-cache-provider`](https://github.com/facebook/react/blob/master/packages/simple-cache-provider/README.md) instead. +* Complex libraries like Relay/Apollo should manage subscriptions manually with the same techniques which this library uses under the hood (as referenced [here](https://gist.github.com/bvaughn/d569177d70b50b58bff69c3c4a5353f3)) in a way that is most optimized for their library usage. + +## What types of subscriptions can this support? + +This abstraction can handle a variety of subscription types, including: +* Event dispatchers like `HTMLInputElement`. +* Custom pub/sub components like Relay's `FragmentSpecResolver`. +* Observable types like RxJS `BehaviorSubject` and `ReplaySubject`. (Types like RxJS `Subject` or `Observable` are not supported, because they provide no way to read the "current" value after it has been emitted.) +* Native Promises. + +# Installation + +```sh +# Yarn +yarn add create-subscription + +# NPM +npm install create-subscription --save +``` + +# Usage + +To configure a subscription, you must provide two methods: `getCurrentValue` and `subscribe`. + +```js +import { createSubscription } from "create-subscription"; + +const Subscription = createSubscription({ + getCurrentValue(source) { + // Return the current value of the subscription (source), + // or `undefined` if the value can't be read synchronously (e.g. native Promises). + }, + subscribe(source, callback) { + // Subscribe (e.g. add an event listener) to the subscription (source). + // Call callback(newValue) whenever a subscription changes. + // Return an unsubscribe method, + // Or a no-op if unsubscribe is not supported (e.g. native Promises). + } +}); +``` + +To use the `Subscription` component, pass the subscribable property (e.g. an event dispatcher, Flux store, observable) as the `source` property and use a [render prop](https://reactjs.org/docs/render-props.html), `children`, to handle the subscribed value when it changes: + +```js + + {value => } + +``` + +# Examples + +This API can be used to subscribe to a variety of "subscribable" sources, from event dispatchers to RxJS observables. Below are a few examples of how to subscribe to common types. + +## Subscribing to event dispatchers + +Below is an example showing how `create-subscription` can be used to subscribe to event dispatchers such as DOM elements. + +```js +import React from "react"; +import { createSubscription } from "create-subscription"; + +// Start with a simple component. +// In this case, it's a functional component, but it could have been a class. +function FollowerComponent({ followersCount }) { + return
You have {followersCount} followers!
; +} + +// Create a wrapper component to manage the subscription. +const EventHandlerSubscription = createSubscription({ + getCurrentValue: eventDispatcher => eventDispatcher.value, + subscribe: (eventDispatcher, callback) => { + const onChange = event => callback(eventDispatcher.value); + eventDispatcher.addEventListener("change", onChange); + return () => eventDispatcher.removeEventListener("change", onChange); + } +}); + +// Your component can now be used as shown below. +// In this example, 'eventDispatcher' represents a generic event dispatcher. + + {value => } +; +``` + +## Subscribing to observables + +Below are examples showing how `create-subscription` can be used to subscribe to certain types of observables (e.g. RxJS `BehaviorSubject` and `ReplaySubject`). + +**Note** that it is not possible to support all observable types (e.g. RxJS `Subject` or `Observable`) because some provide no way to read the "current" value after it has been emitted. + +### `BehaviorSubject` +```js +const BehaviorSubscription = createSubscription({ + getCurrentValue: behaviorSubject => behaviorSubject.getValue(), + subscribe: (behaviorSubject, callback) => { + const subscription = behaviorSubject.subscribe(callback); + return () => subscription.unsubscribe(); + } +}); +``` + +### `ReplaySubject` +```js +const ReplaySubscription = createSubscription({ + getCurrentValue: replaySubject => { + let currentValue; + // ReplaySubject does not have a sync data getter, + // So we need to temporarily subscribe to retrieve the most recent value. + replaySubject + .subscribe(value => { + currentValue = value; + }) + .unsubscribe(); + return currentValue; + }, + subscribe: (replaySubject, callback) => { + const subscription = replaySubject.subscribe(callback); + return () => subscription.unsubscribe(); + } +}); +``` + +## Subscribing to a Promise + +Below is an example showing how `create-subscription` can be used with native Promises. + +**Note** that it an initial render value of `undefined` is unavoidable due to the fact that Promises provide no way to synchronously read their current value. + +**Note** the lack of a way to "unsubscribe" from a Promise can result in memory leaks as long as something has a reference to the Promise. This should be taken into considerationg when determining whether Promises are appropriate to use in this way within your application. + +```js +import React from "react"; +import { createSubscription } from "create-subscription"; + +// Start with a simple component. +function LoadingComponent({ loadingStatus }) { + if (loadingStatus === undefined) { + // Loading + } else if (loadingStatus === null) { + // Error + } else { + // Success + } +} + +// Wrap the functional component with a subscriber HOC. +// This HOC will manage subscriptions and pass values to the decorated component. +// It will add and remove subscriptions in an async-safe way when props change. +const PromiseSubscription = createSubscription({ + getCurrentValue: promise => { + // There is no way to synchronously read a Promise's value, + // So this method should return undefined. + return undefined; + }, + subscribe: (promise, callback) => { + promise.then( + // Success + value => callback(value), + // Failure + () => callback(null) + ); + + // There is no way to "unsubscribe" from a Promise. + // create-subscription will still prevent stale values from rendering. + return () => {}; + } +}); + +// Your component can now be used as shown below. + + {loadingStatus => } + +``` diff --git a/packages/create-subscription/index.js b/packages/create-subscription/index.js new file mode 100644 index 0000000000000..8e84321cc3186 --- /dev/null +++ b/packages/create-subscription/index.js @@ -0,0 +1,12 @@ +/** + * 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. + * + * @flow + */ + +'use strict'; + +export * from './src/createSubscription'; diff --git a/packages/create-subscription/npm/index.js b/packages/create-subscription/npm/index.js new file mode 100644 index 0000000000000..6b7a5b017457d --- /dev/null +++ b/packages/create-subscription/npm/index.js @@ -0,0 +1,7 @@ +'use strict'; + +if (process.env.NODE_ENV === 'production') { + module.exports = require('./cjs/create-subscription.production.min.js'); +} else { + module.exports = require('./cjs/create-subscription.development.js'); +} diff --git a/packages/create-subscription/package.json b/packages/create-subscription/package.json new file mode 100644 index 0000000000000..bd8ae749f46bc --- /dev/null +++ b/packages/create-subscription/package.json @@ -0,0 +1,21 @@ +{ + "name": "create-subscription", + "description": "HOC for creating async-safe React components with subscriptions", + "version": "0.0.1", + "repository": "facebook/react", + "files": [ + "LICENSE", + "README.md", + "index.js", + "cjs/" + ], + "dependencies": { + "fbjs": "^0.8.16" + }, + "peerDependencies": { + "react": "16.3.0-alpha.1" + }, + "devDependencies": { + "rxjs": "^5.5.6" + } +} diff --git a/packages/create-subscription/src/__tests__/createSubscription-test.internal.js b/packages/create-subscription/src/__tests__/createSubscription-test.internal.js new file mode 100644 index 0000000000000..8dee9bfa5c9de --- /dev/null +++ b/packages/create-subscription/src/__tests__/createSubscription-test.internal.js @@ -0,0 +1,457 @@ +/** + * 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'; + +let createSubscription; +let BehaviorSubject; +let ReactFeatureFlags; +let React; +let ReactNoop; +let ReplaySubject; + +describe('createSubscription', () => { + beforeEach(() => { + jest.resetModules(); + createSubscription = require('create-subscription').createSubscription; + ReactFeatureFlags = require('shared/ReactFeatureFlags'); + ReactFeatureFlags.debugRenderPhaseSideEffectsForStrictMode = false; + React = require('react'); + ReactNoop = require('react-noop-renderer'); + + BehaviorSubject = require('rxjs/BehaviorSubject').BehaviorSubject; + ReplaySubject = require('rxjs/ReplaySubject').ReplaySubject; + }); + + function createBehaviorSubject(initialValue) { + const behaviorSubject = new BehaviorSubject(); + if (initialValue) { + behaviorSubject.next(initialValue); + } + return behaviorSubject; + } + + function createReplaySubject(initialValue) { + const replaySubject = new ReplaySubject(); + if (initialValue) { + replaySubject.next(initialValue); + } + return replaySubject; + } + + it('supports basic subscription pattern', () => { + const Subscription = createSubscription({ + getCurrentValue: source => source.getValue(), + subscribe: (source, callback) => { + const subscription = source.subscribe(callback); + return () => subscription.unsubscribe; + }, + }); + + const observable = createBehaviorSubject(); + ReactNoop.render( + + {(value = 'default') => { + ReactNoop.yield(value); + return null; + }} + , + ); + + // Updates while subscribed should re-render the child component + expect(ReactNoop.flush()).toEqual(['default']); + observable.next(123); + expect(ReactNoop.flush()).toEqual([123]); + observable.next('abc'); + expect(ReactNoop.flush()).toEqual(['abc']); + + // Unmounting the subscriber should remove listeners + ReactNoop.render(
); + observable.next(456); + expect(ReactNoop.flush()).toEqual([]); + }); + + it('should support observable types like RxJS ReplaySubject', () => { + const Subscription = createSubscription({ + getCurrentValue: source => { + let currentValue; + source + .subscribe(value => { + currentValue = value; + }) + .unsubscribe(); + return currentValue; + }, + subscribe: (source, callback) => { + const subscription = source.subscribe(callback); + return () => subscription.unsubscribe; + }, + }); + + function render(value = 'default') { + ReactNoop.yield(value); + return null; + } + + const observable = createReplaySubject('initial'); + + ReactNoop.render({render}); + expect(ReactNoop.flush()).toEqual(['initial']); + observable.next('updated'); + expect(ReactNoop.flush()).toEqual(['updated']); + + // Unsetting the subscriber prop should reset subscribed values + ReactNoop.render({render}); + expect(ReactNoop.flush()).toEqual(['default']); + }); + + describe('Promises', () => { + it('should support Promises', async () => { + const Subscription = createSubscription({ + getCurrentValue: source => undefined, + subscribe: (source, callback) => { + source.then(value => callback(value), value => callback(value)); + // (Can't unsubscribe from a Promise) + return () => {}; + }, + }); + + function render(hasLoaded) { + if (hasLoaded === undefined) { + ReactNoop.yield('loading'); + } else { + ReactNoop.yield(hasLoaded ? 'finished' : 'failed'); + } + return null; + } + + let resolveA, rejectB; + const promiseA = new Promise((resolve, reject) => { + resolveA = resolve; + }); + const promiseB = new Promise((resolve, reject) => { + rejectB = reject; + }); + + // Test a promise that resolves after render + ReactNoop.render({render}); + expect(ReactNoop.flush()).toEqual(['loading']); + resolveA(true); + await promiseA; + expect(ReactNoop.flush()).toEqual(['finished']); + + // Test a promise that resolves before render + // Note that this will require an extra render anyway, + // Because there is no way to syncrhonously get a Promise's value + rejectB(false); + ReactNoop.render({render}); + expect(ReactNoop.flush()).toEqual(['loading']); + await promiseB.catch(() => true); + expect(ReactNoop.flush()).toEqual(['failed']); + }); + + it('should still work if unsubscription is managed incorrectly', async () => { + const Subscription = createSubscription({ + getCurrentValue: source => undefined, + subscribe: (source, callback) => { + source.then(callback); + // (Can't unsubscribe from a Promise) + return () => {}; + }, + }); + + function render(value = 'default') { + ReactNoop.yield(value); + return null; + } + + let resolveA, resolveB; + const promiseA = new Promise(resolve => (resolveA = resolve)); + const promiseB = new Promise(resolve => (resolveB = resolve)); + + // Subscribe first to Promise A then Promise B + ReactNoop.render({render}); + expect(ReactNoop.flush()).toEqual(['default']); + ReactNoop.render({render}); + expect(ReactNoop.flush()).toEqual(['default']); + + // Resolve both Promises + resolveB(123); + resolveA('abc'); + await Promise.all([promiseA, promiseB]); + + // Ensure that only Promise B causes an update + expect(ReactNoop.flush()).toEqual([123]); + }); + }); + + it('should unsubscribe from old subscribables and subscribe to new subscribables when props change', () => { + const Subscription = createSubscription({ + getCurrentValue: source => source.getValue(), + subscribe: (source, callback) => { + const subscription = source.subscribe(callback); + return () => subscription.unsubscribe(); + }, + }); + + function render(value = 'default') { + ReactNoop.yield(value); + return null; + } + + const observableA = createBehaviorSubject('a-0'); + const observableB = createBehaviorSubject('b-0'); + + ReactNoop.render( + {render}, + ); + + // Updates while subscribed should re-render the child component + expect(ReactNoop.flush()).toEqual(['a-0']); + + // Unsetting the subscriber prop should reset subscribed values + ReactNoop.render( + {render}, + ); + expect(ReactNoop.flush()).toEqual(['b-0']); + + // Updates to the old subscribable should not re-render the child component + observableA.next('a-1'); + expect(ReactNoop.flush()).toEqual([]); + + // Updates to the bew subscribable should re-render the child component + observableB.next('b-1'); + expect(ReactNoop.flush()).toEqual(['b-1']); + }); + + it('should ignore values emitted by a new subscribable until the commit phase', () => { + const log = []; + let parentInstance; + + function Child({value}) { + ReactNoop.yield('Child: ' + value); + return null; + } + + const Subscription = createSubscription({ + getCurrentValue: source => source.getValue(), + subscribe: (source, callback) => { + const subscription = source.subscribe(callback); + return () => subscription.unsubscribe(); + }, + }); + + class Parent extends React.Component { + state = {}; + + static getDerivedStateFromProps(nextProps, prevState) { + if (nextProps.observed !== prevState.observed) { + return { + observed: nextProps.observed, + }; + } + + return null; + } + + componentDidMount() { + log.push('Parent.componentDidMount'); + } + + componentDidUpdate() { + log.push('Parent.componentDidUpdate'); + } + + render() { + parentInstance = this; + + return ( + + {(value = 'default') => { + ReactNoop.yield('Subscriber: ' + value); + return ; + }} + + ); + } + } + + const observableA = createBehaviorSubject('a-0'); + const observableB = createBehaviorSubject('b-0'); + + ReactNoop.render(); + expect(ReactNoop.flush()).toEqual(['Subscriber: a-0', 'Child: a-0']); + expect(log).toEqual(['Parent.componentDidMount']); + + // Start React update, but don't finish + ReactNoop.render(); + ReactNoop.flushThrough(['Subscriber: b-0']); + expect(log).toEqual(['Parent.componentDidMount']); + + // Emit some updates from the uncommitted subscribable + observableB.next('b-1'); + observableB.next('b-2'); + observableB.next('b-3'); + + // Mimic a higher-priority interruption + parentInstance.setState({observed: observableA}); + + // Flush everything and ensure that the correct subscribable is used + // We expect the last emitted update to be rendered (because of the commit phase value check) + // But the intermediate ones should be ignored, + // And the final rendered output should be the higher-priority observable. + expect(ReactNoop.flush()).toEqual([ + 'Child: b-0', + 'Subscriber: b-3', + 'Child: b-3', + 'Subscriber: a-0', + 'Child: a-0', + ]); + expect(log).toEqual([ + 'Parent.componentDidMount', + 'Parent.componentDidUpdate', + 'Parent.componentDidUpdate', + ]); + }); + + it('should not drop values emitted between updates', () => { + const log = []; + let parentInstance; + + function Child({value}) { + ReactNoop.yield('Child: ' + value); + return null; + } + + const Subscription = createSubscription({ + getCurrentValue: source => source.getValue(), + subscribe: (source, callback) => { + const subscription = source.subscribe(callback); + return () => subscription.unsubscribe(); + }, + }); + + class Parent extends React.Component { + state = {}; + + static getDerivedStateFromProps(nextProps, prevState) { + if (nextProps.observed !== prevState.observed) { + return { + observed: nextProps.observed, + }; + } + + return null; + } + + componentDidMount() { + log.push('Parent.componentDidMount'); + } + + componentDidUpdate() { + log.push('Parent.componentDidUpdate'); + } + + render() { + parentInstance = this; + + return ( + + {(value = 'default') => { + ReactNoop.yield('Subscriber: ' + value); + return ; + }} + + ); + } + } + + const observableA = createBehaviorSubject('a-0'); + const observableB = createBehaviorSubject('b-0'); + + ReactNoop.render(); + expect(ReactNoop.flush()).toEqual(['Subscriber: a-0', 'Child: a-0']); + expect(log).toEqual(['Parent.componentDidMount']); + + // Start React update, but don't finish + ReactNoop.render(); + ReactNoop.flushThrough(['Subscriber: b-0']); + expect(log).toEqual(['Parent.componentDidMount']); + + // Emit some updates from the old subscribable + observableA.next('a-1'); + observableA.next('a-2'); + + // Mimic a higher-priority interruption + parentInstance.setState({observed: observableA}); + + // Flush everything and ensure that the correct subscribable is used + // We expect the new subscribable to finish rendering, + // But then the updated values from the old subscribable should be used. + expect(ReactNoop.flush()).toEqual([ + 'Child: b-0', + 'Subscriber: a-2', + 'Child: a-2', + ]); + expect(log).toEqual([ + 'Parent.componentDidMount', + 'Parent.componentDidUpdate', + 'Parent.componentDidUpdate', + ]); + + // Updates from the new subsribable should be ignored. + observableB.next('b-1'); + expect(ReactNoop.flush()).toEqual([]); + expect(log).toEqual([ + 'Parent.componentDidMount', + 'Parent.componentDidUpdate', + 'Parent.componentDidUpdate', + ]); + }); + + describe('warnings', () => { + it('should warn for invalid missing getCurrentValue', () => { + expect(() => { + createSubscription( + { + subscribe: () => () => {}, + }, + () => null, + ); + }).toWarnDev('Subscription must specify a getCurrentValue function'); + }); + + it('should warn for invalid missing subscribe', () => { + expect(() => { + createSubscription( + { + getCurrentValue: () => () => {}, + }, + () => null, + ); + }).toWarnDev('Subscription must specify a subscribe function'); + }); + + it('should warn if subscribe does not return an unsubscribe method', () => { + const Subscription = createSubscription({ + getCurrentValue: source => undefined, + subscribe: (source, callback) => {}, + }); + + const observable = createBehaviorSubject(); + ReactNoop.render( + {value => null}, + ); + + expect(ReactNoop.flush).toThrow( + 'A subscription must return an unsubscribe function.', + ); + }); + }); +}); diff --git a/packages/create-subscription/src/createSubscription.js b/packages/create-subscription/src/createSubscription.js new file mode 100644 index 0000000000000..748090d6cc961 --- /dev/null +++ b/packages/create-subscription/src/createSubscription.js @@ -0,0 +1,159 @@ +/** + * 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. + * + * @flow + */ + +import React from 'react'; +import invariant from 'fbjs/lib/invariant'; +import warning from 'fbjs/lib/warning'; + +type Unsubscribe = () => void; + +export function createSubscription( + config: $ReadOnly<{| + // Synchronously gets the value for the subscribed property. + // Return undefined if the subscribable value is undefined, + // Or does not support synchronous reading (e.g. native Promise). + getCurrentValue: (source: Property) => Value | void, + + // Setup a subscription for the subscribable value in props, and return an unsubscribe function. + // Return false to indicate the property cannot be unsubscribed from (e.g. native Promises). + // Due to the variety of change event types, subscribers should provide their own handlers. + // Those handlers should not attempt to update state though; + // They should call the callback() instead when a subscription changes. + subscribe: ( + source: Property, + callback: (value: Value | void) => void, + ) => Unsubscribe, + |}>, +): React$ComponentType<{ + children: (value: Value | void) => React$Node, + source: Property, +}> { + const {getCurrentValue, subscribe} = config; + + warning( + typeof getCurrentValue === 'function', + 'Subscription must specify a getCurrentValue function', + ); + warning( + typeof subscribe === 'function', + 'Subscription must specify a subscribe function', + ); + + type Props = { + children: (value: Value) => React$Element, + source: Property, + }; + type State = { + source: Property, + unsubscribeContainer: { + unsubscribe: Unsubscribe | null, + }, + value: Value | void, + }; + + // Reference: https://gist.github.com/bvaughn/d569177d70b50b58bff69c3c4a5353f3 + class Subscription extends React.Component { + state: State = { + source: this.props.source, + unsubscribeContainer: { + unsubscribe: null, + }, + value: + this.props.source != null + ? getCurrentValue(this.props.source) + : undefined, + }; + + static getDerivedStateFromProps(nextProps, prevState) { + if (nextProps.source !== prevState.source) { + return { + source: nextProps.source, + unsubscribeContainer: { + unsubscribe: null, + }, + value: + nextProps.source != null + ? getCurrentValue(nextProps.source) + : undefined, + }; + } + + return null; + } + + componentDidMount() { + this.subscribe(); + } + + componentDidUpdate(prevProps, prevState) { + if (this.state.source !== prevState.source) { + this.unsubscribe(prevState); + this.subscribe(); + } + } + + componentWillUnmount() { + this.unsubscribe(this.state); + } + + render() { + return this.props.children(this.state.value); + } + + subscribe() { + const {source} = this.state; + if (source != null) { + const callback = (value: Value | void) => { + this.setState(state => { + // If the value is the same, skip the unnecessary state update. + if (value === state.value) { + return null; + } + + // If this event belongs to an old or uncommitted data source, ignore it. + if (source !== state.source) { + return null; + } + + return {value}; + }); + }; + + // Store subscription for later (in case it's needed to unsubscribe). + // This is safe to do via mutation since: + // 1) It does not impact render. + // 2) This method will only be called during the "commit" phase. + const unsubscribe = subscribe(source, callback); + + invariant( + typeof unsubscribe === 'function', + 'A subscription must return an unsubscribe function.', + ); + + this.state.unsubscribeContainer.unsubscribe = unsubscribe; + + // External values could change between render and mount, + // In some cases it may be important to handle this case. + const value = getCurrentValue(this.props.source); + if (value !== this.state.value) { + this.setState({value}); + } + } + } + + unsubscribe(state: State) { + const {unsubscribe} = state.unsubscribeContainer; + if (typeof unsubscribe === 'function') { + unsubscribe(); + } + } + } + + return Subscription; +} diff --git a/packages/react-dom/src/__tests__/ReactComponent-test.js b/packages/react-dom/src/__tests__/ReactComponent-test.js index d08ba34a27db8..9c2d6447ff56b 100644 --- a/packages/react-dom/src/__tests__/ReactComponent-test.js +++ b/packages/react-dom/src/__tests__/ReactComponent-test.js @@ -235,8 +235,8 @@ describe('ReactComponent', () => { } componentDidMount() { - expect(this.innerRef.value.getObject()).toEqual(innerObj); - expect(this.outerRef.value.getObject()).toEqual(outerObj); + expect(this.innerRef.current.getObject()).toEqual(innerObj); + expect(this.outerRef.current.getObject()).toEqual(outerObj); mounted = true; } } diff --git a/packages/react-dom/src/__tests__/ReactErrorBoundaries-test.internal.js b/packages/react-dom/src/__tests__/ReactErrorBoundaries-test.internal.js index defcd6228bdc4..bec420aeabc97 100644 --- a/packages/react-dom/src/__tests__/ReactErrorBoundaries-test.internal.js +++ b/packages/react-dom/src/__tests__/ReactErrorBoundaries-test.internal.js @@ -1019,12 +1019,14 @@ describe('ReactErrorBoundaries', () => { 'ErrorBoundary render error', 'ErrorBoundary componentDidUpdate', ]); - expect(errorMessageRef.value.toString()).toEqual('[object HTMLDivElement]'); + expect(errorMessageRef.current.toString()).toEqual( + '[object HTMLDivElement]', + ); log.length = 0; ReactDOM.unmountComponentAtNode(container); expect(log).toEqual(['ErrorBoundary componentWillUnmount']); - expect(errorMessageRef.value).toEqual(null); + expect(errorMessageRef.current).toEqual(null); }); it('successfully mounts if no error occurs', () => { diff --git a/packages/react-reconciler/src/ReactFiberClassComponent.js b/packages/react-reconciler/src/ReactFiberClassComponent.js index 6d682cd2329a6..bf8abba92d2f8 100644 --- a/packages/react-reconciler/src/ReactFiberClassComponent.js +++ b/packages/react-reconciler/src/ReactFiberClassComponent.js @@ -789,20 +789,20 @@ export default function( // In order to support react-lifecycles-compat polyfilled components, // Unsafe lifecycles should not be invoked for any component with the new gDSFP. if ( - (typeof instance.UNSAFE_componentWillUpdate === 'function' || - typeof instance.componentWillUpdate === 'function') && + (typeof instance.UNSAFE_componentWillMount === 'function' || + typeof instance.componentWillMount === 'function') && typeof ctor.getDerivedStateFromProps !== 'function' ) { - startPhaseTimer(workInProgress, 'componentWillUpdate'); - if (typeof instance.componentWillUpdate === 'function') { - instance.componentWillUpdate(newProps, newState, newContext); + startPhaseTimer(workInProgress, 'componentWillMount'); + if (typeof instance.componentWillMount === 'function') { + instance.componentWillMount(); } - if (typeof instance.UNSAFE_componentWillUpdate === 'function') { - instance.UNSAFE_componentWillUpdate(newProps, newState, newContext); + if (typeof instance.UNSAFE_componentWillMount === 'function') { + instance.UNSAFE_componentWillMount(); } stopPhaseTimer(); } - if (typeof instance.componentDidUpdate === 'function') { + if (typeof instance.componentDidMount === 'function') { workInProgress.effectTag |= Update; } } else { diff --git a/packages/react-reconciler/src/ReactFiberCommitWork.js b/packages/react-reconciler/src/ReactFiberCommitWork.js index 831a391d42ded..cd688556434cc 100644 --- a/packages/react-reconciler/src/ReactFiberCommitWork.js +++ b/packages/react-reconciler/src/ReactFiberCommitWork.js @@ -29,6 +29,7 @@ import { import ReactErrorUtils from 'shared/ReactErrorUtils'; import {Placement, Update, ContentReset} from 'shared/ReactTypeOfSideEffect'; import invariant from 'fbjs/lib/invariant'; +import warning from 'fbjs/lib/warning'; import {commitCallbacks} from './ReactFiberUpdateQueue'; import {onCommitUnmount} from './ReactFiberDevToolsHook'; @@ -147,7 +148,7 @@ export default function( } } } else { - ref.value = null; + ref.current = null; } } } @@ -315,7 +316,19 @@ export default function( if (typeof ref === 'function') { ref(instanceToUse); } else { - ref.value = instanceToUse; + if (__DEV__) { + if (!ref.hasOwnProperty('current')) { + warning( + false, + 'Unexpected ref object provided for %s. ' + + 'Use either a ref-setter function or Reacte.createRef().%s', + getComponentName(finishedWork), + getStackAddendumByWorkInProgressFiber(finishedWork), + ); + } + } + + ref.current = instanceToUse; } } } @@ -326,7 +339,7 @@ export default function( if (typeof currentRef === 'function') { currentRef(null); } else { - currentRef.value = null; + currentRef.current = null; } } } diff --git a/packages/react-reconciler/src/__tests__/ReactIncrementalErrorHandling-test.internal.js b/packages/react-reconciler/src/__tests__/ReactIncrementalErrorHandling-test.internal.js index ef44ce3bb11b0..cc45510c43883 100644 --- a/packages/react-reconciler/src/__tests__/ReactIncrementalErrorHandling-test.internal.js +++ b/packages/react-reconciler/src/__tests__/ReactIncrementalErrorHandling-test.internal.js @@ -1259,4 +1259,47 @@ describe('ReactIncrementalErrorHandling', () => { ]); expect(ReactNoop.getChildren()).toEqual([span('Caught an error: oops!')]); }); + + it('calls the correct lifecycles on the error boundary after catching an error (mixed)', () => { + // This test seems a bit contrived, but it's based on an actual regression + // where we checked for the existence of didUpdate instead of didMount, and + // didMount was not defined. + function BadRender() { + ReactNoop.yield('throw'); + throw new Error('oops!'); + } + + let parent; + class Parent extends React.Component { + state = {error: null, other: false}; + componentDidCatch(error) { + ReactNoop.yield('did catch'); + this.setState({error}); + } + componentDidUpdate() { + ReactNoop.yield('did update'); + } + render() { + parent = this; + if (this.state.error) { + ReactNoop.yield('render error message'); + return ; + } + ReactNoop.yield('render'); + return ; + } + } + + ReactNoop.render(); + ReactNoop.flushThrough(['render', 'throw']); + expect(ReactNoop.getChildren()).toEqual([]); + + parent.setState({other: true}); + expect(ReactNoop.flush()).toEqual([ + 'did catch', + 'render error message', + 'did update', + ]); + expect(ReactNoop.getChildren()).toEqual([span('Caught an error: oops!')]); + }); }); diff --git a/packages/react/src/ReactCreateRef.js b/packages/react/src/ReactCreateRef.js index 8af60100e64d7..326caddae9324 100644 --- a/packages/react/src/ReactCreateRef.js +++ b/packages/react/src/ReactCreateRef.js @@ -11,7 +11,7 @@ import type {RefObject} from 'shared/ReactTypes'; // an immutable object with a single mutable value export function createRef(): RefObject { const refObject = { - value: null, + current: null, }; if (__DEV__) { Object.seal(refObject); diff --git a/packages/react/src/__tests__/ReactCreateRef-test.js b/packages/react/src/__tests__/ReactCreateRef-test.js new file mode 100644 index 0000000000000..683d3ddf01027 --- /dev/null +++ b/packages/react/src/__tests__/ReactCreateRef-test.js @@ -0,0 +1,60 @@ +/** + * 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'; + +let React; +let ReactTestRenderer; + +describe('ReactCreateRef', () => { + beforeEach(() => { + jest.resetModules(); + + React = require('react'); + ReactTestRenderer = require('react-test-renderer'); + }); + + it('should warn in dev if an invalid ref object is provided', () => { + function Wrapper({children}) { + return children; + } + + class ExampleComponent extends React.Component { + render() { + return null; + } + } + + expect(() => + ReactTestRenderer.create( + +
+ , + ), + ).toWarnDev( + 'Unexpected ref object provided for div. ' + + 'Use either a ref-setter function or Reacte.createRef().\n' + + ' in div (at **)\n' + + ' in Wrapper (at **)', + ); + + expect(() => + ReactTestRenderer.create( + + + , + ), + ).toWarnDev( + 'Unexpected ref object provided for ExampleComponent. ' + + 'Use either a ref-setter function or Reacte.createRef().\n' + + ' in ExampleComponent (at **)\n' + + ' in Wrapper (at **)', + ); + }); +}); diff --git a/packages/shared/ReactTypes.js b/packages/shared/ReactTypes.js index bede9f00e3006..f6a56ccc96ed4 100644 --- a/packages/shared/ReactTypes.js +++ b/packages/shared/ReactTypes.js @@ -101,5 +101,5 @@ export type ReactPortal = { }; export type RefObject = {| - value: any, + current: any, |}; diff --git a/scripts/rollup/bundles.js b/scripts/rollup/bundles.js index 0be2375a58b9a..b9c5f57558a56 100644 --- a/scripts/rollup/bundles.js +++ b/scripts/rollup/bundles.js @@ -254,6 +254,16 @@ const bundles = [ global: 'SimpleCacheProvider', externals: ['react'], }, + + /******* createComponentWithSubscriptions (experimental) *******/ + { + label: 'create-subscription', + bundleTypes: [NODE_DEV, NODE_PROD], + moduleType: ISOMORPHIC, + entry: 'create-subscription', + global: 'createSubscription', + externals: ['react'], + }, ]; // Based on deep-freeze by substack (public domain) diff --git a/scripts/rollup/results.json b/scripts/rollup/results.json index 77a21c1b12d68..cd88e85620eae 100644 --- a/scripts/rollup/results.json +++ b/scripts/rollup/results.json @@ -4,8 +4,8 @@ "filename": "react.development.js", "bundleType": "UMD_DEV", "packageName": "react", - "size": 55674, - "gzip": 15255 + "size": 55675, + "gzip": 15253 }, { "filename": "react.production.min.js", @@ -18,8 +18,8 @@ "filename": "react.development.js", "bundleType": "NODE_DEV", "packageName": "react", - "size": 46095, - "gzip": 12925 + "size": 46096, + "gzip": 12924 }, { "filename": "react.production.min.js", @@ -32,8 +32,8 @@ "filename": "React-dev.js", "bundleType": "FB_DEV", "packageName": "react", - "size": 45476, - "gzip": 12448 + "size": 45477, + "gzip": 12446 }, { "filename": "React-prod.js", @@ -46,50 +46,50 @@ "filename": "react-dom.development.js", "bundleType": "UMD_DEV", "packageName": "react-dom", - "size": 591513, - "gzip": 138743 + "size": 600642, + "gzip": 139543 }, { "filename": "react-dom.production.min.js", "bundleType": "UMD_PROD", "packageName": "react-dom", - "size": 96778, - "gzip": 31445 + "size": 100738, + "gzip": 32495 }, { "filename": "react-dom.development.js", "bundleType": "NODE_DEV", "packageName": "react-dom", - "size": 575526, - "gzip": 134516 + "size": 584651, + "gzip": 135289 }, { "filename": "react-dom.production.min.js", "bundleType": "NODE_PROD", "packageName": "react-dom", - "size": 95503, - "gzip": 30619 + "size": 99167, + "gzip": 31568 }, { "filename": "ReactDOM-dev.js", "bundleType": "FB_DEV", "packageName": "react-dom", - "size": 594783, - "gzip": 136782 + "size": 604987, + "gzip": 137591 }, { "filename": "ReactDOM-prod.js", "bundleType": "FB_PROD", "packageName": "react-dom", - "size": 279046, - "gzip": 53062 + "size": 290412, + "gzip": 54502 }, { "filename": "react-dom-test-utils.development.js", "bundleType": "UMD_DEV", "packageName": "react-dom", - "size": 41697, - "gzip": 11964 + "size": 41803, + "gzip": 12011 }, { "filename": "react-dom-test-utils.production.min.js", @@ -102,8 +102,8 @@ "filename": "react-dom-test-utils.development.js", "bundleType": "NODE_DEV", "packageName": "react-dom", - "size": 36434, - "gzip": 10505 + "size": 36540, + "gzip": 10554 }, { "filename": "react-dom-test-utils.production.min.js", @@ -116,8 +116,8 @@ "filename": "ReactTestUtils-dev.js", "bundleType": "FB_DEV", "packageName": "react-dom", - "size": 37155, - "gzip": 10582 + "size": 37255, + "gzip": 10630 }, { "filename": "react-dom-unstable-native-dependencies.development.js", @@ -165,141 +165,141 @@ "filename": "react-dom-server.browser.development.js", "bundleType": "UMD_DEV", "packageName": "react-dom", - "size": 102991, - "gzip": 26927 + "size": 103067, + "gzip": 27041 }, { "filename": "react-dom-server.browser.production.min.js", "bundleType": "UMD_PROD", "packageName": "react-dom", - "size": 15184, - "gzip": 5856 + "size": 15133, + "gzip": 5835 }, { "filename": "react-dom-server.browser.development.js", "bundleType": "NODE_DEV", "packageName": "react-dom", - "size": 92035, - "gzip": 24618 + "size": 92111, + "gzip": 24739 }, { "filename": "react-dom-server.browser.production.min.js", "bundleType": "NODE_PROD", "packageName": "react-dom", - "size": 14818, - "gzip": 5705 + "size": 14771, + "gzip": 5680 }, { "filename": "ReactDOMServer-dev.js", "bundleType": "FB_DEV", "packageName": "react-dom", - "size": 95165, - "gzip": 24327 + "size": 95191, + "gzip": 24410 }, { "filename": "ReactDOMServer-prod.js", "bundleType": "FB_PROD", "packageName": "react-dom", - "size": 33262, - "gzip": 8299 + "size": 33064, + "gzip": 8279 }, { "filename": "react-dom-server.node.development.js", "bundleType": "NODE_DEV", "packageName": "react-dom", - "size": 94003, - "gzip": 25175 + "size": 94079, + "gzip": 25295 }, { "filename": "react-dom-server.node.production.min.js", "bundleType": "NODE_PROD", "packageName": "react-dom", - "size": 15642, - "gzip": 6010 + "size": 15595, + "gzip": 5990 }, { "filename": "react-art.development.js", "bundleType": "UMD_DEV", "packageName": "react-art", - "size": 389869, - "gzip": 86413 + "size": 399001, + "gzip": 87190 }, { "filename": "react-art.production.min.js", "bundleType": "UMD_PROD", "packageName": "react-art", - "size": 86808, - "gzip": 26944 + "size": 90690, + "gzip": 27874 }, { "filename": "react-art.development.js", "bundleType": "NODE_DEV", "packageName": "react-art", - "size": 313942, - "gzip": 67385 + "size": 323070, + "gzip": 68147 }, { "filename": "react-art.production.min.js", "bundleType": "NODE_PROD", "packageName": "react-art", - "size": 50754, - "gzip": 16005 + "size": 54355, + "gzip": 16860 }, { "filename": "ReactART-dev.js", "bundleType": "FB_DEV", "packageName": "react-art", - "size": 318024, - "gzip": 66603 + "size": 328230, + "gzip": 67375 }, { "filename": "ReactART-prod.js", "bundleType": "FB_PROD", "packageName": "react-art", - "size": 157473, - "gzip": 27225 + "size": 168749, + "gzip": 28640 }, { "filename": "ReactNativeRenderer-dev.js", "bundleType": "RN_DEV", "packageName": "react-native-renderer", - "size": 443941, - "gzip": 97414 + "size": 454044, + "gzip": 98203 }, { "filename": "ReactNativeRenderer-prod.js", "bundleType": "RN_PROD", "packageName": "react-native-renderer", - "size": 209855, - "gzip": 36492 + "size": 220436, + "gzip": 37780 }, { "filename": "react-test-renderer.development.js", "bundleType": "NODE_DEV", "packageName": "react-test-renderer", - "size": 310910, - "gzip": 66329 + "size": 320190, + "gzip": 67115 }, { "filename": "react-test-renderer.production.min.js", "bundleType": "NODE_PROD", "packageName": "react-test-renderer", - "size": 49219, - "gzip": 15315 + "size": 52870, + "gzip": 16241 }, { "filename": "ReactTestRenderer-dev.js", "bundleType": "FB_DEV", "packageName": "react-test-renderer", - "size": 315000, - "gzip": 65520 + "size": 325364, + "gzip": 66316 }, { "filename": "react-test-renderer-shallow.development.js", "bundleType": "NODE_DEV", "packageName": "react-test-renderer", - "size": 21221, - "gzip": 5193 + "size": 21475, + "gzip": 5309 }, { "filename": "react-test-renderer-shallow.production.min.js", @@ -312,43 +312,43 @@ "filename": "ReactShallowRenderer-dev.js", "bundleType": "FB_DEV", "packageName": "react-test-renderer", - "size": 20928, - "gzip": 4566 + "size": 21120, + "gzip": 4625 }, { "filename": "react-noop-renderer.development.js", "bundleType": "NODE_DEV", "packageName": "react-noop-renderer", - "size": 18777, - "gzip": 5303 + "size": 19408, + "gzip": 5482 }, { "filename": "react-noop-renderer.production.min.js", "bundleType": "NODE_PROD", "packageName": "react-noop-renderer", - "size": 6429, - "gzip": 2573 + "size": 6643, + "gzip": 2618 }, { "filename": "react-reconciler.development.js", "bundleType": "NODE_DEV", "packageName": "react-reconciler", - "size": 292377, - "gzip": 61765 + "size": 301505, + "gzip": 62567 }, { "filename": "react-reconciler.production.min.js", "bundleType": "NODE_PROD", "packageName": "react-reconciler", - "size": 42443, - "gzip": 13358 + "size": 46055, + "gzip": 14278 }, { "filename": "react-reconciler-reflection.development.js", "bundleType": "NODE_DEV", "packageName": "react-reconciler", - "size": 10934, - "gzip": 3388 + "size": 11040, + "gzip": 3435 }, { "filename": "react-reconciler-reflection.production.min.js", @@ -375,29 +375,29 @@ "filename": "ReactFabric-dev.js", "bundleType": "RN_DEV", "packageName": "react-native-renderer", - "size": 438218, - "gzip": 96267 + "size": 438891, + "gzip": 94687 }, { "filename": "ReactFabric-prod.js", "bundleType": "RN_PROD", "packageName": "react-native-renderer", - "size": 201883, - "gzip": 35448 + "size": 204481, + "gzip": 35139 }, { "filename": "react-reconciler-persistent.development.js", "bundleType": "NODE_DEV", "packageName": "react-reconciler", - "size": 291949, - "gzip": 61587 + "size": 300825, + "gzip": 62279 }, { "filename": "react-reconciler-persistent.production.min.js", "bundleType": "NODE_PROD", "packageName": "react-reconciler", - "size": 41327, - "gzip": 13133 + "size": 44927, + "gzip": 14054 }, { "filename": "react-is.development.js", @@ -431,15 +431,43 @@ "filename": "simple-cache-provider.development.js", "bundleType": "NODE_DEV", "packageName": "simple-cache-provider", - "size": 5830, - "gzip": 1904 + "size": 5759, + "gzip": 1870 }, { "filename": "simple-cache-provider.production.min.js", "bundleType": "NODE_PROD", "packageName": "simple-cache-provider", - "size": 1313, - "gzip": 665 + "size": 1295, + "gzip": 656 + }, + { + "filename": "create-component-with-subscriptions.development.js", + "bundleType": "NODE_DEV", + "packageName": "create-component-with-subscriptions", + "size": 9931, + "gzip": 3067 + }, + { + "filename": "create-component-with-subscriptions.production.min.js", + "bundleType": "NODE_PROD", + "packageName": "create-component-with-subscriptions", + "size": 3783, + "gzip": 1637 + }, + { + "filename": "create-subscription.development.js", + "bundleType": "NODE_DEV", + "packageName": "create-subscription", + "size": 5491, + "gzip": 1896 + }, + { + "filename": "create-subscription.production.min.js", + "bundleType": "NODE_PROD", + "packageName": "create-subscription", + "size": 2190, + "gzip": 1007 } ] } \ No newline at end of file diff --git a/yarn.lock b/yarn.lock index 5f1e2ce632188..2846df0a248db 100644 --- a/yarn.lock +++ b/yarn.lock @@ -4897,6 +4897,12 @@ rx-lite@*, rx-lite@^4.0.8: version "4.0.8" resolved "https://registry.yarnpkg.com/rx-lite/-/rx-lite-4.0.8.tgz#0b1e11af8bc44836f04a6407e92da42467b79444" +rxjs@^5.5.6: + version "5.5.6" + resolved "https://registry.yarnpkg.com/rxjs/-/rxjs-5.5.6.tgz#e31fb96d6fd2ff1fd84bcea8ae9c02d007179c02" + dependencies: + symbol-observable "1.0.1" + safe-buffer@^5.0.1: version "5.0.1" resolved "https://registry.yarnpkg.com/safe-buffer/-/safe-buffer-5.0.1.tgz#d263ca54696cd8a306b5ca6551e92de57918fbe7" @@ -5217,6 +5223,10 @@ supports-hyperlinks@^1.0.1: has-flag "^2.0.0" supports-color "^5.0.0" +symbol-observable@1.0.1: + version "1.0.1" + resolved "https://registry.yarnpkg.com/symbol-observable/-/symbol-observable-1.0.1.tgz#8340fc4702c3122df5d22288f88283f513d3fdd4" + symbol-tree@^3.2.1: version "3.2.2" resolved "https://registry.yarnpkg.com/symbol-tree/-/symbol-tree-3.2.2.tgz#ae27db38f660a7ae2e1c3b7d1bc290819b8519e6" From 33bb14126483b0cac10e0db67ed9a3a45e119570 Mon Sep 17 00:00:00 2001 From: Brian Vaughn Date: Wed, 14 Mar 2018 12:51:20 -0700 Subject: [PATCH 21/21] Updated createRef usage to use .current --- .../ReactDOMServerIntegrationRefs-test.js | 4 ++-- .../src/__tests__/forwardRef-test.internal.js | 14 +++++++------- 2 files changed, 9 insertions(+), 9 deletions(-) diff --git a/packages/react-dom/src/__tests__/ReactDOMServerIntegrationRefs-test.js b/packages/react-dom/src/__tests__/ReactDOMServerIntegrationRefs-test.js index ad3dd46792bd6..37a83b2ff2581 100644 --- a/packages/react-dom/src/__tests__/ReactDOMServerIntegrationRefs-test.js +++ b/packages/react-dom/src/__tests__/ReactDOMServerIntegrationRefs-test.js @@ -114,7 +114,7 @@ describe('ReactDOMServerIntegration', () => { , ); - expect(divRef.value).not.toBe(null); - expect(divRef.value.textContent).toBe('hello'); + expect(divRef.current).not.toBe(null); + expect(divRef.current.textContent).toBe('hello'); }); }); diff --git a/packages/react/src/__tests__/forwardRef-test.internal.js b/packages/react/src/__tests__/forwardRef-test.internal.js index 2f91e5f64d6eb..068448c437400 100644 --- a/packages/react/src/__tests__/forwardRef-test.internal.js +++ b/packages/react/src/__tests__/forwardRef-test.internal.js @@ -62,7 +62,7 @@ describe('forwardRef', () => { ReactNoop.render(); expect(ReactNoop.flush()).toEqual([123]); - expect(ref.value instanceof Child).toBe(true); + expect(ref.current instanceof Child).toBe(true); }); it('should forward a ref for multiple children', () => { @@ -91,7 +91,7 @@ describe('forwardRef', () => {
, ); expect(ReactNoop.flush()).toEqual([123]); - expect(ref.value instanceof Child).toBe(true); + expect(ref.current instanceof Child).toBe(true); }); it('should update refs when switching between children', () => { @@ -112,11 +112,11 @@ describe('forwardRef', () => { ReactNoop.render(); ReactNoop.flush(); - expect(ref.value.type).toBe('div'); + expect(ref.current.type).toBe('div'); ReactNoop.render(); ReactNoop.flush(); - expect(ref.value.type).toBe('span'); + expect(ref.current.type).toBe('span'); }); it('should maintain child instance and ref through updates', () => { @@ -203,7 +203,7 @@ describe('forwardRef', () => { 'ErrorBoundary.componentDidCatch', 'ErrorBoundary.render: catch', ]); - expect(ref.value).toBe(null); + expect(ref.current).toBe(null); }); it('should support rendering null', () => { @@ -213,7 +213,7 @@ describe('forwardRef', () => { ReactNoop.render(); ReactNoop.flush(); - expect(ref.value).toBe(null); + expect(ref.current).toBe(null); }); it('should support rendering null for multiple children', () => { @@ -229,7 +229,7 @@ describe('forwardRef', () => {
, ); ReactNoop.flush(); - expect(ref.value).toBe(null); + expect(ref.current).toBe(null); }); it('should warn if not provided a callback during creation', () => {