False positive getDefaultProps warning when mixing development and production versions #9999

mondwan · 2017-06-19T05:50:17Z

As migrating from 15.4 to 15.5, I am rewrite original react components with create-react-class.

However, there is a warning.

react-15.5.4.js?bust=1497843639843:3287 Warning: getDefaultProps is only used on classic React.createClass definitions. Use a static property named `defaultProps` instead.

As I am really transferring from a classic React.createClass definition, I would like to ask is this warning appropriate? Or, this is a depreciation sames as the level of migrating to create-react-class?

Ooops, I get ticket #9999 :D

The text was updated successfully, but these errors were encountered:

aweary · 2017-06-19T13:59:51Z

@mondwan can you share an example reproducing the issue? I'm not seeing the warning when using the latest create-react-class version, or the last 15.5 release. Example. You can use this JSFiddle as a starting point for reproducing.

mondwan · 2017-06-20T02:00:12Z

https://jsfiddle.net/84v837e9/105/

For non minified version, it works as expected. However, it does not if using minified version

Maybe related to issue #9689?

aweary · 2017-06-20T10:00:49Z

@mondwan the issue is that you're using the minified create-react-class build, but the unminified react build. This means that the internal flag that create-react-class uses to suppress this warning is not applied, so React warns.

Make sure you use the correct build for your environment (minified for production, unminified for development).

mondwan · 2017-06-20T10:16:07Z

Umm, but why this is a warning at the first place? 2017年6月20日 18:03，"Brandon Dail" <[email protected]>寫道：

…

Closed #9999 <#9999>. — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#9999 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/ADuAo2St_74neexrI9m8B4o0p6WQ3uXhks5sF5jkgaJpZM4N9x7V> .

aweary · 2017-06-20T10:30:09Z

Because getDefaultProps is only used for components created with createReactClass. For example, the warning will catch cases where someone is moving from createClass to ES6 classes and they forget to remove getDefaultProps in favor of defaultProps.

You'll never get the warning when using the correct build of create-react-class. It's also important to use minified bundles for react and react-dom in production, as they're much more performant. On the same token, it's useful to use the unminified build in development as it contains a sizeable set of useful checks and warnings.

mondwan · 2017-06-21T01:03:49Z

That's the point. I am still using react.createClass instead of es6 class even though this has been replaced by create-react-class. It should not get a warning at the first place. Or, create-react-class is made of es6 class. So, rules like that are effective? 2017年6月20日 18:32，"Brandon Dail" <[email protected]>寫道：

…

Because getDefaultProps is only used for components created with createReactClass. For example, the warning will catch cases where someone is moving from createClass to ES6 classes and they forget to remove getDefaultProps in favor of defaultProps. You'll never get the warning when using the correct build of create-react-class. It's also important to use minified bundles for react and react-dom in production, as they're much more performant. On the same token, it's useful to use the unminified build in development as it contains a sizeable set of useful checks and warnings. — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#9999 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/ADuAow64dLOLEjrIDwGn5DOSqeebBpttks5sF5-lgaJpZM4N9x7V> .

aweary · 2017-06-21T07:51:08Z

You're getting the warning because you're using the production create-react-class build with the development react build. The production build strips out the code that prevents this warning in development

gaearon · 2017-06-21T09:01:35Z

In general we don't support mixing production and development versions of libraries.

gaearon · 2017-06-21T09:03:15Z

It would still be nice if there was a way to avoid the false positive as this is pretty confusing. Maybe we can turn a flag into a number, and always set it in DEV. If it's not set then we're in PROD mode and shouldn't check.

mondwan · 2017-06-21T09:56:07Z

Ahhh. I see your points. Thanks for the explanations. Actually, it always show warnings about this problem. However, it is hided by development codes and those codes get striped when running in production. In order to get rid of the warnings, I better changes those calls to defaultProps. Am I correct? 2017年6月21日下午3:52，"Brandon Dail" <[email protected]>寫道：

…

You're getting the warning because you're using the production create-react-class build with the development react build. The production build strips out the code that prevents this warning in development — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#9999 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/ADuAo5JvSA_WtxTh_YlhpNCQOTZqDLOlks5sGMvXgaJpZM4N9x7V> .

