New home for reliability metrics #10
Conversation
| * Use of this source code is governed by a BSD-style license that can be | ||
| * found in the LICENSE file. | ||
| * | ||
| * @generated by xplat/js/react-native-github/scripts/debugger-frontend/gen-type-defs.js |
There was a problem hiding this comment.
Whats the latest on that? Will we sync these files manually for now?
There was a problem hiding this comment.
Yeah friendship ended with with code gen, ⌘ + C is now my best friend.
Edited to "@generated by WWW D54582949" instead. I still like the VSCode hover warning so people know this isn't the source of truth
| * Use of this source code is governed by a BSD-style license that can be | ||
| * found in the LICENSE file. | ||
| * | ||
| * @generated by WWW D54582949 |
There was a problem hiding this comment.
Ideally this should be a signedsource comment rather than a diff reference. You can put a diff reference in a text file, or just in the commit message when updating any of these files' contents. This way there would be a clear source of truth for the latest WWW diff that touches these definitions. (Especially in the case of partial updates)
| export const RNReliabilityMetrics = ((): RNReliabilityMetrics => { | ||
| function sendEvent(_event: ReactNativeChromeDevToolsEvent): void { | ||
| } | ||
|
|
||
| return { | ||
| sendEvent, | ||
| }; | ||
| })(); |
There was a problem hiding this comment.
Why not
| export const RNReliabilityMetrics = ((): RNReliabilityMetrics => { | |
| function sendEvent(_event: ReactNativeChromeDevToolsEvent): void { | |
| } | |
| return { | |
| sendEvent, | |
| }; | |
| })(); | |
| export const RNReliabilityMetrics: RNReliabilityMetrics = { | |
| sendEvent(_event: ReactNativeChromeDevToolsEvent): void {} | |
| }; |
?
Also, reusing the exact same name for a type and value seems problematic.
There was a problem hiding this comment.
Also: How are we going to actually inject a sendEvent handler here?
There was a problem hiding this comment.
This plan was for this to be a pub/sub API, so sending the actual events won't be here:
EdmondChuiHW@ecf72b6#diff-c62722ca9646ac9ec9e205faf447976c33eccbea2ce1a39791bf6f95d5041233R17
The closure style is my muscle memory for private variables + singleton; I will change it to a class + instance export instead as JS has real private fields now.
Will rework this PR to focus only on what are Reliability Metrics, how are safe defaults included, and the actual listener wiring (similar to facebook/react#22276)
# Summary Accidentally added in #10 # Test plan N / A # Upstreaming plan <!-- Pick one: --> - [ ] This commit should be sent as a patch to the upstream `devtools-frontend` repo. I've reviewed the [contribution guide](https://docs.google.com/document/d/1WNF-KqRSzPLUUfZqQG5AFeU_Ll8TfWYcJasa_XGf7ro/edit#heading=h.9kj7femz1xg5). - [x] This commit is React Native-specific and cannot be upstreamed.
# Summary Accidentally added in #10 # Test plan N / A # Upstreaming plan <!-- Pick one: --> - [ ] This commit should be sent as a patch to the upstream `devtools-frontend` repo. I've reviewed the [contribution guide](https://docs.google.com/document/d/1WNF-KqRSzPLUUfZqQG5AFeU_Ll8TfWYcJasa_XGf7ro/edit#heading=h.9kj7femz1xg5). - [x] This commit is React Native-specific and cannot be upstreamed.
Moved to #16