From e2280e7afe144de483f62b5fc242ed75405b6e66 Mon Sep 17 00:00:00 2001 From: Dominic Gannaway Date: Tue, 6 Feb 2018 11:50:39 +0000 Subject: [PATCH 1/2] Adds createRef() as per RFC --- .../src/__tests__/ReactComponent-test.js | 49 +++++++++++++++++-- .../__tests__/ReactErrorBoundaries-test.js | 40 ++++++++++++++- .../react-reconciler/src/ReactChildFiber.js | 6 ++- packages/react-reconciler/src/ReactFiber.js | 4 +- .../src/ReactFiberCommitWork.js | 40 ++++++++++----- packages/react/src/React.js | 2 + packages/react/src/ReactCreateRef.js | 20 ++++++++ packages/shared/ReactTypes.js | 4 ++ 8 files changed, 145 insertions(+), 20 deletions(-) create mode 100644 packages/react/src/ReactCreateRef.js diff --git a/packages/react-dom/src/__tests__/ReactComponent-test.js b/packages/react-dom/src/__tests__/ReactComponent-test.js index 94d16543e43..7db461e2c4a 100644 --- a/packages/react-dom/src/__tests__/ReactComponent-test.js +++ b/packages/react-dom/src/__tests__/ReactComponent-test.js @@ -162,9 +162,9 @@ describe('ReactComponent', () => { ReactTestUtils.renderIntoDocument(} />); }); - it('should support new-style refs', () => { - const innerObj = {}; - const outerObj = {}; + it('should support callback-style refs', () => { + var innerObj = {}; + var outerObj = {}; class Wrapper extends React.Component { getObject = () => { @@ -202,6 +202,49 @@ describe('ReactComponent', () => { expect(mounted).toBe(true); }); + it('should support createRef() style refs', () => { + var innerObj = {}; + var outerObj = {}; + + class Wrapper extends React.Component { + getObject = () => { + return this.props.object; + }; + + render() { + return
{this.props.children}
; + } + } + + var mounted = false; + + class Component extends React.Component { + constructor() { + super(); + this.innerRef = React.createRef(); + this.outerRef = React.createRef(); + } + render() { + var inner = ; + var outer = ( + + {inner} + + ); + return outer; + } + + componentDidMount() { + expect(this.innerRef.value.getObject()).toEqual(innerObj); + expect(this.outerRef.value.getObject()).toEqual(outerObj); + mounted = true; + } + } + + ReactTestUtils.renderIntoDocument(); + expect(mounted).toBe(true); + }); + it('should support new-style refs with mixed-up owners', () => { class Wrapper extends React.Component { getTitle = () => { diff --git a/packages/react-dom/src/__tests__/ReactErrorBoundaries-test.js b/packages/react-dom/src/__tests__/ReactErrorBoundaries-test.js index 42fc69cca00..518db954dfa 100644 --- a/packages/react-dom/src/__tests__/ReactErrorBoundaries-test.js +++ b/packages/react-dom/src/__tests__/ReactErrorBoundaries-test.js @@ -937,7 +937,7 @@ describe('ReactErrorBoundaries', () => { expect(log).toEqual(['ErrorBoundary componentWillUnmount']); }); - it('resets refs if mounting aborts', () => { + it('resets callback refs if mounting aborts', () => { function childRef(x) { log.push('Child ref is set to ' + x); } @@ -981,6 +981,44 @@ describe('ReactErrorBoundaries', () => { ]); }); + it('resets object refs if mounting aborts', () => { + let childRef = React.createRef(); + let errorMessageRef = React.createRef(); + + var container = document.createElement('div'); + ReactDOM.render( + +
+ + , + container, + ); + expect(container.textContent).toBe('Caught an error: Hello.'); + expect(log).toEqual([ + 'ErrorBoundary constructor', + 'ErrorBoundary componentWillMount', + 'ErrorBoundary render success', + 'BrokenRender constructor', + 'BrokenRender componentWillMount', + 'BrokenRender render [!]', + // Handle error: + // Finish mounting with null children + 'ErrorBoundary componentDidMount', + // Handle the error + 'ErrorBoundary componentDidCatch', + // Render the error message + 'ErrorBoundary componentWillUpdate', + 'ErrorBoundary render error', + 'ErrorBoundary componentDidUpdate', + ]); + expect(errorMessageRef.value.toString()).toEqual('[object HTMLDivElement]'); + + log.length = 0; + ReactDOM.unmountComponentAtNode(container); + expect(log).toEqual(['ErrorBoundary componentWillUnmount']); + expect(errorMessageRef.value).toEqual(null); + }); + it('successfully mounts if no error occurs', () => { const container = document.createElement('div'); ReactDOM.render( diff --git a/packages/react-reconciler/src/ReactChildFiber.js b/packages/react-reconciler/src/ReactChildFiber.js index b1886ad0a69..81587bd1d7c 100644 --- a/packages/react-reconciler/src/ReactChildFiber.js +++ b/packages/react-reconciler/src/ReactChildFiber.js @@ -94,7 +94,11 @@ const isArray = Array.isArray; function coerceRef(current: Fiber | null, element: ReactElement) { let mixedRef = element.ref; - if (mixedRef !== null && typeof mixedRef !== 'function') { + if ( + mixedRef !== null && + typeof mixedRef !== 'function' && + typeof mixedRef !== 'object' + ) { if (element._owner) { const owner: ?Fiber = (element._owner: any); let inst; diff --git a/packages/react-reconciler/src/ReactFiber.js b/packages/react-reconciler/src/ReactFiber.js index a5f7cd7a3e6..578986fcf22 100644 --- a/packages/react-reconciler/src/ReactFiber.js +++ b/packages/react-reconciler/src/ReactFiber.js @@ -7,7 +7,7 @@ */ import type {ReactElement, Source} from 'shared/ReactElementType'; -import type {ReactPortal} from 'shared/ReactTypes'; +import type {ReactPortal, RefObject} from 'shared/ReactTypes'; import type {TypeOfWork} from 'shared/ReactTypeOfWork'; import type {TypeOfMode} from './ReactTypeOfMode'; import type {TypeOfSideEffect} from 'shared/ReactTypeOfSideEffect'; @@ -107,7 +107,7 @@ export type Fiber = {| // The ref last used to attach this node. // I'll avoid adding an owner field for prod and model that as functions. - ref: null | (((handle: mixed) => void) & {_stringRef: ?string}), + ref: null | (((handle: mixed) => void) & {_stringRef: ?string}) | RefObject, // Input is the data coming into process this fiber. Arguments. Props. pendingProps: any, // This type will be more specific once we overload the tag. diff --git a/packages/react-reconciler/src/ReactFiberCommitWork.js b/packages/react-reconciler/src/ReactFiberCommitWork.js index cf6d598f7ef..b18c8d275c9 100644 --- a/packages/react-reconciler/src/ReactFiberCommitWork.js +++ b/packages/react-reconciler/src/ReactFiberCommitWork.js @@ -77,18 +77,22 @@ export default function( function safelyDetachRef(current: Fiber) { const ref = current.ref; if (ref !== null) { - if (__DEV__) { - invokeGuardedCallback(null, ref, null, null); - if (hasCaughtError()) { - const refError = clearCaughtError(); - captureError(current, refError); + if (typeof ref === 'function') { + if (__DEV__) { + invokeGuardedCallback(null, ref, null, null); + if (hasCaughtError()) { + const refError = clearCaughtError(); + captureError(current, refError); + } + } else { + try { + ref(null); + } catch (refError) { + captureError(current, refError); + } } } else { - try { - ref(null); - } catch (refError) { - captureError(current, refError); - } + ref.value = null; } } } @@ -175,12 +179,18 @@ export default function( const ref = finishedWork.ref; if (ref !== null) { const instance = finishedWork.stateNode; + let instanceToUse; switch (finishedWork.tag) { case HostComponent: - ref(getPublicInstance(instance)); + instanceToUse = getPublicInstance(instance); break; default: - ref(instance); + instanceToUse = instance; + } + if (typeof ref === 'function') { + ref(instanceToUse); + } else { + ref.value = instanceToUse; } } } @@ -188,7 +198,11 @@ export default function( function commitDetachRef(current: Fiber) { const currentRef = current.ref; if (currentRef !== null) { - currentRef(null); + if (typeof currentRef === 'function') { + currentRef(null); + } else { + currentRef.value = null; + } } } diff --git a/packages/react/src/React.js b/packages/react/src/React.js index d6845d8d328..89c777c38c6 100644 --- a/packages/react/src/React.js +++ b/packages/react/src/React.js @@ -14,6 +14,7 @@ import { } from 'shared/ReactSymbols'; import {Component, PureComponent} from './ReactBaseClasses'; +import {createRef} from './ReactCreateRef'; import {forEach, map, count, toArray, only} from './ReactChildren'; import ReactCurrentOwner from './ReactCurrentOwner'; import { @@ -39,6 +40,7 @@ const React = { only, }, + createRef, Component, PureComponent, diff --git a/packages/react/src/ReactCreateRef.js b/packages/react/src/ReactCreateRef.js new file mode 100644 index 00000000000..8af60100e64 --- /dev/null +++ b/packages/react/src/ReactCreateRef.js @@ -0,0 +1,20 @@ +/** + * 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 + */ + +import type {RefObject} from 'shared/ReactTypes'; + +// an immutable object with a single mutable value +export function createRef(): RefObject { + const refObject = { + value: null, + }; + if (__DEV__) { + Object.seal(refObject); + } + return refObject; +} diff --git a/packages/shared/ReactTypes.js b/packages/shared/ReactTypes.js index 9f0dbcff867..26ada3c5e84 100644 --- a/packages/shared/ReactTypes.js +++ b/packages/shared/ReactTypes.js @@ -97,3 +97,7 @@ export type ReactPortal = { // TODO: figure out the API for cross-renderer implementation. implementation: any, }; + +export type RefObject = {| + value: any, +|}; From 9b19254b74e39c6c73f3260b0011cf41923a5941 Mon Sep 17 00:00:00 2001 From: Dominic Gannaway Date: Tue, 6 Feb 2018 12:11:01 +0000 Subject: [PATCH 2/2] Fix linting errors --- .../src/__tests__/ReactComponent-test.js | 16 +- .../__tests__/ReactErrorBoundaries-test.js | 2 +- scripts/rollup/results.json | 154 +++++++++--------- 3 files changed, 86 insertions(+), 86 deletions(-) diff --git a/packages/react-dom/src/__tests__/ReactComponent-test.js b/packages/react-dom/src/__tests__/ReactComponent-test.js index 7db461e2c4a..d08ba34a27d 100644 --- a/packages/react-dom/src/__tests__/ReactComponent-test.js +++ b/packages/react-dom/src/__tests__/ReactComponent-test.js @@ -163,8 +163,8 @@ describe('ReactComponent', () => { }); it('should support callback-style refs', () => { - var innerObj = {}; - var outerObj = {}; + const innerObj = {}; + const outerObj = {}; class Wrapper extends React.Component { getObject = () => { @@ -202,9 +202,9 @@ describe('ReactComponent', () => { expect(mounted).toBe(true); }); - it('should support createRef() style refs', () => { - var innerObj = {}; - var outerObj = {}; + it('should support object-style refs', () => { + const innerObj = {}; + const outerObj = {}; class Wrapper extends React.Component { getObject = () => { @@ -216,7 +216,7 @@ describe('ReactComponent', () => { } } - var mounted = false; + let mounted = false; class Component extends React.Component { constructor() { @@ -225,8 +225,8 @@ describe('ReactComponent', () => { this.outerRef = React.createRef(); } render() { - var inner = ; - var outer = ( + const inner = ; + const outer = ( {inner} diff --git a/packages/react-dom/src/__tests__/ReactErrorBoundaries-test.js b/packages/react-dom/src/__tests__/ReactErrorBoundaries-test.js index 518db954dfa..78d50ee8806 100644 --- a/packages/react-dom/src/__tests__/ReactErrorBoundaries-test.js +++ b/packages/react-dom/src/__tests__/ReactErrorBoundaries-test.js @@ -985,7 +985,7 @@ describe('ReactErrorBoundaries', () => { let childRef = React.createRef(); let errorMessageRef = React.createRef(); - var container = document.createElement('div'); + const container = document.createElement('div'); ReactDOM.render(
diff --git a/scripts/rollup/results.json b/scripts/rollup/results.json index cfea1493f5c..ecf8e41c377 100644 --- a/scripts/rollup/results.json +++ b/scripts/rollup/results.json @@ -4,85 +4,85 @@ "filename": "react.development.js", "bundleType": "UMD_DEV", "packageName": "react", - "size": 56000, - "gzip": 15330 + "size": 56198, + "gzip": 15393 }, { "filename": "react.production.min.js", "bundleType": "UMD_PROD", "packageName": "react", - "size": 6743, - "gzip": 2903 + "size": 6784, + "gzip": 2918 }, { "filename": "react.development.js", "bundleType": "NODE_DEV", "packageName": "react", - "size": 46421, - "gzip": 12997 + "size": 46619, + "gzip": 13063 }, { "filename": "react.production.min.js", "bundleType": "NODE_PROD", "packageName": "react", - "size": 5534, - "gzip": 2445 + "size": 5575, + "gzip": 2457 }, { "filename": "React-dev.js", "bundleType": "FB_DEV", "packageName": "react", - "size": 45838, - "gzip": 12527 + "size": 46036, + "gzip": 12587 }, { "filename": "React-prod.js", "bundleType": "FB_PROD", "packageName": "react", - "size": 13108, - "gzip": 3584 + "size": 13173, + "gzip": 3600 }, { "filename": "react-dom.development.js", "bundleType": "UMD_DEV", "packageName": "react-dom", - "size": 591035, - "gzip": 138248 + "size": 591487, + "gzip": 138323 }, { "filename": "react-dom.production.min.js", "bundleType": "UMD_PROD", "packageName": "react-dom", - "size": 96636, - "gzip": 31413 + "size": 96771, + "gzip": 31441 }, { "filename": "react-dom.development.js", "bundleType": "NODE_DEV", "packageName": "react-dom", - "size": 575044, - "gzip": 134405 + "size": 575502, + "gzip": 134479 }, { "filename": "react-dom.production.min.js", "bundleType": "NODE_PROD", "packageName": "react-dom", - "size": 95362, - "gzip": 30593 + "size": 95496, + "gzip": 30637 }, { "filename": "ReactDOM-dev.js", "bundleType": "FB_DEV", "packageName": "react-dom", - "size": 594192, - "gzip": 136733 + "size": 594660, + "gzip": 136800 }, { "filename": "ReactDOM-prod.js", "bundleType": "FB_PROD", "packageName": "react-dom", - "size": 278297, - "gzip": 53015 + "size": 278632, + "gzip": 53068 }, { "filename": "react-dom-test-utils.development.js", @@ -165,141 +165,141 @@ "filename": "react-dom-server.browser.development.js", "bundleType": "UMD_DEV", "packageName": "react-dom", - "size": 100958, - "gzip": 26662 + "size": 102749, + "gzip": 26863 }, { "filename": "react-dom-server.browser.production.min.js", "bundleType": "UMD_PROD", "packageName": "react-dom", - "size": 15074, - "gzip": 5828 + "size": 15077, + "gzip": 5830 }, { "filename": "react-dom-server.browser.development.js", "bundleType": "NODE_DEV", "packageName": "react-dom", - "size": 90004, - "gzip": 23956 + "size": 91793, + "gzip": 24560 }, { "filename": "react-dom-server.browser.production.min.js", "bundleType": "NODE_PROD", "packageName": "react-dom", - "size": 14708, - "gzip": 5671 + "size": 14711, + "gzip": 5670 }, { "filename": "ReactDOMServer-dev.js", "bundleType": "FB_DEV", "packageName": "react-dom", - "size": 94210, - "gzip": 24064 + "size": 94915, + "gzip": 24259 }, { "filename": "ReactDOMServer-prod.js", "bundleType": "FB_PROD", "packageName": "react-dom", - "size": 33003, - "gzip": 8238 + "size": 33017, + "gzip": 8239 }, { "filename": "react-dom-server.node.development.js", "bundleType": "NODE_DEV", "packageName": "react-dom", - "size": 91972, - "gzip": 24458 + "size": 93761, + "gzip": 25115 }, { "filename": "react-dom-server.node.production.min.js", "bundleType": "NODE_PROD", "packageName": "react-dom", - "size": 15532, + "size": 15535, "gzip": 5978 }, { "filename": "react-art.development.js", "bundleType": "UMD_DEV", "packageName": "react-art", - "size": 386710, - "gzip": 85457 + "size": 388976, + "gzip": 86189 }, { "filename": "react-art.production.min.js", "bundleType": "UMD_PROD", "packageName": "react-art", - "size": 86628, - "gzip": 26939 + "size": 86763, + "gzip": 26952 }, { "filename": "react-art.development.js", "bundleType": "NODE_DEV", "packageName": "react-art", - "size": 310783, - "gzip": 66430 + "size": 313051, + "gzip": 67157 }, { "filename": "react-art.production.min.js", "bundleType": "NODE_PROD", "packageName": "react-art", - "size": 50577, - "gzip": 15966 + "size": 50711, + "gzip": 15991 }, { "filename": "ReactART-dev.js", "bundleType": "FB_DEV", "packageName": "react-art", - "size": 316438, - "gzip": 66256 + "size": 316963, + "gzip": 66329 }, { "filename": "ReactART-prod.js", "bundleType": "FB_PROD", "packageName": "react-art", - "size": 156650, - "gzip": 27129 + "size": 157016, + "gzip": 27217 }, { "filename": "ReactNativeRenderer-dev.js", "bundleType": "RN_DEV", "packageName": "react-native-renderer", - "size": 442459, - "gzip": 97044 + "size": 442933, + "gzip": 97112 }, { "filename": "ReactNativeRenderer-prod.js", "bundleType": "RN_PROD", "packageName": "react-native-renderer", - "size": 209063, - "gzip": 36423 + "size": 209398, + "gzip": 36492 }, { "filename": "react-test-renderer.development.js", "bundleType": "NODE_DEV", "packageName": "react-test-renderer", - "size": 307818, - "gzip": 65370 + "size": 310595, + "gzip": 66239 }, { "filename": "react-test-renderer.production.min.js", "bundleType": "NODE_PROD", "packageName": "react-test-renderer", - "size": 48986, - "gzip": 15317 + "size": 49342, + "gzip": 15389 }, { "filename": "ReactTestRenderer-dev.js", "bundleType": "FB_DEV", "packageName": "react-test-renderer", - "size": 313568, - "gzip": 65199 + "size": 314540, + "gzip": 65394 }, { "filename": "react-test-renderer-shallow.development.js", "bundleType": "NODE_DEV", "packageName": "react-test-renderer", - "size": 19372, - "gzip": 4486 + "size": 21221, + "gzip": 5193 }, { "filename": "react-test-renderer-shallow.production.min.js", @@ -312,8 +312,8 @@ "filename": "ReactShallowRenderer-dev.js", "bundleType": "FB_DEV", "packageName": "react-test-renderer", - "size": 20838, - "gzip": 4545 + "size": 20928, + "gzip": 4566 }, { "filename": "react-noop-renderer.development.js", @@ -333,15 +333,15 @@ "filename": "react-reconciler.development.js", "bundleType": "NODE_DEV", "packageName": "react-reconciler", - "size": 289218, - "gzip": 60807 + "size": 291486, + "gzip": 61540 }, { "filename": "react-reconciler.production.min.js", "bundleType": "NODE_PROD", "packageName": "react-reconciler", - "size": 42266, - "gzip": 13301 + "size": 42400, + "gzip": 13326 }, { "filename": "react-reconciler-reflection.development.js", @@ -375,29 +375,29 @@ "filename": "ReactFabric-dev.js", "bundleType": "RN_DEV", "packageName": "react-native-renderer", - "size": 437242, - "gzip": 96088 + "size": 437716, + "gzip": 96157 }, { "filename": "ReactFabric-prod.js", "bundleType": "RN_PROD", "packageName": "react-native-renderer", - "size": 204787, - "gzip": 35784 + "size": 205122, + "gzip": 35849 }, { "filename": "react-reconciler-persistent.development.js", "bundleType": "NODE_DEV", "packageName": "react-reconciler", - "size": 288790, - "gzip": 60630 + "size": 291058, + "gzip": 61361 }, { "filename": "react-reconciler-persistent.production.min.js", "bundleType": "NODE_PROD", "packageName": "react-reconciler", - "size": 41151, - "gzip": 13111 + "size": 41285, + "gzip": 13145 } ] } \ No newline at end of file