Skip to content

Commit

Permalink
ForwardRefs supports propTypes (#12911)
Browse files Browse the repository at this point in the history
* Moved some internal forwardRef tests to not be internal
* ForwardRef supports propTypes
  • Loading branch information
bvaughn authored May 29, 2018
1 parent 4f1f909 commit 83f76e4
Show file tree
Hide file tree
Showing 3 changed files with 194 additions and 103 deletions.
49 changes: 36 additions & 13 deletions packages/react/src/ReactElementValidator.js
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,11 @@ import lowPriorityWarning from 'shared/lowPriorityWarning';
import describeComponentFrame from 'shared/describeComponentFrame';
import isValidElementType from 'shared/isValidElementType';
import getComponentName from 'shared/getComponentName';
import {getIteratorFn, REACT_FRAGMENT_TYPE} from 'shared/ReactSymbols';
import {
getIteratorFn,
REACT_FORWARD_REF_TYPE,
REACT_FRAGMENT_TYPE,
} from 'shared/ReactSymbols';
import checkPropTypes from 'prop-types/checkPropTypes';
import warning from 'fbjs/lib/warning';

Expand All @@ -42,10 +46,20 @@ if (__DEV__) {
return '#text';
} else if (typeof element.type === 'string') {
return element.type;
} else if (element.type === REACT_FRAGMENT_TYPE) {
}

const type = element.type;
if (type === REACT_FRAGMENT_TYPE) {
return 'React.Fragment';
} else if (
typeof type === 'object' &&
type !== null &&
type.$$typeof === REACT_FORWARD_REF_TYPE
) {
const functionName = type.render.displayName || type.render.name || '';
return functionName !== '' ? `ForwardRef(${functionName})` : 'ForwardRef';
} else {
return element.type.displayName || element.type.name || 'Unknown';
return type.displayName || type.name || 'Unknown';
}
};

