Fix dead code elimination for feature flags #11453
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This was referenced Nov 4, 2017
|
May i ask: why there are some special care of |
Collaborator
Author
|
www is Facebook.com codebase. That's where we develop React. Historically it's been a playground for our new features since it lets us get feedback from product engineers without committing to supporting a particular API, and ensure new features are solid before they are pushed into open source. There's nothing preventing you from using our custom builds too, if that's something you want to do. |
Turning flags into named exports fixes dead code elimination. This required some restructuring of how we verify that flag types match up. I used the Check<> trick combined with import typeof, as suggested by @calebmer. For www, we can no longer re-export `require('ReactFeatureFlags')` directly, and instead destructure it. This means flags have to be known at init time. This is already the case so it's not a problem. In fact it may be better since it removes extra property access in tight paths. For things that we *want* to be dynamic on www (currently, only performance flag) we can export a function to toggle it, and then put it on the secret exports. In fact this is better than just letting everyone mutate the flag at arbitrary times since we can provide, e.g., a ref counting interface to it.
Ethan-Arrowood
pushed a commit
to Ethan-Arrowood/react
that referenced
this pull request
Dec 8, 2017
* Fix dead code elimination for feature flags Turning flags into named exports fixes dead code elimination. This required some restructuring of how we verify that flag types match up. I used the Check<> trick combined with import typeof, as suggested by @calebmer. For www, we can no longer re-export `require('ReactFeatureFlags')` directly, and instead destructure it. This means flags have to be known at init time. This is already the case so it's not a problem. In fact it may be better since it removes extra property access in tight paths. For things that we *want* to be dynamic on www (currently, only performance flag) we can export a function to toggle it, and then put it on the secret exports. In fact this is better than just letting everyone mutate the flag at arbitrary times since we can provide, e.g., a ref counting interface to it. * Record sizes
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
4 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Turning flags into named exports fixes the regression in dead code elimination (#11450).
This required some restructuring of how we verify that flag types match up. I used the
Check<>trick combined withimport typeof, as suggested by @calebmer.For www, we can no longer re-export
require('ReactFeatureFlags')directly, and instead destructure it. This means flags have to be known at init time. This is already the case so it's not a problem. In fact it may be better since it removes extra property access in tight paths.For things that we want to be dynamic on www (currently, only performance flag which I haven't added yet) we can export a function to toggle it, and then put it on the secret exports. In fact this is better than just letting everyone mutate the flag at arbitrary times since we can provide, e.g., a ref counting interface to it.