Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

RFC #30: React.forwardRef implementation #12346

Merged
merged 22 commits into from
Mar 14, 2018
Merged
Show file tree
Hide file tree
Changes from 5 commits
Commits
Show all changes
22 commits
Select commit Hold shift + click to select a range
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 6 additions & 0 deletions packages/react-is/src/ReactIs.js
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand All @@ -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;
Expand All @@ -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;
Expand All @@ -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;
}
9 changes: 9 additions & 0 deletions packages/react-is/src/__tests__/ReactIs-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -100,4 +100,13 @@ describe('ReactIs', () => {
expect(ReactIs.isStrictMode(<React.unstable_AsyncMode />)).toBe(false);
expect(ReactIs.isStrictMode(<div />)).toBe(false);
});

it('should identify ref forwarding component', () => {
const RefForwardingComponent = React.useRef((props, ref) => null);
expect(ReactIs.typeOf(<RefForwardingComponent />)).toBe(ReactIs.UseRef);
expect(ReactIs.isUseRef(<RefForwardingComponent />)).toBe(true);
expect(ReactIs.isUseRef({type: ReactIs.StrictMode})).toBe(false);
expect(ReactIs.isUseRef(<React.unstable_AsyncMode />)).toBe(false);
expect(ReactIs.isUseRef(<div />)).toBe(false);
});
});
5 changes: 5 additions & 0 deletions packages/react-reconciler/src/ReactFiber.js
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@ import {
Mode,
ContextProvider,
ContextConsumer,
UseRef,
} from 'shared/ReactTypeOfWork';
import getComponentName from 'shared/getComponentName';

Expand All @@ -42,6 +43,7 @@ import {
REACT_PROVIDER_TYPE,
REACT_CONTEXT_TYPE,
REACT_ASYNC_MODE_TYPE,
REACT_USE_REF_TYPE,
} from 'shared/ReactSymbols';

let hasBadMapPolyfill;
Expand Down Expand Up @@ -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
Expand Down
20 changes: 20 additions & 0 deletions packages/react-reconciler/src/ReactFiberBeginWork.js
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@ import {
Mode,
ContextProvider,
ContextConsumer,
UseRef,
} from 'shared/ReactTypeOfWork';
import {
PerformedWork,
Expand Down Expand Up @@ -166,6 +167,23 @@ export default function<T, P, I, TI, HI, PI, C, CC, CX, PL>(
return workInProgress.child;
}

function updateUseRef(current, workInProgress) {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Wow why are these things so easy with Fiber

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I know! 😁

const renderProp = workInProgress.type.renderProp;
invariant(
typeof renderProp === 'function',
'useRef requires a render function but was given %s.%s',
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This error message isn't great 😄 Any suggestions?

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()) {
Expand Down Expand Up @@ -1102,6 +1120,8 @@ export default function<T, P, I, TI, HI, PI, C, CC, CX, PL>(
return updateFragment(current, workInProgress);
case Mode:
return updateMode(current, workInProgress);
case UseRef:
return updateUseRef(current, workInProgress);
case ContextProvider:
return updateContextProvider(
current,
Expand Down
3 changes: 3 additions & 0 deletions packages/react-reconciler/src/ReactFiberCompleteWork.js
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,7 @@ import {
ContextConsumer,
Fragment,
Mode,
UseRef,
} from 'shared/ReactTypeOfWork';
import {
Placement,
Expand Down Expand Up @@ -617,6 +618,8 @@ export default function<T, P, I, TI, HI, PI, C, CC, CX, PL>(
return null;
case ContextConsumer:
return null;
case UseRef:
return null;
// Error cases
case IndeterminateComponent:
invariant(
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 @@ -24,6 +24,7 @@ import {
isValidElement,
} from './ReactElement';
import {createContext} from './ReactContext';
import useRef from './useRef';
import {
createElementWithValidation,
createFactoryWithValidation,
Expand All @@ -45,6 +46,7 @@ const React = {
PureComponent,

createContext,
useRef,

Fragment: REACT_FRAGMENT_TYPE,
StrictMode: REACT_STRICT_MODE_TYPE,
Expand Down
4 changes: 3 additions & 1 deletion packages/react/src/ReactElementValidator.js
Original file line number Diff line number Diff line change
Expand Up @@ -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';
Expand Down Expand Up @@ -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.
Expand Down
202 changes: 202 additions & 0 deletions packages/react/src/__tests__/useRef-test.internal.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,202 @@
/**
* 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;

function normalizeCodeLocInfo(str) {
return str && str.replace(/\(at .+?:\d+\)/g, '(at **)');
}

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 <Child {...props} ref={props.forwardedRef} />;
}

const RefForwardingComponent = React.useRef((props, ref) => (
<Wrapper {...props} forwardedRef={ref} />
));

ReactNoop.render(<RefForwardingComponent value={123} />);
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 <Child {...props} ref={props.forwardedRef} />;
}

const RefForwardingComponent = React.useRef((props, ref) => (
<Wrapper {...props} forwardedRef={ref} />
));

const ref = React.createRef();

ReactNoop.render(<RefForwardingComponent ref={ref} value={123} />);
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 <Child {...props} ref={props.forwardedRef} />;
}

const RefForwardingComponent = React.useRef((props, ref) => (
<Wrapper {...props} forwardedRef={ref} />
));

let setRefCount = 0;
let ref;

const setRef = r => {
setRefCount++;
ref = r;
};

ReactNoop.render(<RefForwardingComponent ref={setRef} value={123} />);
expect(ReactNoop.flush()).toEqual([123]);
expect(ref instanceof Child).toBe(true);
expect(setRefCount).toBe(1);
ReactNoop.render(<RefForwardingComponent ref={setRef} value={456} />);
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 <BadRender {...props} ref={props.forwardedRef} />;
}

const RefForwardingComponent = React.useRef((props, ref) => (
<Wrapper {...props} forwardedRef={ref} />
));

const ref = React.createRef();

ReactNoop.render(
<ErrorBoundary>
<RefForwardingComponent ref={ref} />
</ErrorBoundary>,
);
expect(ReactNoop.flush()).toEqual([
'ErrorBoundary.render: try',
'Wrapper',
'BadRender throw',
'ErrorBoundary.componentDidCatch',
'ErrorBoundary.render: catch',
]);
expect(ref.value).toBe(null);
});

it('should support rendering null', () => {
const RefForwardingComponent = React.useRef((props, ref) => null);

const ref = React.createRef();

ReactNoop.render(<RefForwardingComponent ref={ref} />);
ReactNoop.flush();
expect(ref.value).toBe(null);
});

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.useRef(null)).toWarnDev(
'useRef requires a render function but was given null.',
);
expect(() => React.useRef('foo')).toWarnDev(
'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(
<div>
<RefForwardingComponent />
</div>,
);

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 **)' : ''),
);
});
});
25 changes: 25 additions & 0 deletions packages/react/src/useRef.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,25 @@
/**
* 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';

import warning from 'fbjs/lib/warning';

export default function useRef<Props, ElementType: React$ElementType>(
renderProp: (props: Props, ref: React$ElementRef<ElementType>) => React$Node,
) {
warning(
typeof renderProp === 'function',
'useRef requires a render function but was given %s.',
renderProp === null ? 'null' : typeof renderProp,
);

return {
$$typeof: REACT_USE_REF_TYPE,
renderProp,
};
}
Loading