Expand Down Expand Up @@ -213,30 +227,39 @@ function validateChildKeys(node, parentType) {
* @param {ReactElement} element
*/
function validatePropTypes(element) {
const componentClass = element.type;
if (typeof componentClass !== 'function') {
const type = element.type;
let name, propTypes;
if (typeof type === 'function') {
// Class or functional component
name = type.displayName || type.name;
propTypes = type.propTypes;
} else if (
typeof type === 'object' &&
type !== null &&
type.$$typeof === REACT_FORWARD_REF_TYPE
) {
// ForwardRef
const functionName = type.render.displayName || type.render.name || '';
name = functionName !== '' ? `ForwardRef(${functionName})` : 'ForwardRef';
propTypes = type.propTypes;
} else {
return;
}
const name = componentClass.displayName || componentClass.name;
const propTypes = componentClass.propTypes;
if (propTypes) {
currentlyValidatingElement = element;
checkPropTypes(propTypes, element.props, 'prop', name, getStackAddendum);
currentlyValidatingElement = null;
} else if (
componentClass.PropTypes !== undefined &&
!propTypesMisspellWarningShown
) {
} else if (type.PropTypes !== undefined && !propTypesMisspellWarningShown) {
propTypesMisspellWarningShown = true;
warning(
false,
'Component %s declared `PropTypes` instead of `propTypes`. Did you misspell the property assignment?',
name || 'Unknown',
);
}
if (typeof componentClass.getDefaultProps === 'function') {
if (typeof type.getDefaultProps === 'function') {
warning(
componentClass.getDefaultProps.isReactClassApproved,
type.getDefaultProps.isReactClassApproved,
'getDefaultProps is only used on classic React.createClass ' +
'definitions. Use a static property named `defaultProps` instead.',
);
Expand Down
90 changes: 0 additions & 90 deletions packages/react/src/__tests__/forwardRef-test.internal.js
Original file line number Diff line number Diff line change
Expand Up @@ -94,31 +94,6 @@ describe('forwardRef', () => {
expect(ref.current instanceof Child).toBe(true);
});

it('should update refs when switching between children', () => {
function FunctionalComponent({forwardedRef, setRefOnDiv}) {
return (
<section>
<div ref={setRefOnDiv ? forwardedRef : null}>First</div>
<span ref={setRefOnDiv ? null : forwardedRef}>Second</span>
</section>
);
}

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

const ref = React.createRef();

ReactNoop.render(<RefForwardingComponent ref={ref} setRefOnDiv={true} />);
ReactNoop.flush();
expect(ref.current.type).toBe('div');

ReactNoop.render(<RefForwardingComponent ref={ref} setRefOnDiv={false} />);
ReactNoop.flush();
expect(ref.current.type).toBe('span');
});

it('should maintain child instance and ref through updates', () => {
class Child extends React.Component {
constructor(props) {
Expand Down Expand Up @@ -206,32 +181,6 @@ describe('forwardRef', () => {
expect(ref.current).toBe(null);
});

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

const ref = React.createRef();

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

it('should support rendering null for multiple children', () => {
const RefForwardingComponent = React.forwardRef((props, ref) => null);

const ref = React.createRef();

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

it('should not re-run the render callback on a deep setState', () => {
let inst;

Expand Down Expand Up @@ -264,43 +213,4 @@ describe('forwardRef', () => {
inst.setState({});
expect(ReactNoop.flush()).toEqual(['Inner']);
});

it('should warn if not provided a callback during creation', () => {
expect(() => React.forwardRef(undefined)).toWarnDev(
'forwardRef requires a render function but was given undefined.',
);
expect(() => React.forwardRef(null)).toWarnDev(
'forwardRef requires a render function but was given null.',
);
expect(() => React.forwardRef('foo')).toWarnDev(
'forwardRef requires a render function but was given string.',
);
});

it('should warn if no render function is provided', () => {
expect(React.forwardRef).toWarnDev(
'forwardRef requires a render function but was given undefined.',
);
});

it('should warn if the render function provided has propTypes or defaultProps attributes', () => {
function renderWithPropTypes() {
return null;
}
renderWithPropTypes.propTypes = {};

function renderWithDefaultProps() {
return null;
}
renderWithDefaultProps.defaultProps = {};

expect(() => React.forwardRef(renderWithPropTypes)).toWarnDev(
'forwardRef render functions do not support propTypes or defaultProps. ' +
'Did you accidentally pass a React component?',
);
expect(() => React.forwardRef(renderWithDefaultProps)).toWarnDev(
'forwardRef render functions do not support propTypes or defaultProps. ' +
'Did you accidentally pass a React component?',
);
});
});
158 changes: 158 additions & 0 deletions packages/react/src/__tests__/forwardRef-test.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,158 @@
/**
* Copyright (c) 2013-present, Facebook, Inc.
*
* This source code is licensed under the MIT license found in the
* LICENSE file in the root directory of this source tree.
*
* @emails react-core
*/

'use strict';

describe('forwardRef', () => {
let PropTypes;
let React;
let ReactNoop;

beforeEach(() => {
jest.resetModules();
PropTypes = require('prop-types');
React = require('react');
ReactNoop = require('react-noop-renderer');
});

it('should update refs when switching between children', () => {
function FunctionalComponent({forwardedRef, setRefOnDiv}) {
return (
<section>
<div ref={setRefOnDiv ? forwardedRef : null}>First</div>
<span ref={setRefOnDiv ? null : forwardedRef}>Second</span>
</section>
);
}

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

const ref = React.createRef();

ReactNoop.render(<RefForwardingComponent ref={ref} setRefOnDiv={true} />);
ReactNoop.flush();
expect(ref.current.type).toBe('div');

ReactNoop.render(<RefForwardingComponent ref={ref} setRefOnDiv={false} />);
ReactNoop.flush();
expect(ref.current.type).toBe('span');
});

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

const ref = React.createRef();

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

it('should support rendering null for multiple children', () => {
const RefForwardingComponent = React.forwardRef((props, ref) => null);

const ref = React.createRef();

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

it('should support propTypes and defaultProps', () => {
function FunctionalComponent({forwardedRef, optional, required}) {
return (
<div ref={forwardedRef}>
{optional}
{required}
</div>
);
}

const RefForwardingComponent = React.forwardRef(function NamedFunction(
props,
ref,
) {
return <FunctionalComponent {...props} forwardedRef={ref} />;
});
RefForwardingComponent.propTypes = {
optional: PropTypes.string,
required: PropTypes.string.isRequired,
};
RefForwardingComponent.defaultProps = {
optional: 'default',
};

const ref = React.createRef();

ReactNoop.render(
<RefForwardingComponent ref={ref} optional="foo" required="bar" />,
);
ReactNoop.flush();
expect(ref.current.children).toEqual([{text: 'foo'}, {text: 'bar'}]);

ReactNoop.render(<RefForwardingComponent ref={ref} required="foo" />);
ReactNoop.flush();
expect(ref.current.children).toEqual([{text: 'default'}, {text: 'foo'}]);

expect(() =>
ReactNoop.render(<RefForwardingComponent ref={ref} optional="foo" />),
).toWarnDev(
'Warning: Failed prop type: The prop `required` is marked as required in ' +
'`ForwardRef(NamedFunction)`, but its value is `undefined`.\n' +
' in ForwardRef(NamedFunction) (at **)',
);
});

it('should warn if not provided a callback during creation', () => {
expect(() => React.forwardRef(undefined)).toWarnDev(
'forwardRef requires a render function but was given undefined.',
);
expect(() => React.forwardRef(null)).toWarnDev(
'forwardRef requires a render function but was given null.',
);
expect(() => React.forwardRef('foo')).toWarnDev(
'forwardRef requires a render function but was given string.',
);
});

it('should warn if no render function is provided', () => {
expect(React.forwardRef).toWarnDev(
'forwardRef requires a render function but was given undefined.',
);
});

it('should warn if the render function provided has propTypes or defaultProps attributes', () => {
function renderWithPropTypes() {
return null;
}
renderWithPropTypes.propTypes = {};

function renderWithDefaultProps() {
return null;
}
renderWithDefaultProps.defaultProps = {};

expect(() => React.forwardRef(renderWithPropTypes)).toWarnDev(
'forwardRef render functions do not support propTypes or defaultProps. ' +
'Did you accidentally pass a React component?',
);
expect(() => React.forwardRef(renderWithDefaultProps)).toWarnDev(
'forwardRef render functions do not support propTypes or defaultProps. ' +
'Did you accidentally pass a React component?',
);
});
});

0 comments on commit 83f76e4

Please sign in to comment.