gaearon · 2017-06-21T10:19:25Z

No, to fix the false positive warnings, you need to either use:

react.js and create-react-class.js (development)
react.min.js and create-react-class.min.js (production)

The mistaken warning occur when you mix development and production versions of these two libraries.

mondwan · 2017-06-21T10:25:19Z

But it shows no warnings after changing to defaultProps as it suggested. 2017年6月21日下午6:21，"Dan Abramov" <[email protected]>寫道：

…

No, to fix the false positive warnings, you need to either use: - react.js and create-react-class.js (development) - react.min.js and create-react-class.min.js (production) The mistaken warning occur when you mix development and production versions of these two libraries. — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#9999 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/ADuAo0l2pw-Cielxxz32lpFlMcvgjIfUks5sGO62gaJpZM4N9x7V> .

gaearon · 2017-06-21T10:27:56Z

Both of them work.

But even without changing it, it will still work correctly with createClass. The only issue is mistaken warning. And that mistaken warning happens because you mix development and production versions.

elCarda · 2017-07-25T14:58:17Z

@gaearon If I do understand it correctly that this can also affect the case when create-react-class is loaded via webpack and minified using uglifyjs during build with whole bundle from sources. The internal token can be named differently -> dozens of false positive warnings in console.

gaearon · 2017-07-25T15:07:47Z

It is not super clear to me what you mean. Uglify doesn't mangle property names because it is generally unsafe.

gaearon · 2018-01-05T21:12:59Z

The action item here is to:

Reproduce the false positive when mixing development and production versions as described below.
Figure out why it happens. You can check create-react-class source code on 15-stable branch. As described above it probably happens because we set some flag only in development.
Figure out a fix.

sportnak · 2018-01-20T21:05:15Z

I'll try looking into this one. I'll update if I'm unable to make any progress. Thanks!

sportnak · 2018-01-21T09:07:05Z

I've opened a PR for this. react-with-addons.[min].js will warn if isReactClassApproved is null. create-react-class only sets this in development mode, but I think it should be set in both. So I removed the check. Running the test suite shows everything passing as before, so it still warns when it should. But I was uncertain on how to add a production script to a dev build.

Here's the test I used that reproduces the error.

    spyOn(console, 'error');
    var altCreateReactClass = require('./create-react-class.js');
    if (process.env.NODE_ENV !== 'production') {
      altCreateReactClass = require('./create-react-class.min.js');
    }

    var Component = altCreateReactClass({
      getDefaultProps: function() {
        return {
          foo: 0
        };
      },

      render: function() {
        return <span />;
      }
    });

    ReactDOM.render(React.createFactory(Component)(), document.createElement('div'));
    expect(console.error.calls.count()).toBe(0);

benschac · 2018-08-14T15:11:15Z

@mondwan @gaearon I'd like to grab a good-first-issue. This one looks a bit stale. If it's still needed I'd love to take a crack at it.

gaearon · 2018-08-14T18:56:56Z

Sure, feel free to look at it. Note #12064 is the most obvious fix but it doesn't actually solve the problem: we want to not do any extra steps in production.

CharlieTruong · 2018-09-16T19:23:02Z

@benschac Are you still poking at this? Would like to take a crack at it if it's not being worked on.

benschac · 2018-09-16T19:35:38Z

Nah, it's all you. Went into the rabbit hole of chatbots :D

…

On Sun, Sep 16, 2018 at 3:25 PM, Charlie Truong ***@***.***> wrote: @benschac <https://github.com/benschac> Are you still poking at this? Would like to take a crack at it if it's not being worked on. — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#9999 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/ACYxIyHoAHEwkH-qRLXtJw9hTYI-ZAuxks5ubqWYgaJpZM4N9x7V> .

ryansaam · 2018-10-01T09:43:37Z

I would like to take this as my good first issue

CharlieTruong · 2018-10-01T13:38:36Z

I have a PR open (referenced above your comment) that attempts to address this issue. Awaiting feedback from @gaearon or someone else. Everyone on the core team likely very busy.

ryansaam · 2018-10-04T10:05:25Z

Yeah I'm sorry I wasn't sure. I should've asked you first. I'm new to contributing so I am still trying to figure out how the flow of things work here. @CharlieTruong

