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

Warn when Using DefaultProps on Function Components #16210

Merged
merged 8 commits into from Jul 25, 2019
@@ -13,6 +13,7 @@ let PropTypes;
let React;
let ReactDOM;
let ReactTestUtils;
let ReactFeatureFlags;

function FunctionComponent(props) {
return <div>{props.name}</div>;
@@ -25,6 +26,8 @@ describe('ReactFunctionComponent', () => {
React = require('react');
ReactDOM = require('react-dom');
ReactTestUtils = require('react-dom/test-utils');
ReactFeatureFlags = require('shared/ReactFeatureFlags');
ReactFeatureFlags.warnAbouDefaultPropsOnFunctionComponents = true;

This comment has been minimized.

Copy link
@threepointone

threepointone Jul 25, 2019

Contributor

s/warnAbouDefaultPropsOnFunctionComponents/warnAboutDefaultPropsOnFunctionComponents (note the extra t at the end of About)

});

This comment has been minimized.

Copy link
@threepointone

threepointone Jul 25, 2019

Contributor
Suggested change
afterEach(() => {
ReactFeatureFlags.warnAboutDefaultPropsOnFunctionComponents = false;
})
it('should render stateless component', () => {
@@ -206,6 +209,24 @@ describe('ReactFunctionComponent', () => {
ReactTestUtils.renderIntoDocument(<ParentUsingStringRef />);
});

it('should warn when given defaultProps', () => {
function FunctionalComponent(props) {
return null;
}

FunctionalComponent.defaultProps = {
testProp: true,
};

expect(() =>
ReactTestUtils.renderIntoDocument(<FunctionalComponent />),
).toWarnDev(
'Warning: FunctionalComponent: Function components do not support defaultProps. ' +

This comment has been minimized.

Copy link
@ljharb

ljharb Jul 25, 2019

Contributor

do not, or will not in a future version?

'Use Javascript default arguments instead.',

This comment has been minimized.

Copy link
@ljharb

ljharb Jul 25, 2019

Contributor
Suggested change
'Use Javascript default arguments instead.',
'Use JavaScript default arguments instead.',
{withoutStack: true},
);
});

it('should warn when given a function ref', () => {
function Indirection(props) {
return <div>{props.children}</div>;
@@ -365,7 +386,11 @@ describe('ReactFunctionComponent', () => {
);
});

// TODO: deprecate default props support after we remove defaultProps
// from function components

This comment has been minimized.

Copy link
@gaearon

gaearon Jul 25, 2019

Member

Didn't we plan to keep them for classes forever?

This comment has been minimized.

Copy link
@lunaruan

lunaruan Jul 25, 2019

Author Contributor

Oh sorry, poor phrasing. I just meant that I need to change this test at some point. Will rephrase!

it('should support default props and prop types', () => {
ReactFeatureFlags.warnAbouDefaultPropsOnFunctionComponents = false;

function Child(props) {
return <div>{props.test}</div>;
}
@@ -62,6 +62,7 @@ import {
enableSuspenseServerRenderer,
enableFlareAPI,
enableFundamentalAPI,
warnAbouDefaultPropsOnFunctionComponents,
} from 'shared/ReactFeatureFlags';
import invariant from 'shared/invariant';
import shallowEqual from 'shared/shallowEqual';
@@ -187,6 +188,7 @@ export let didWarnAboutReassigningProps;
let didWarnAboutMaxDuration;
let didWarnAboutRevealOrder;
let didWarnAboutTailOptions;
let didWarnAboutDefaultPropsOnFunctionComponent;

if (__DEV__) {
didWarnAboutBadClass = {};
@@ -198,6 +200,7 @@ if (__DEV__) {
didWarnAboutMaxDuration = false;
didWarnAboutRevealOrder = {};
didWarnAboutTailOptions = {};
didWarnAboutDefaultPropsOnFunctionComponent = {};
}

export function reconcileChildren(
@@ -1424,6 +1427,23 @@ function validateFunctionComponentInDev(workInProgress: Fiber, Component: any) {
}
}

if (
warnAbouDefaultPropsOnFunctionComponents &&
Component.defaultProps !== undefined
) {
const componentName = getComponentName(Component) || 'Unknown';

if (!didWarnAboutDefaultPropsOnFunctionComponent[componentName]) {
warningWithoutStack(
false,
'%s: Function components do not support defaultProps. ' +
'Use Javascript default arguments instead.',

This comment has been minimized.

Copy link
@ljharb

ljharb Jul 25, 2019

Contributor
Suggested change
'Use Javascript default arguments instead.',
'Use JavaScript default arguments instead.',
componentName,
);
didWarnAboutDefaultPropsOnFunctionComponent[componentName] = true;
}
}

if (typeof Component.getDerivedStateFromProps === 'function') {
const componentName = getComponentName(Component) || 'Unknown';

@@ -83,3 +83,8 @@ export const enableUserBlockingEvents = false;
// in the update queue. This allows reporting and tracing of what is causing
// the user to see a loading state.
export const enableSuspenseCallback = false;

// Part of the simplification of React.createElement so we can eventually move
// from React.createElement to React.jsx
// https://github.com/reactjs/rfcs/blob/createlement-rfc/text/0000-create-element-changes.md
export const warnAbouDefaultPropsOnFunctionComponents = false;
@@ -38,6 +38,7 @@ export const warnAboutMissingMockScheduler = true;
export const revertPassiveEffectsChange = false;
export const enableUserBlockingEvents = false;
export const enableSuspenseCallback = false;
export const warnAbouDefaultPropsOnFunctionComponents = false;

// Only used in www builds.
export function addUserTimingListener() {
@@ -78,6 +78,8 @@ export const warnAboutMissingMockScheduler = true;

export const enableSuspenseCallback = true;

export const warnAbouDefaultPropsOnFunctionComponents = false;

// Flow magic to verify the exports of this file match the original version.
// eslint-disable-next-line no-unused-vars
type Check<_X, Y: _X, X: Y = _X> = null;
ProTip! Use n and p to navigate between commits in a pull request.
You can’t perform that action at this time.