React Native raw event EventEmitter - intended for app-specific perf listeners and debugging #23232

JoshuaGross · 2022-02-04T21:16:55Z

Summary

This is a replacement / alternative to the Pressability-based event telemetry system that has existed since ~1 year ago. Instead of relying on the internals of the Pressability state machine, and only being able to instrument certain touch events, this system will allow us to instrument any/all events that flow through React Native into ReactJS.

How did you test this change?

Testing manually on an internal stack, with a very small interface.

…utside of Pressability to capture all touch events, and other event types

sizebot · 2022-02-04T21:22:07Z

Comparing: 5318971...143b04b

Critical size changes

Includes critical production bundles, as well as any change greater than 2%:

Name	+/-	Base	Current	+/- gzip	Base gzip	Current gzip
oss-stable/react-dom/cjs/react-dom.production.min.js	=	129.75 kB	129.75 kB	=	41.59 kB	41.59 kB
oss-experimental/react-dom/cjs/react-dom.production.min.js	=	134.93 kB	134.93 kB	=	43.12 kB	43.12 kB
facebook-www/ReactDOM-prod.classic.js	=	428.75 kB	428.75 kB	=	78.70 kB	78.70 kB
facebook-www/ReactDOM-prod.modern.js	=	418.74 kB	418.74 kB	=	77.25 kB	77.25 kB
facebook-www/ReactDOMForked-prod.classic.js	=	428.75 kB	428.75 kB	=	78.70 kB	78.70 kB

Significant size changes

Includes any change greater than 0.2%:

Expand to show

Name	+/-	Base	Current	+/- gzip	Base gzip	Current gzip
react-native/implementations/ReactFabric-dev.js	+0.22%	719.74 kB	721.32 kB	+0.38%	156.53 kB	157.13 kB
react-native/implementations/ReactFabric-dev.fb.js	+0.21%	762.00 kB	763.58 kB	+0.36%	164.29 kB	164.88 kB

Generated by 🚫 dangerJS against 143b04b

gaearon · 2022-02-04T21:25:42Z

Is there a way to catch these before they flow into React? We’ve been trying to gradually move away from this idea of a “plugin system” so adding more of these seems not great.

JoshuaGross · 2022-02-04T21:28:22Z

cc @yungsters any ideas? I think React sees these events before anything sees them in RN?

JoshuaGross · 2022-02-04T21:29:21Z

@gaearon Also, anywhere I can read more about this effort? The plugin system in RN seems to have been untouched, largely, for the past 5+ years, and I wasn't aware of any efforts to change it one way or another.

yungsters · 2022-02-04T21:49:27Z

The first bit of JavaScript that executes in response to an event is dispatchEvent in ReactFabricEventEmitter:

react/packages/react-native-renderer/src/ReactFabricEventEmitter.js

Lines 74 to 101 in 848e802

    
           export function dispatchEvent( 
        
             target: null | Object, 
        
             topLevelType: TopLevelType, 
        
             nativeEvent: AnyNativeEvent, 
        
           ) { 
        
             const targetFiber = (target: null | Fiber); 
        
             let eventTarget = null; 
        
             if (targetFiber != null) { 
        
               const stateNode = targetFiber.stateNode; 
        
               // Guard against Fiber being unmounted 
        
               if (stateNode != null) { 
        
                 eventTarget = stateNode.canonical; 
        
               } 
        
             } 
        
             batchedUpdates(function() { 
        
               // Heritage plugin event system 
        
               runExtractedPluginEventsInBatch( 
        
                 topLevelType, 
        
                 targetFiber, 
        
                 nativeEvent, 
        
                 eventTarget, 
        
               ); 
        
             }); 
        
             // React Native doesn't use ReactControlledComponent but if it did, here's 
        
             // where it would do it. 
        
           }

@gaearon — Are you asking whether there is something that executes before the Event Plugin System? If so, would emitting these events before runExtractedPluginEventsInBatch in that function (that I linked to) address your concerns?

The main upside of doing this ^ would be to avoid conforming more logic to the current Event Plugin System. But for the relatively low complexity of what @JoshuaGross introduces in this pull request, I don't think it is a big deal either way. (This extra plugin is not going to make it much harder for anyone to propose and build a replacement.)

...-renderer/src/__mocks__/react-native/Libraries/ReactPrivate/RawEventTelemetryEventEmitter.js

yungsters · 2022-02-04T21:51:16Z

packages/react-native-renderer/src/ReactNativeEventTelemetryPlugin.js

