Skip to content
This repository has been archived by the owner on Sep 10, 2022. It is now read-only.

should isReferentiallyTransparentFunctionComponent be completely off for non-production env? #442

Closed
deepsweet opened this issue Jul 24, 2017 · 6 comments

Comments

@deepsweet
Copy link
Contributor

Hi.

Right now

(process.env.NODE_ENV === 'production' || !Component.propTypes)

from here leads to lack of actual element while testing with Enzyme (NODE_ENV=test) so very dummy stateless components can't be found in a tree. Same problem with react-dev-tools (NODE_ENV=development), I can't see the component because function was directly invoked with props.

I understand the purpose of createEagerFactory, and the idea is really nice, but in my opinion it should be production only optimization. Should we consider removing || !Component.propTypes part from the condition or something like this? It's really confusing and took a lot of time to find the reason.

@deepsweet deepsweet changed the title should isReferentiallyTransparentFunctionComponent be completely off not for production? should isReferentiallyTransparentFunctionComponent be completely off for non-production env? Jul 24, 2017
@istarkov
Copy link
Contributor

istarkov commented Jul 24, 2017

The same confusion as here let us think a little,
seems like you are right
cc @wuct your thoughts?

The only problem I see is small perf degrade at dev mode.

@deepsweet
Copy link
Contributor Author

deepsweet commented Jul 24, 2017

Also it sounds like a breaking change because it might break many tests based on children count or (which is more likely) Jest snapshots.

The only problem I see is small perf degrade at dev mode.

One can't expect much performance from React in dev mode, all of these warnings, PropTypes, setter guards, and so on :) As for me it's totally fine.

@deepsweet
Copy link
Contributor Author

Guys, sorry for pushing, but is there any conclusion? Plans for the next major version?

@istarkov
Copy link
Contributor

istarkov commented Aug 8, 2017

I remember about this, just took a few weeks break with any computer work. Now Im drinking vodka with bears in deep Russia and have mobile phone only ;-)

@istarkov
Copy link
Contributor

istarkov commented Aug 8, 2017

You can provide a PR with 90% probability we will accept it

@wuct
Copy link
Contributor

wuct commented Aug 14, 2017

Sorry for late reply and yes, a PR is welcome. This change probably can land at 0.25.

🐻

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants