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
Merged
Original file line number Diff line number Diff line change
@@ -0,0 +1,46 @@
/**
* Copyright (c) Facebook, Inc. and its affiliates.
*
* 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';

let React;
let ReactTestUtils;
let ReactFeatureFlags;

describe('ReactDeprecationWarnings', () => {
beforeEach(() => {
jest.resetModules();
React = require('react');
ReactFeatureFlags = require('shared/ReactFeatureFlags');
ReactTestUtils = require('react-dom/test-utils');
ReactFeatureFlags.warnAboutDefaultPropsOnFunctionComponents = true;
});

afterEach(() => {
ReactFeatureFlags.warnAboutDefaultPropsOnFunctionComponents = false;
});

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

FunctionalComponent.defaultProps = {
testProp: true,
};

expect(() =>
ReactTestUtils.renderIntoDocument(<FunctionalComponent />),
).toWarnDev(
'Warning: FunctionalComponent: Function components will not support defaultProps. ' +
'Use JavaScript default arguments instead.',
Copy link
Collaborator

Choose a reason for hiding this comment

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

Alt suggestion for first sentence: "Support for defaultProps will be removed from function components in a future release."

Copy link
Contributor

Choose a reason for hiding this comment

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

ideally, a future major release

Copy link
Collaborator

@acdlite acdlite Jul 25, 2019

Choose a reason for hiding this comment

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

Right, I assumed that was implied but should probably be specific

{withoutStack: true},
);
});
});
Original file line number Diff line number Diff line change
Expand Up @@ -365,6 +365,8 @@ describe('ReactFunctionComponent', () => {
);
});

// TODO: change this test after we deprecate default props support
// for function components
it('should support default props and prop types', () => {
function Child(props) {
return <div>{props.test}</div>;
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 @@ -62,6 +62,7 @@ import {
enableSuspenseServerRenderer,
enableFlareAPI,
enableFundamentalAPI,
warnAboutDefaultPropsOnFunctionComponents,
} from 'shared/ReactFeatureFlags';
import invariant from 'shared/invariant';
import shallowEqual from 'shared/shallowEqual';
Expand Down Expand Up @@ -187,6 +188,7 @@ export let didWarnAboutReassigningProps;
let didWarnAboutMaxDuration;
let didWarnAboutRevealOrder;
let didWarnAboutTailOptions;
let didWarnAboutDefaultPropsOnFunctionComponent;

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

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

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

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

Choose a reason for hiding this comment

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

Might be worth mentioning it's ES2015 feature ... or not. 🤷‍♂

componentName,
);
didWarnAboutDefaultPropsOnFunctionComponent[componentName] = true;
}
}

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

Expand Down
5 changes: 5 additions & 0 deletions packages/shared/ReactFeatureFlags.js
Original file line number Diff line number Diff line change
Expand Up @@ -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 warnAboutDefaultPropsOnFunctionComponents = false;
1 change: 1 addition & 0 deletions packages/shared/forks/ReactFeatureFlags.native-fb.js
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,7 @@ export const warnAboutMissingMockScheduler = true;
export const revertPassiveEffectsChange = false;
export const enableUserBlockingEvents = false;
export const enableSuspenseCallback = false;
export const warnAboutDefaultPropsOnFunctionComponents = false;

// Only used in www builds.
export function addUserTimingListener() {
Expand Down
1 change: 1 addition & 0 deletions packages/shared/forks/ReactFeatureFlags.native-oss.js
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,7 @@ export const warnAboutMissingMockScheduler = false;
export const revertPassiveEffectsChange = false;
export const enableUserBlockingEvents = false;
export const enableSuspenseCallback = false;
export const warnAboutDefaultPropsOnFunctionComponents = false;

// Only used in www builds.
export function addUserTimingListener() {
Expand Down
1 change: 1 addition & 0 deletions packages/shared/forks/ReactFeatureFlags.persistent.js
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,7 @@ export const warnAboutMissingMockScheduler = true;
export const revertPassiveEffectsChange = false;
export const enableUserBlockingEvents = false;
export const enableSuspenseCallback = false;
export const warnAboutDefaultPropsOnFunctionComponents = false;

// Only used in www builds.
export function addUserTimingListener() {
Expand Down
1 change: 1 addition & 0 deletions packages/shared/forks/ReactFeatureFlags.test-renderer.js
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,7 @@ export const warnAboutMissingMockScheduler = false;
export const revertPassiveEffectsChange = false;
export const enableUserBlockingEvents = false;
export const enableSuspenseCallback = false;
export const warnAboutDefaultPropsOnFunctionComponents = false;

// Only used in www builds.
export function addUserTimingListener() {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,7 @@ export const enableJSXTransformAPI = true;
export const warnAboutMissingMockScheduler = true;
export const enableUserBlockingEvents = false;
export const enableSuspenseCallback = true;
export const warnAboutDefaultPropsOnFunctionComponents = false;

// Only used in www builds.
export function addUserTimingListener() {
Expand Down
2 changes: 2 additions & 0 deletions packages/shared/forks/ReactFeatureFlags.www.js
Original file line number Diff line number Diff line change
Expand Up @@ -78,6 +78,8 @@ export const warnAboutMissingMockScheduler = true;

export const enableSuspenseCallback = true;

export const warnAboutDefaultPropsOnFunctionComponents = 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;
Expand Down