+import {RawEventTelemetryEventEmitter} from 'react-native/Libraries/ReactPrivate/ReactNativePrivateInterface';
+
+const ReactNativeEventTelemetryPlugin = {
+  extractEvents: function(


Nit: Concise notation.

Suggested change

extractEvents: function(

extractEvents(

gaearon · 2022-02-04T23:23:05Z

If so, would emitting these events before runExtractedPluginEventsInBatch in that function (that I linked to) address your concerns?

Yes, I would prefer that.

…ntEmitter, to reduce reliance on the plugin system and move the emit call further into the core

JoshuaGross · 2022-02-07T19:09:10Z

Thanks for the feedback @gaearon @yungsters, I backed-out changes to the Plugin system and I'm emitting the event from ReactFabricEventEmitter now.

…amed and catchall

gaearon · 2022-02-07T21:59:34Z

packages/react-native-renderer/src/ReactFabricEventEmitter.js

+    // to be notified for all events.
+    // Note that extracted events are *not* emitted into the telemetry system,
+    // only events that have a 1:1 mapping with a native event, at least for now.
+    const topLevelTypeStr: string = ((topLevelType: any): string);


Should we name this somehow to make it clearer this is opt-in and that RN does not collect any telemetry by default? I can absolutely see someone “finding” it in code and writing an HN article about it. Same goes for the comment.

Suggestion: RawEventTelemetryOffByDefault :)

That is an extremely valid concern.

JoshuaGross · 2022-02-07T22:54:06Z

Are the comments I added enough? I'm sort of happy to rename everything to RawEventTelemetryEventEmitterOffByDefault but this name sucks a lot, and will be confusing when it is in use. cc @gaearon @yungsters

packages/react-native-renderer/src/ReactFabricEventEmitter.js

Summary: This sync includes the following changes: - **[a3bde7974](facebook/react@a3bde7974 )**: Exclude react-dom/unstable_testing entry point from stable releases ([#23258](facebook/react#23258)) //<Sebastian Markbåge>// - **[569093276](facebook/react@569093276 )**: Add onErrorShell Callback ([#23247](facebook/react#23247)) //<Sebastian Markbåge>// - **[0dedfcc68](facebook/react@0dedfcc68 )**: Update the exports field ([#23257](facebook/react#23257)) //<Sebastian Markbåge>// - **[9d4e8e84f](facebook/react@9d4e8e84f )**: React Native raw event EventEmitter - intended for app-specific perf listeners and debugging ([#23232](facebook/react#23232)) //<Joshua Gross>// - **[1dece5235](facebook/react@1dece5235 )**: Add back warning with component stack on Hydration mismatch ([#23241](facebook/react#23241)) //<salazarm>// - **[cd4eb116c](facebook/react@cd4eb116c )**: Revert "update node.js version for CI ([#23236](facebook/react#23236))" ([#23239](facebook/react#23239)) //<dan>// - **[1d7728bf9](facebook/react@1d7728bf9 )**: update node.js version for CI ([#23236](facebook/react#23236)) //<sunderls>// - **[848e802d2](facebook/react@848e802d2 )**: Add onRecoverableError option to hydrateRoot, createRoot ([#23207](facebook/react#23207)) //<Andrew Clark>// - **[5318971f5](facebook/react@5318971f5 )**: Remove logic for multiple error recovery attempts ([#23227](facebook/react#23227)) //<Andrew Clark>// - **[3a4462129](facebook/react@3a4462129 )**: Disable avoidThisFallback support in Fizz ([#23224](facebook/react#23224)) //<salazarm>// - **[0318ac2c4](facebook/react@0318ac2c4 )**: Revert 4f5449 //<Ricky>// - **[4f5449eb4](facebook/react@4f5449eb4 )**: Remove main from scheduler `index.js` //<Ricky>// - **[3f5ff16c1](facebook/react@3f5ff16c1 )**: [Hydration] Fallback to client render if server rendered extra nodes ([#23176](facebook/react#23176)) //<salazarm>// - **[529dc3ce8](facebook/react@529dc3ce8 )**: Fix context providers in SSR when handling multiple requests ([#23171](facebook/react#23171)) //<Fran Dios>// - **[505c15c9e](facebook/react@505c15c9e )**: Don't inject timeline hooks unless React supports profiling ([#23151](facebook/react#23151)) //<Brian Vaughn>// - **[e12a9dfc9](facebook/react@e12a9dfc9 )**: Fix production-only updateSyncExternalStore() crash when doing setState in render ([#23150](facebook/react#23150)) //<Douglas Armstrong>// - **[e48940255](facebook/react@e48940255 )**: Warn when a callback ref returns a function ([#23145](facebook/react#23145)) //<Dan Abramov>// - **[d8cfeaf22](facebook/react@d8cfeaf22 )**: Fix context propagation for offscreen/fallback trees ([#23095](facebook/react#23095)) //<Dan Abramov>// - **[d50482478](facebook/react@d50482478 )**: Enable scheduling profiler flag in react-dom/testing builds ([#23142](facebook/react#23142)) //<Brian Vaughn>// - **[790b5246f](facebook/react@790b5246f )**: Fix setState ignored in Safari when iframe is added to DOM in the same commit ([#23111](facebook/react#23111)) //<Dan Abramov>// Changelog: [General][Changed] - React Native sync for revisions 51947a1...a3bde79 jest_e2e[run_all_tests] Reviewed By: ShikaSD Differential Revision: D34108924 fbshipit-source-id: 96acb66c1a7da79d6ef9403490cd0e29ad23d9fb

…listeners and debugging (facebook#23232) * RawEventEmitter: new event perf profiling mechanism outside of Pressability to capture all touch events, and other event types * sync * concise notation * Move event telemetry event emitter call from Plugin to ReactFabricEventEmitter, to reduce reliance on the plugin system and move the emit call further into the core * Backout changes to ReactNativeEventPluginOrder * Properly flow typing event emitter, and emit event to two channels: named and catchall * fix typing for event name string * fix typing for event name string * fix flow * Add more comments about how the event telemetry system works * Add more comments about how the event telemetry system works * rename to RawEventTelemetryEventEmitterOffByDefault * yarn prettier-all * rename event * comments * improve flow types * renamed file

Summary: This sync includes the following changes: - **[a3bde7974](facebook/react@a3bde7974 )**: Exclude react-dom/unstable_testing entry point from stable releases ([facebook#23258](facebook/react#23258)) //<Sebastian Markbåge>// - **[569093276](facebook/react@569093276 )**: Add onErrorShell Callback ([facebook#23247](facebook/react#23247)) //<Sebastian Markbåge>// - **[0dedfcc68](facebook/react@0dedfcc68 )**: Update the exports field ([facebook#23257](facebook/react#23257)) //<Sebastian Markbåge>// - **[9d4e8e84f](facebook/react@9d4e8e84f )**: React Native raw event EventEmitter - intended for app-specific perf listeners and debugging ([facebook#23232](facebook/react#23232)) //<Joshua Gross>// - **[1dece5235](facebook/react@1dece5235 )**: Add back warning with component stack on Hydration mismatch ([facebook#23241](facebook/react#23241)) //<salazarm>// - **[cd4eb116c](facebook/react@cd4eb116c )**: Revert "update node.js version for CI ([facebook#23236](facebook/react#23236))" ([facebook#23239](facebook/react#23239)) //<dan>// - **[1d7728bf9](facebook/react@1d7728bf9 )**: update node.js version for CI ([facebook#23236](facebook/react#23236)) //<sunderls>// - **[848e802d2](facebook/react@848e802d2 )**: Add onRecoverableError option to hydrateRoot, createRoot ([facebook#23207](facebook/react#23207)) //<Andrew Clark>// - **[5318971f5](facebook/react@5318971f5 )**: Remove logic for multiple error recovery attempts ([facebook#23227](facebook/react#23227)) //<Andrew Clark>// - **[3a4462129](facebook/react@3a4462129 )**: Disable avoidThisFallback support in Fizz ([facebook#23224](facebook/react#23224)) //<salazarm>// - **[0318ac2c4](facebook/react@0318ac2c4 )**: Revert 4f5449 //<Ricky>// - **[4f5449eb4](facebook/react@4f5449eb4 )**: Remove main from scheduler `index.js` //<Ricky>// - **[3f5ff16c1](facebook/react@3f5ff16c1 )**: [Hydration] Fallback to client render if server rendered extra nodes ([facebook#23176](facebook/react#23176)) //<salazarm>// - **[529dc3ce8](facebook/react@529dc3ce8 )**: Fix context providers in SSR when handling multiple requests ([facebook#23171](facebook/react#23171)) //<Fran Dios>// - **[505c15c9e](facebook/react@505c15c9e )**: Don't inject timeline hooks unless React supports profiling ([facebook#23151](facebook/react#23151)) //<Brian Vaughn>// - **[e12a9dfc9](facebook/react@e12a9dfc9 )**: Fix production-only updateSyncExternalStore() crash when doing setState in render ([facebook#23150](facebook/react#23150)) //<Douglas Armstrong>// - **[e48940255](facebook/react@e48940255 )**: Warn when a callback ref returns a function ([facebook#23145](facebook/react#23145)) //<Dan Abramov>// - **[d8cfeaf22](facebook/react@d8cfeaf22 )**: Fix context propagation for offscreen/fallback trees ([facebook#23095](facebook/react#23095)) //<Dan Abramov>// - **[d50482478](facebook/react@d50482478 )**: Enable scheduling profiler flag in react-dom/testing builds ([facebook#23142](facebook/react#23142)) //<Brian Vaughn>// - **[790b5246f](facebook/react@790b5246f )**: Fix setState ignored in Safari when iframe is added to DOM in the same commit ([facebook#23111](facebook/react#23111)) //<Dan Abramov>// Changelog: [General][Changed] - React Native sync for revisions 51947a1...a3bde79 jest_e2e[run_all_tests] Reviewed By: ShikaSD Differential Revision: D34108924 fbshipit-source-id: 96acb66c1a7da79d6ef9403490cd0e29ad23d9fb

JoshuaGross added 2 commits February 3, 2022 15:14

ReactNativeEventTelemetryPlugin: new event perf profiling mechanism o…

dd754e6

…utside of Pressability to capture all touch events, and other event types

sync

ac7915f

facebook-github-bot added the CLA Signed label Feb 4, 2022

yungsters reviewed Feb 4, 2022

View reviewed changes

JoshuaGross added 2 commits February 7, 2022 11:00

concise notation

0125355

Move event telemetry event emitter call from Plugin to ReactFabricEve…

d04781d

…ntEmitter, to reduce reliance on the plugin system and move the emit call further into the core

gaearon approved these changes Feb 7, 2022

View reviewed changes

JoshuaGross added 2 commits February 7, 2022 11:10

Backout changes to ReactNativeEventPluginOrder

4b72b07

Properly flow typing event emitter, and emit event to two channels: n…

7e76ce6

…amed and catchall

JoshuaGross changed the title ~~WIP: React Native raw event telemetry plugin~~ React Native raw event telemetry plugin Feb 7, 2022

JoshuaGross added 3 commits February 7, 2022 11:48

fix typing for event name string

97a1b0f

fix typing for event name string

a6deda3

fix flow

bc6905c

gaearon reviewed Feb 7, 2022

View reviewed changes

JoshuaGross added 3 commits February 7, 2022 14:40

Add more comments about how the event telemetry system works

d9a85fe

Add more comments about how the event telemetry system works

229595d

rename to RawEventTelemetryEventEmitterOffByDefault

1825e07

yarn prettier-all

0eef8e9

yungsters requested changes Feb 7, 2022

View reviewed changes

packages/react-native-renderer/src/ReactFabricEventEmitter.js Outdated Show resolved Hide resolved

JoshuaGross added 3 commits February 7, 2022 16:25

rename event

bbf3f78

comments

bff359e

improve flow types

62124f3

JoshuaGross changed the title ~~React Native raw event telemetry plugin~~ React Native raw event EventEmitter - intended for app-specific perf listeners and debugging Feb 8, 2022

renamed file

143b04b

JoshuaGross merged commit 9d4e8e8 into facebook:main Feb 8, 2022

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

React Native raw event EventEmitter - intended for app-specific perf listeners and debugging #23232

React Native raw event EventEmitter - intended for app-specific perf listeners and debugging #23232

JoshuaGross commented Feb 4, 2022

sizebot commented Feb 4, 2022 •

edited

Loading

gaearon commented Feb 4, 2022 •

edited

Loading

JoshuaGross commented Feb 4, 2022

JoshuaGross commented Feb 4, 2022

yungsters commented Feb 4, 2022

yungsters Feb 4, 2022

gaearon commented Feb 4, 2022

JoshuaGross commented Feb 7, 2022

gaearon Feb 7, 2022

gaearon Feb 7, 2022

JoshuaGross Feb 7, 2022

JoshuaGross commented Feb 7, 2022

React Native raw event EventEmitter - intended for app-specific perf listeners and debugging #23232

React Native raw event EventEmitter - intended for app-specific perf listeners and debugging #23232

Conversation

JoshuaGross commented Feb 4, 2022

Summary

How did you test this change?

sizebot commented Feb 4, 2022 • edited Loading

Critical size changes

Significant size changes

gaearon commented Feb 4, 2022 • edited Loading

JoshuaGross commented Feb 4, 2022

JoshuaGross commented Feb 4, 2022

yungsters commented Feb 4, 2022

yungsters Feb 4, 2022

Choose a reason for hiding this comment

gaearon commented Feb 4, 2022

JoshuaGross commented Feb 7, 2022

gaearon Feb 7, 2022

Choose a reason for hiding this comment

gaearon Feb 7, 2022

Choose a reason for hiding this comment

JoshuaGross Feb 7, 2022

Choose a reason for hiding this comment

JoshuaGross commented Feb 7, 2022

sizebot commented Feb 4, 2022 •

edited

Loading

gaearon commented Feb 4, 2022 •

edited

Loading