Skip to content

Commit

Permalink
Adds createRef() as per RFC (#12162)
Browse files Browse the repository at this point in the history
* Adds createRef() as per RFC
  • Loading branch information
trueadm authored Feb 6, 2018
1 parent 3d8f465 commit 8dc8f88
Show file tree
Hide file tree
Showing 9 changed files with 146 additions and 21 deletions.
45 changes: 44 additions & 1 deletion packages/react-dom/src/__tests__/ReactComponent-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -162,7 +162,7 @@ describe('ReactComponent', () => {
ReactTestUtils.renderIntoDocument(<Parent child={<span />} />);
});

it('should support new-style refs', () => {
it('should support callback-style refs', () => {
const innerObj = {};
const outerObj = {};

Expand Down Expand Up @@ -202,6 +202,49 @@ describe('ReactComponent', () => {
expect(mounted).toBe(true);
});

it('should support object-style refs', () => {
const innerObj = {};
const outerObj = {};

class Wrapper extends React.Component {
getObject = () => {
return this.props.object;
};

render() {
return <div>{this.props.children}</div>;
}
}

let mounted = false;

class Component extends React.Component {
constructor() {
super();
this.innerRef = React.createRef();
this.outerRef = React.createRef();
}
render() {
const inner = <Wrapper object={innerObj} ref={this.innerRef} />;
const outer = (
<Wrapper object={outerObj} ref={this.outerRef}>
{inner}
</Wrapper>
);
return outer;
}

componentDidMount() {
expect(this.innerRef.value.getObject()).toEqual(innerObj);
expect(this.outerRef.value.getObject()).toEqual(outerObj);
mounted = true;
}
}

ReactTestUtils.renderIntoDocument(<Component />);
expect(mounted).toBe(true);
});

it('should support new-style refs with mixed-up owners', () => {
class Wrapper extends React.Component {
getTitle = () => {
Expand Down
40 changes: 39 additions & 1 deletion packages/react-dom/src/__tests__/ReactErrorBoundaries-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}
Expand Down Expand Up @@ -981,6 +981,44 @@ describe('ReactErrorBoundaries', () => {
]);
});

it('resets object refs if mounting aborts', () => {
let childRef = React.createRef();
let errorMessageRef = React.createRef();

const container = document.createElement('div');
ReactDOM.render(
<ErrorBoundary errorMessageRef={errorMessageRef}>
<div ref={childRef} />
<BrokenRender />
</ErrorBoundary>,
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(
Expand Down
8 changes: 6 additions & 2 deletions packages/react-reconciler/src/ReactChildFiber.js
Original file line number Diff line number Diff line change
Expand Up @@ -104,7 +104,11 @@ function coerceRef(
element: ReactElement,
) {
let mixedRef = element.ref;
if (mixedRef !== null && typeof mixedRef !== 'function') {
if (
mixedRef !== null &&
typeof mixedRef !== 'function' &&
typeof mixedRef !== 'object'
) {
if (__DEV__) {
if (returnFiber.mode & StrictMode) {
const componentName = getComponentName(returnFiber) || 'Component';
Expand All @@ -113,7 +117,7 @@ function coerceRef(
false,
'A string ref, "%s", has been found within a strict mode tree. ' +
'String refs are a source of potential bugs and should be avoided. ' +
'We recommend using a ref callback instead.' +
'We recommend using createRef() instead.' +
'\n%s' +
'\n\nLearn more about using refs safely here:' +
'\nhttps://fb.me/react-strict-mode-string-ref',
Expand Down
4 changes: 2 additions & 2 deletions packages/react-reconciler/src/ReactFiber.js
Original file line number Diff line number Diff line change
Expand Up @@ -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';
Expand Down Expand Up @@ -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.
Expand Down
40 changes: 27 additions & 13 deletions packages/react-reconciler/src/ReactFiberCommitWork.js
Original file line number Diff line number Diff line change
Expand Up @@ -77,18 +77,22 @@ export default function<T, P, I, TI, HI, PI, C, CC, CX, PL>(
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;
}
}
}
Expand Down Expand Up @@ -175,20 +179,30 @@ export default function<T, P, I, TI, HI, PI, C, CC, CX, PL>(
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;
}
}
}

function commitDetachRef(current: Fiber) {
const currentRef = current.ref;
if (currentRef !== null) {
currentRef(null);
if (typeof currentRef === 'function') {
currentRef(null);
} else {
currentRef.value = null;
}
}
}

Expand Down
2 changes: 2 additions & 0 deletions packages/react/src/React.js
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand All @@ -39,6 +40,7 @@ const React = {
only,
},

createRef,
Component,
PureComponent,

Expand Down
20 changes: 20 additions & 0 deletions packages/react/src/ReactCreateRef.js
Original file line number Diff line number Diff line change
@@ -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;
}
4 changes: 2 additions & 2 deletions packages/react/src/__tests__/ReactStrictMode-test.internal.js
Original file line number Diff line number Diff line change
Expand Up @@ -778,7 +778,7 @@ describe('ReactStrictMode', () => {
}).toWarnDev(
'Warning: A string ref, "somestring", has been found within a strict mode tree. ' +
'String refs are a source of potential bugs and should be avoided. ' +
'We recommend using a ref callback instead.\n\n' +
'We recommend using createRef() instead.\n\n' +
' in OuterComponent (at **)\n\n' +
'Learn more about using refs safely here:\n' +
'https://fb.me/react-strict-mode-string-ref',
Expand Down Expand Up @@ -819,7 +819,7 @@ describe('ReactStrictMode', () => {
}).toWarnDev(
'Warning: A string ref, "somestring", has been found within a strict mode tree. ' +
'String refs are a source of potential bugs and should be avoided. ' +
'We recommend using a ref callback instead.\n\n' +
'We recommend using createRef() instead.\n\n' +
' in InnerComponent (at **)\n' +
' in OuterComponent (at **)\n\n' +
'Learn more about using refs safely here:\n' +
Expand Down
4 changes: 4 additions & 0 deletions packages/shared/ReactTypes.js
Original file line number Diff line number Diff line change
Expand Up @@ -97,3 +97,7 @@ export type ReactPortal = {
// TODO: figure out the API for cross-renderer implementation.
implementation: any,
};

export type RefObject = {|
value: any,
|};

0 comments on commit 8dc8f88

Please sign in to comment.