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

Conversation

@lunaruan
Copy link
Contributor

commented Jul 25, 2019

As part of the process to deprecate defaultProps on function components (as per a larger proposal outlined here), add a warning whenever someone does this.

@lunaruan lunaruan requested a review from acdlite Jul 25, 2019

@@ -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!

@@ -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)

@@ -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
Suggested change
afterEach(() => {
ReactFeatureFlags.warnAboutDefaultPropsOnFunctionComponents = false;
})
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?

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.',
ReactTestUtils.renderIntoDocument(<FunctionalComponent />),
).toWarnDev(
'Warning: FunctionalComponent: 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.',
@acdlite

This comment has been minimized.

Copy link
Member

commented Jul 25, 2019

Tests suites that modify feature flags need an internal suffix.

The reason is that we have a CI job that runs our unit tests against the actual npm packages. shared/ReactFeatureFlags doesn't exist in those packages, because it gets inlined by our build pipeline.

@acdlite

This comment has been minimized.

Copy link
Member

commented Jul 25, 2019

Need to update all the feature flag files to include the new one you added. E.g. ReactFeatureFlags.native-oss.js. That's why the Flow and build CI jobs are failing.

Luna Ruan added some commits Jul 25, 2019

Luna Ruan
fixed spelling/wording, moved test to new file, and updated all the R…
…eactFeatureFlag files to include new flag
Luna Ruan
@sizebot

This comment has been minimized.

Copy link

commented Jul 25, 2019

ReactDOM: size: 0.0%, gzip: -0.0%

Details of bundled changes.

Comparing: 121bfb0...9c7d282

react-art

File Filesize Diff Gzip Diff Prev Size Current Size Prev Gzip Current Gzip ENV
react-art.development.js +0.1% +0.1% 641.1 KB 641.98 KB 139.75 KB 139.94 KB UMD_DEV
react-art.production.min.js 0.0% 0.0% 102.43 KB 102.43 KB 31.25 KB 31.25 KB UMD_PROD
react-art.development.js +0.2% +0.2% 571.97 KB 572.85 KB 122.46 KB 122.66 KB NODE_DEV
react-art.production.min.js 0.0% 0.0% 67.47 KB 67.47 KB 20.58 KB 20.59 KB NODE_PROD
ReactART-dev.js +0.1% +0.1% 585.94 KB 586.66 KB 122.07 KB 122.21 KB FB_WWW_DEV

react-dom

File Filesize Diff Gzip Diff Prev Size Current Size Prev Gzip Current Gzip ENV
react-dom.profiling.min.js 0.0% -0.0% 115.17 KB 115.17 KB 36.24 KB 36.23 KB NODE_PROFILING
react-dom-server.browser.development.js +0.2% +0.2% 136.62 KB 136.83 KB 36.11 KB 36.17 KB UMD_DEV
ReactDOM-dev.js +0.1% +0.1% 928.89 KB 929.61 KB 205.42 KB 205.55 KB FB_WWW_DEV
react-dom-server.browser.production.min.js 0.0% -0.0% 19.34 KB 19.34 KB 7.23 KB 7.23 KB UMD_PROD
react-dom-test-utils.development.js +0.4% +0.4% 58.39 KB 58.6 KB 16.04 KB 16.1 KB UMD_DEV
react-dom-unstable-fizz.browser.development.js 0.0% -0.1% 3.78 KB 3.78 KB 1.52 KB 1.52 KB UMD_DEV
react-dom-test-utils.production.min.js 0.0% -0.0% 11.17 KB 11.17 KB 4.1 KB 4.1 KB UMD_PROD
react-dom-unstable-fizz.browser.production.min.js 0.0% 🔺+0.1% 1.21 KB 1.21 KB 705 B 706 B UMD_PROD
react-dom-test-utils.development.js +0.4% +0.4% 56.66 KB 56.87 KB 15.72 KB 15.78 KB NODE_DEV
react-dom-test-utils.production.min.js 0.0% -0.0% 10.95 KB 10.95 KB 4.04 KB 4.04 KB NODE_PROD
react-dom-unstable-fizz.browser.production.min.js 0.0% 🔺+0.2% 1.05 KB 1.05 KB 636 B 637 B NODE_PROD
react-dom.development.js +0.1% +0.1% 905.43 KB 906.3 KB 205.04 KB 205.24 KB UMD_DEV
react-dom.production.min.js 0.0% 0.0% 111.46 KB 111.46 KB 35.79 KB 35.79 KB UMD_PROD
react-dom.profiling.min.js 0.0% 0.0% 114.9 KB 114.9 KB 36.82 KB 36.82 KB UMD_PROFILING
react-dom.development.js +0.1% +0.1% 899.71 KB 900.59 KB 203.55 KB 203.76 KB NODE_DEV
react-dom-server.node.development.js +0.2% +0.2% 134.67 KB 134.88 KB 35.72 KB 35.78 KB NODE_DEV
react-dom-server.node.production.min.js 0.0% -0.0% 20.11 KB 20.11 KB 7.54 KB 7.54 KB NODE_PROD
react-dom-server.browser.development.js +0.2% +0.2% 132.75 KB 132.96 KB 35.18 KB 35.24 KB NODE_DEV
react-dom-unstable-native-dependencies.development.js 0.0% 0.0% 60.76 KB 60.76 KB 15.85 KB 15.85 KB UMD_DEV
react-dom-unstable-native-dependencies.production.min.js 0.0% 0.0% 10.74 KB 10.74 KB 3.68 KB 3.68 KB UMD_PROD
ReactDOMServer-dev.js 0.0% 0.0% 135.1 KB 135.1 KB 34.68 KB 34.68 KB FB_WWW_DEV
react-dom-unstable-native-dependencies.development.js 0.0% 0.0% 60.43 KB 60.43 KB 15.72 KB 15.72 KB NODE_DEV
react-dom-unstable-fizz.node.development.js 0.0% -0.1% 3.85 KB 3.85 KB 1.5 KB 1.5 KB NODE_DEV
react-dom-unstable-native-dependencies.production.min.js 0.0% 0.0% 10.48 KB 10.48 KB 3.58 KB 3.58 KB NODE_PROD
react-dom-unstable-fizz.node.production.min.js 0.0% 🔺+0.2% 1.1 KB 1.1 KB 666 B 667 B NODE_PROD

react-test-renderer

File Filesize Diff Gzip Diff Prev Size Current Size Prev Gzip Current Gzip ENV
ReactTestRenderer-dev.js +0.1% +0.1% 597.64 KB 598.36 KB 124.75 KB 124.88 KB FB_WWW_DEV
react-test-renderer-shallow.development.js 0.0% 0.0% 39.37 KB 39.37 KB 9.92 KB 9.92 KB UMD_DEV
react-test-renderer-shallow.production.min.js 0.0% 0.0% 11.62 KB 11.62 KB 3.54 KB 3.55 KB UMD_PROD
react-test-renderer.development.js +0.1% +0.1% 585.86 KB 586.53 KB 125.4 KB 125.53 KB UMD_DEV
react-test-renderer.production.min.js 0.0% 0.0% 68.96 KB 68.96 KB 21.09 KB 21.09 KB UMD_PROD
react-test-renderer.development.js +0.1% +0.1% 581.4 KB 582.07 KB 124.29 KB 124.42 KB NODE_DEV
react-test-renderer.production.min.js 0.0% 0.0% 68.71 KB 68.71 KB 20.88 KB 20.88 KB NODE_PROD

react-reconciler

File Filesize Diff Gzip Diff Prev Size Current Size Prev Gzip Current Gzip ENV
react-reconciler.development.js +0.2% +0.2% 570.05 KB 570.93 KB 120.95 KB 121.16 KB NODE_DEV
react-reconciler.production.min.js 0.0% 0.0% 68.68 KB 68.68 KB 20.43 KB 20.43 KB NODE_PROD
react-reconciler-reflection.production.min.js 0.0% 🔺+0.1% 2.59 KB 2.59 KB 1.14 KB 1.14 KB NODE_PROD
react-reconciler-persistent.development.js +0.1% +0.1% 567.41 KB 568.08 KB 119.85 KB 119.98 KB NODE_DEV
react-reconciler-persistent.production.min.js 0.0% 0.0% 68.7 KB 68.7 KB 20.43 KB 20.43 KB NODE_PROD

react-native-renderer

File Filesize Diff Gzip Diff Prev Size Current Size Prev Gzip Current Gzip ENV
ReactFabric-profiling.js 0.0% 0.0% 274.37 KB 274.37 KB 47.21 KB 47.21 KB RN_OSS_PROFILING
ReactFabric-dev.js +0.1% +0.1% 732.69 KB 733.42 KB 154.83 KB 154.95 KB RN_FB_DEV
ReactFabric-prod.js 0.0% 0.0% 265.51 KB 265.51 KB 45.72 KB 45.72 KB RN_FB_PROD
ReactNativeRenderer-dev.js +0.1% +0.1% 719.95 KB 720.67 KB 152.34 KB 152.47 KB RN_OSS_DEV
ReactNativeRenderer-dev.js +0.1% +0.1% 720.04 KB 720.77 KB 152.39 KB 152.51 KB RN_FB_DEV
ReactFabric-dev.js +0.1% +0.1% 732.59 KB 733.31 KB 154.78 KB 154.9 KB RN_OSS_DEV

Generated by 🚫 dangerJS

warningWithoutStack(
false,
'%s: Function components will not support defaultProps. ' +
'Use JavaScript default arguments instead.',

This comment has been minimized.

Copy link
@FredyC

FredyC Jul 25, 2019

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

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

This comment has been minimized.

Copy link
@acdlite

acdlite Jul 25, 2019

Member

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

This comment has been minimized.

Copy link
@ljharb

ljharb Jul 25, 2019

Contributor

ideally, a future major release

This comment has been minimized.

Copy link
@acdlite

acdlite Jul 25, 2019

Member

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

Luna Ruan

@lunaruan lunaruan merged commit 857deb2 into facebook:master Jul 25, 2019

12 checks passed

ci/circleci: build Your tests passed on CircleCI!
Details
ci/circleci: flow Your tests passed on CircleCI!
Details
ci/circleci: lint Your tests passed on CircleCI!
Details
ci/circleci: lint_build Your tests passed on CircleCI!
Details
ci/circleci: process_artifacts Your tests passed on CircleCI!
Details
ci/circleci: setup Your tests passed on CircleCI!
Details
ci/circleci: test_build Your tests passed on CircleCI!
Details
ci/circleci: test_build_prod Your tests passed on CircleCI!
Details
ci/circleci: test_dom_fixtures Your tests passed on CircleCI!
Details
ci/circleci: test_source Your tests passed on CircleCI!
Details
ci/circleci: test_source_persistent Your tests passed on CircleCI!
Details
ci/circleci: test_source_prod Your tests passed on CircleCI!
Details

@lunaruan lunaruan deleted the lunaruan:deprecate_default_props branch Jul 25, 2019

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
8 participants
You can’t perform that action at this time.