CharlieTruong · 2018-10-04T23:18:10Z

@ryansaam no worries. I'm new to contributing as well so still trying to figure it out. Feel free to look at my PR and provide feedback. May I approached it wrong.

garyxuehong · 2018-11-18T22:59:15Z

Hi I made a PR for this issue. The way I did it is adding a flag for dev mode and skip the check if it is not in dev mode.

I need help for writing the test, especially how to test the mix development/release bundle in the test. Much appreciate if anyone can help.

I also has some questions:

Should I make PR against branch 15-stable or master?
Should I even discuss my changes before I make the PR so it won't wastes someone else's time?

Sorry I am a new comer for react codebase and OSS contribution.

ps: @CharlieTruong , hi my PR is highly inspired from yours. Only thing I changed is to use an explicit flag instead of implicitly rely on the existent of the getDefaultProps function.

jamesyesware · 2018-11-25T01:27:38Z

@garyxuehong The React contributions docs mention that PRs should always be made against master. Commits are cherrypicked for release branches later on. Looks like your PR is pointing to a stable version branch, though!

garyxuehong · 2018-11-26T04:38:03Z

@jamesyesware The file I fixed against is in 15-stable branch only (addons/create-react-class/factory.js). The file is not present in master branch. Does that mean this issue doesn't need to be fixed?

JongminKimSyd · 2019-02-21T03:48:12Z

@jamesyesware and @s-kennedy, it was already explained #12064 (comment) by @gaearon .

george-veras · 2019-05-11T14:46:08Z

Hi! Is this work still needed?
I'm looking to grab a first issue to contribute to, but after spending some time on this thread it seems to have been just left out.

Stexxe · 2020-02-03T07:45:14Z

When function createClass is executed and there is getDefaultProps in the spec and we are not in production mode, then we approve that getDefaultProps is intentional. As was mentioned before we approve react class only in development mode, because of performance reasons, so we cannot approve it in any environment.

I think the problem is not in mixing development and production versions, but in relying on isReactClassApproved property. The error message says that: getDefaultProps is only used on classic React.createClass definitions, so from logic of the message we should warn only if we have getDefaultProps and element was created not from React.createClass, despite the fact of mixing environments.

The table below describes different states and where bug occurs.

react	create-react-class	bug
dev	prod	✓ (`isReactClassApproved` is not set)
dev	dev	- (`isReactClassApproved` is set)
prod	dev	- (no check)
prod	prod	- (no check)

I have three possible ways in mind of how to fix that bug:

We can add extra description of mixing environments issue to warning message to turn the bug into expected behaviour and make it obvious.
We can indicate that type was created from React.createClass by adding some property (like __fromCreateClass) to type on any environment and then check that property exists instead of isReactClassApproved.
From definition of React.createClass, if the spec has getDefaultProps then we execute that function and set its return value to defaultProps property. We could remove that function from Constructor after filling defaultProps if that won't hurt compatibility with older versions.

The question is how we suppose to fix this if there is no React.createClass in master branch, but only in branch 15.6-dev?

rahul3002 · 2021-09-21T05:48:17Z

is anyone Working on this issue Can i?

gsprasanna · 2023-02-23T06:15:18Z

is this issue resolved?

Shurtu-gal · 2023-06-15T17:51:27Z

Hey @mondwan is this still relevant?

mondwan · 2023-06-15T18:25:06Z

To be honest, i am not sure. Hopefully, it was fixed a long time ago.

…

On Thu, 15 Jun 2023, 18:51 Ashish Padhy, ***@***.***> wrote: Hey @mondwan <https://github.com/mondwan> is this still relevant? — Reply to this email directly, view it on GitHub <#9999 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AA5YBI2SOEVVG4JGQRL6LZDXLNDS5ANCNFSM4DPXD3KQ> . You are receiving this because you were mentioned.Message ID: ***@***.***>

techwizard31 · 2024-02-03T16:07:23Z

I want to work on this issue, please give me a chance

QQ07 · 2024-02-12T17:13:56Z

@techwizard31 grow up bud!

strategy155 · 2024-03-01T20:36:55Z

If the issue is not relevant, let's close it then :)

