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

Introduce __PROFILE__ build #6627

Open
gaearon opened this issue Apr 27, 2016 · 4 comments

Comments

Projects
None yet
5 participants
@gaearon
Copy link
Member

commented Apr 27, 2016

As discussed in #6015, we plan to add a new build configuration that does not have the __DEV__ overhead but that comes with the new ReactPerf enabled.

This means that developer warnings, etc, will need to be gated by __DEV__, but the component tree (#6549) and some other events (e.g. #6612) will need to be gated by __PROFILE__.

I’m curious how this could be implemented. Right now our system is simple:

Current System

Variables

  • __DEV__ = (process.env.NODE_ENV !== 'production')

Development Build (any NODE_ENV except 'production')

  • __DEV__ is true

Production Build (NODE_ENV is 'production')

  • __DEV__ is false

As you can see, if process.env.NODE_ENV is omitted, we assume __DEV__ mode. This is a sensible assumption, and not the one we want to change, as most projects today don’t specify anything as NODE_ENV in development, and we don’t want them to suddenly lose all developer warnings.

Therefore, I propose the following new system:

Proposed System

Variables

  • __DEV__ = (process.env.NODE_ENV !== 'profile' && process.env.NODE_ENV !== 'production')
  • __PROFILE__ = (process.env.NODE_ENV === 'profile')

Development Build (any NODE_ENV except 'profile' or 'production')

  • __DEV__ is true
  • __PROFILE__ is true

Profile Build (NODE_ENV is 'profile')

  • __DEV__ is false
  • __PROFILE__ is true

Production Build (NODE_ENV is 'production')

  • __DEV__ is false
  • __PROFILE__ is false

This would let us have three separate build configurations. We can use the same pattern in other fbjs projects as well, if desired. I would say it’s unlikely we’d ever want to add a separate fourth configuration so this should cover all our needs.

Any thoughts why this would be a bad idea? Should I implement this in fbjs?

cc @facebook/react-core

@gaearon

This comment has been minimized.

Copy link
Member Author

commented Apr 27, 2016

If we go with this system, we’ll also probably want to run some tests (like ReactPerf tests) with __DEV__ set to false but __PROFILE__ set to true, to emulate the profile build.

@gaearon gaearon added this to the 15.x milestone Apr 27, 2016

@sebmarkbage

This comment has been minimized.

Copy link
Member

commented Apr 27, 2016

Makes sense to me. The only issue here is what we do about internal Facebook code and React Native because this system doesn't exist there. We would probably need to make this push universal and talk to stakeholders instead of just going our own way. At the very least we'll need a way to deploy this to www where we don't use the pre-compiled version. cc @zpao

@ForbesLindesay

This comment has been minimized.

Copy link

commented Apr 27, 2016

I think for profiling you want libraries like redux in prod mode by default. I would therefore stick to only two values for NODE_ENV and instead add a new PROFILING variable (which would default to NODE_ENV != production

@AlicanC

This comment has been minimized.

Copy link

commented Sep 7, 2016

REACT_ENV = process.env.REACT_ENV || process.env.NODE_ENV;
__DEV__ = (REACT_ENV !== 'profile' && REACT_ENV !== 'production');
__PROFILE__ = __DEV__ || (REACT_ENV === 'profile');

NODE_ENV=production REACT_ENV=profile npm run whatever

Making it like this fixes both @ForbesLindesay's problem and #7512.

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