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

[JoshuaGross]

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.

[sizebot]

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]

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]

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

[JoshuaGross]

@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]

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.)

[yungsters]

Nit: Concise notation.

Suggested change

	extractEvents: function(
	extractEvents(

[gaearon]

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

Yes, I would prefer that.

[JoshuaGross]

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

[gaearon]

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.

[gaearon]

Suggestion: RawEventTelemetryOffByDefault :)

[JoshuaGross]

That is an extremely valid concern.

[JoshuaGross]

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