aweary added the Resolution: Needs More Information label Jun 19, 2017

aweary closed this as completed Jun 20, 2017

gaearon reopened this Jun 21, 2017

gaearon changed the title ~~Question on react's warning getDefaultProps~~ False positive getDefaultProps warning when mixing development and production versions Jul 11, 2017

gaearon added Type: Bug and removed Resolution: Needs More Information labels Jul 11, 2017

stevewillard mentioned this issue Aug 3, 2017

getDefaultProps warning in React 15.6 joshwnj/react-visibility-sensor#88

Open

gaearon added the Component: Core Utilities label Oct 4, 2017

guyfedwards mentioned this issue Oct 19, 2017

Bundle is including development and production versions of react webpack/webpack#5851

Closed

gaearon mentioned this issue Oct 27, 2017

Chain dependency causes warnings with create-react-class #11382

Closed

amanmahajan7 mentioned this issue Oct 30, 2017

Getting getDefaultProps warnings with react 15.6+ adazzle/react-data-grid#978

Closed

11 tasks

gaearon added Difficulty: medium good first issue labels Jan 5, 2018

sportnak mentioned this issue Jan 21, 2018

Allow getDefaultProps in dev and production. #12064

Closed

CharlieTruong mentioned this issue Sep 18, 2018

Ensure there are no warnings for getDefaultProps when using createReactClass #13675

Closed

garyxuehong mentioned this issue Nov 19, 2018

Fix false positive getDefaultProps warning when mixing development and production versions #14269

Closed

s-kennedy mentioned this issue Feb 8, 2019

Infinite scroll p2pu/p2pu-components#4

Merged

amensum mentioned this issue Feb 10, 2021

Fix false positive getDefaultProps warning (issue #9999) #20780

Closed

gaearon removed the good first issue label Mar 1, 2024

False positive getDefaultProps warning when mixing development and production versions #9999

False positive getDefaultProps warning when mixing development and production versions #9999

Comments

mondwan commented Jun 19, 2017 • edited Loading

aweary commented Jun 19, 2017

mondwan commented Jun 20, 2017 • edited Loading

aweary commented Jun 20, 2017

mondwan commented Jun 20, 2017 via email

aweary commented Jun 20, 2017

mondwan commented Jun 21, 2017 via email

aweary commented Jun 21, 2017

gaearon commented Jun 21, 2017

gaearon commented Jun 21, 2017

mondwan commented Jun 21, 2017 via email

gaearon commented Jun 21, 2017

mondwan commented Jun 21, 2017 via email

gaearon commented Jun 21, 2017

elCarda commented Jul 25, 2017 • edited Loading

gaearon commented Jul 25, 2017

gaearon commented Jan 5, 2018

sportnak commented Jan 20, 2018

sportnak commented Jan 21, 2018

benschac commented Aug 14, 2018

gaearon commented Aug 14, 2018

CharlieTruong commented Sep 16, 2018

benschac commented Sep 16, 2018 via email

ryansaam commented Oct 1, 2018

CharlieTruong commented Oct 1, 2018 • edited Loading

ryansaam commented Oct 4, 2018 • edited Loading

CharlieTruong commented Oct 4, 2018

garyxuehong commented Nov 18, 2018

jamesyesware commented Nov 25, 2018

garyxuehong commented Nov 26, 2018

JongminKimSyd commented Feb 21, 2019

george-veras commented May 11, 2019

Stexxe commented Feb 3, 2020 • edited Loading

rahul3002 commented Sep 21, 2021

gsprasanna commented Feb 23, 2023

Shurtu-gal commented Jun 15, 2023

mondwan commented Jun 15, 2023 via email

techwizard31 commented Feb 3, 2024

QQ07 commented Feb 12, 2024

strategy155 commented Mar 1, 2024

mondwan commented Jun 19, 2017 •

edited

Loading

mondwan commented Jun 20, 2017 •

edited

Loading

elCarda commented Jul 25, 2017 •

edited

Loading

CharlieTruong commented Oct 1, 2018 •

edited

Loading

ryansaam commented Oct 4, 2018 •

edited

Loading

Stexxe commented Feb 3, 2020 •

edited

Loading