-
Notifications
You must be signed in to change notification settings - Fork 49.1k
build: make enableComponentPerformanceTrack dynamic for native-fb #33560
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
build: make enableComponentPerformanceTrack dynamic for native-fb #33560
Conversation
Comparing: e1dc034...9aa04f7 Critical size changesIncludes critical production bundles, as well as any change greater than 2%: Significant size changesIncludes any change greater than 0.2%: Expand to show
|
@@ -26,3 +26,4 @@ export const passChildrenWhenCloningPersistedNodes = __VARIANT__; | |||
export const enableLazyPublicInstanceInFabric = __VARIANT__; | |||
export const renameElementSymbol = __VARIANT__; | |||
export const enableFragmentRefs = __VARIANT__; | |||
export const enableComponentPerformanceTrack = __VARIANT__; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think this should be __PROFILE__ && __VARIANT__
so we can enable the feature in dev and profiling, but the code is never included in release builds.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah, that sounds reasonable.
Right now it still compiles out everything what's gated under enableComponentPerformanceTrack
, so production artifacts don't have references to it or console.timeStamp
. enableComponentPerformanceTrack
is a subset of enableProfilerTimer
, which already gated under __PROFILE__
:
react/packages/shared/ReactFeatureFlags.js
Lines 238 to 239 in 5d24c64
// Gather advanced timing metrics for Profiler subtrees. | |
export const enableProfilerTimer = __PROFILE__; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Updated to this to make Flow happy:
react/packages/shared/forks/ReactFeatureFlags.native-fb.js
Lines 86 to 87 in 9aa04f7
export const enableComponentPerformanceTrack: boolean = | |
__PROFILE__ && dynamicFlags.enableComponentPerformanceTrack; |
22664b2
to
8f217ff
Compare
8f217ff
to
9aa04f7
Compare
Summary
Make this flag dynamic, so it can be controlled internally.
How did you test this change?
Build, observe that
console.timeStamp
is only present in FB artifacts andenableComponentPerformanceTrack
is referenced.