Skip to content

Commit

Permalink
Removed unused imperative events implementation from React Native ren…
Browse files Browse the repository at this point in the history 
…derer (#26282)

## Summary

I'm going to start implementing parts of this proposal
react-native-community/discussions-and-proposals#607

As part of that implementation I'm going to refactor a few parts of the
interface between React and React Native. One of the main problems we
have right now is that we have private parts used by React and React
Native in the public instance exported by refs. I want to properly
separate that.

I saw that a few methods to attach event handlers imperatively on refs
were also exposing some things in the public instance (the
`_eventListeners`). I checked and these methods are unused, so we can
just clean them up instead of having to refactor them too. Adding
support for imperative event listeners is in the roadmap after this
proposal, and its implementation might differ after this refactor.

This is essentially a manual revert of #23386.

I'll submit more PRs after this for the rest of the refactor.

## How did you test this change?

Existing jest tests. Will test a React sync internally at Meta.
  • Loading branch information
@rubennorte
rubennorte authored Mar 2, 2023
1 parent 4111002 commit d49e0e0
Show file tree
Hide file tree
Showing 10 changed files with 61 additions and 368 deletions.
4 changes: 2 additions & 2 deletions packages/react-native-renderer/src/ReactFabricEventEmitter.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -25,12 +25,12 @@ import {
import {batchedUpdates} from './legacy-events/ReactGenericBatching';
import accumulateInto from './legacy-events/accumulateInto';

import getListeners from './ReactNativeGetListeners';
import getListener from './ReactNativeGetListener';
import {runEventsInBatch} from './legacy-events/EventBatching';

import {RawEventEmitter} from 'react-native/Libraries/ReactPrivate/ReactNativePrivateInterface';

export {getListeners, registrationNameModules as registrationNames};
export {getListener, registrationNameModules as registrationNames};

/**
* Allows registered plugins an opportunity to extract events from top-level
Expand Down
122 changes: 0 additions & 122 deletions packages/react-native-renderer/src/ReactFabricHostConfig.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -99,28 +99,6 @@ export type RendererInspectionConfig = $ReadOnly<{
) => void,
}>;

// TODO?: find a better place for this type to live
export type EventListenerOptions = $ReadOnly<{
capture?: boolean,
once?: boolean,
passive?: boolean,
signal: mixed, // not yet implemented
}>;
export type EventListenerRemoveOptions = $ReadOnly<{
capture?: boolean,
}>;

// TODO?: this will be changed in the future to be w3c-compatible and allow "EventListener" objects as well as functions.
export type EventListener = Function;

type InternalEventListeners = {
[string]: {
listener: EventListener,
options: EventListenerOptions,
invalidated: boolean,
}[],
};

// TODO: Remove this conditional once all changes have propagated.
if (registerEventHandler) {
/**
Expand All @@ -137,7 +115,6 @@ class ReactFabricHostComponent {
viewConfig: ViewConfig;
currentProps: Props;
_internalInstanceHandle: Object;
_eventListeners: ?InternalEventListeners;

constructor(
tag: number,
Expand Down Expand Up @@ -236,105 +213,6 @@ class ReactFabricHostComponent {
setNativeProps(stateNode.node, updatePayload);
}
}

// This API (addEventListener, removeEventListener) attempts to adhere to the
// w3 Level2 Events spec as much as possible, treating HostComponent as a DOM node.
//
// Unless otherwise noted, these methods should "just work" and adhere to the W3 specs.
// If they deviate in a way that is not explicitly noted here, you've found a bug!
//
// See:
// * https://www.w3.org/TR/DOM-Level-2-Events/events.html
// * https://developer.mozilla.org/en-US/docs/Web/API/EventTarget/addEventListener
// * https://developer.mozilla.org/en-US/docs/Web/API/EventTarget/removeEventListener
//
// And notably, not implemented (yet?):
// * https://developer.mozilla.org/en-US/docs/Web/API/EventTarget/dispatchEvent
//
//
// Deviations from spec/TODOs:
// (1) listener must currently be a function, we do not support EventListener objects yet.
// (2) we do not support the `signal` option / AbortSignal yet
addEventListener_unstable(
eventType: string,
listener: EventListener,
options: EventListenerOptions | boolean,
// $FlowFixMe[missing-local-annot]
) {
if (typeof eventType !== 'string') {
throw new Error('addEventListener_unstable eventType must be a string');
}
if (typeof listener !== 'function') {
throw new Error('addEventListener_unstable listener must be a function');
}

// The third argument is either boolean indicating "captures" or an object.
const optionsObj =
typeof options === 'object' && options !== null ? options : {};
const capture =
(typeof options === 'boolean' ? options : optionsObj.capture) || false;
const once = optionsObj.once || false;
const passive = optionsObj.passive || false;
const signal = null; // TODO: implement signal/AbortSignal

/* $FlowFixMe the old version of Flow doesn't have a good way to define an
* empty exact object. */
const eventListeners: InternalEventListeners = this._eventListeners || {};
if (this._eventListeners == null) {
this._eventListeners = eventListeners;
}

const namedEventListeners = eventListeners[eventType] || [];
if (eventListeners[eventType] == null) {
eventListeners[eventType] = namedEventListeners;
}

namedEventListeners.push({
listener: listener,
invalidated: false,
options: {
capture: capture,
once: once,
passive: passive,
signal: signal,
},
});
}

// See https://developer.mozilla.org/en-US/docs/Web/API/EventTarget/removeEventListener
removeEventListener_unstable(
eventType: string,
listener: EventListener,
options: EventListenerRemoveOptions | boolean,
) {
// eventType and listener must be referentially equal to be removed from the listeners
// data structure, but in "options" we only check the `capture` flag, according to spec.
// That means if you add the same function as a listener with capture set to true and false,
// you must also call removeEventListener twice with capture set to true/false.
const optionsObj =
typeof options === 'object' && options !== null ? options : {};
const capture =
(typeof options === 'boolean' ? options : optionsObj.capture) || false;

// If there are no event listeners or named event listeners, we can bail early - our
// job is already done.
const eventListeners = this._eventListeners;
if (!eventListeners) {
return;
}
const namedEventListeners = eventListeners[eventType];
if (!namedEventListeners) {
return;
}

// TODO: optimize this path to make remove cheaper
eventListeners[eventType] = namedEventListeners.filter(listenerObj => {
return !(
listenerObj.listener === listener &&
listenerObj.options.capture === capture
);
});
}
}

// $FlowFixMe[class-object-subtyping] found when upgrading Flow
Expand Down
57 changes: 20 additions & 37 deletions packages/react-native-renderer/src/ReactNativeBridgeEventPlugin.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -13,15 +13,13 @@ import type {
} from './legacy-events/PluginModuleType';
import type {TopLevelType} from './legacy-events/TopLevelEventTypes';
import SyntheticEvent from './legacy-events/SyntheticEvent';
import type {PropagationPhases} from './legacy-events/PropagationPhases';

// Module provided by RN:
import {ReactNativeViewConfigRegistry} from 'react-native/Libraries/ReactPrivate/ReactNativePrivateInterface';
import accumulateInto from './legacy-events/accumulateInto';
import getListeners from './ReactNativeGetListeners';
import getListener from './ReactNativeGetListener';
import forEachAccumulated from './legacy-events/forEachAccumulated';
import {HostComponent} from 'react-reconciler/src/ReactWorkTags';
import isArray from 'shared/isArray';

const {customBubblingEventTypes, customDirectEventTypes} =
ReactNativeViewConfigRegistry;
Expand All @@ -30,38 +28,10 @@ const {customBubblingEventTypes, customDirectEventTypes} =
// EventPropagator.js, as they deviated from ReactDOM's newer
// implementations.
// $FlowFixMe[missing-local-annot]
function listenersAtPhase(inst, event, propagationPhase: PropagationPhases) {
function listenerAtPhase(inst, event, propagationPhase: PropagationPhases) {
const registrationName =
event.dispatchConfig.phasedRegistrationNames[propagationPhase];
return getListeners(inst, registrationName, propagationPhase, true);
}

// $FlowFixMe[missing-local-annot]
function accumulateListenersAndInstances(inst, event, listeners) {
const listenersLength = listeners
? isArray(listeners)
? listeners.length
: 1
: 0;
if (listenersLength > 0) {
event._dispatchListeners = accumulateInto(
event._dispatchListeners,
listeners,
);

// Avoid allocating additional arrays here
if (event._dispatchInstances == null && listenersLength === 1) {
event._dispatchInstances = inst;
} else {
event._dispatchInstances = event._dispatchInstances || [];
if (!isArray(event._dispatchInstances)) {
event._dispatchInstances = [event._dispatchInstances];
}
for (let i = 0; i < listenersLength; i++) {
event._dispatchInstances.push(inst);
}
}
}
return getListener(inst, registrationName);
}

// $FlowFixMe[missing-local-annot]
Expand All @@ -71,8 +41,14 @@ function accumulateDirectionalDispatches(inst, phase, event) {
console.error('Dispatching inst must not be null');
}
}
const listeners = listenersAtPhase(inst, event, phase);
accumulateListenersAndInstances(inst, event, listeners);
const listener = listenerAtPhase(inst, event, phase);
if (listener) {
event._dispatchListeners = accumulateInto(
event._dispatchListeners,
listener,
);
event._dispatchInstances = accumulateInto(event._dispatchInstances, inst);
}
}

// $FlowFixMe[missing-local-annot]
Expand Down Expand Up @@ -160,8 +136,14 @@ function accumulateDispatches(
): void {
if (inst && event && event.dispatchConfig.registrationName) {
const registrationName = event.dispatchConfig.registrationName;
const listeners = getListeners(inst, registrationName, 'bubbled', false);
accumulateListenersAndInstances(inst, event, listeners);
const listener = getListener(inst, registrationName);
if (listener) {
event._dispatchListeners = accumulateInto(
event._dispatchListeners,
listener,
);
event._dispatchInstances = accumulateInto(event._dispatchInstances, inst);
}
}
}

Expand All @@ -181,6 +163,7 @@ function accumulateDirectDispatches(events: ?(Array<Object> | Object)) {
}

// End of inline
type PropagationPhases = 'bubbled' | 'captured';

const ReactNativeBridgeEventPlugin = {
eventTypes: ({}: EventTypes),
Expand Down
4 changes: 2 additions & 2 deletions packages/react-native-renderer/src/ReactNativeEventEmitter.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -21,12 +21,12 @@ import {
} from './legacy-events/EventPluginRegistry';
import {batchedUpdates} from './legacy-events/ReactGenericBatching';
import {runEventsInBatch} from './legacy-events/EventBatching';
import getListeners from './ReactNativeGetListeners';
import getListener from './ReactNativeGetListener';
import accumulateInto from './legacy-events/accumulateInto';

import {getInstanceFromNode} from './ReactNativeComponentTree';

export {getListeners, registrationNameModules as registrationNames};
export {getListener, registrationNameModules as registrationNames};

/**
* Version of `ReactBrowserEventEmitter` that works on the receiving side of a
Expand Down
37 changes: 37 additions & 0 deletions packages/react-native-renderer/src/ReactNativeGetListener.js
View file
Original file line number Diff line number Diff line change
@@ -0,0 +1,37 @@
/**
* Copyright (c) Meta Platforms, Inc. and affiliates.
*
* This source code is licensed under the MIT license found in the
* LICENSE file in the root directory of this source tree.
*
* @flow
*/

import type {Fiber} from 'react-reconciler/src/ReactInternalTypes';

import {getFiberCurrentPropsFromNode} from './legacy-events/EventPluginUtils';

export default function getListener(
inst: Fiber,
registrationName: string,
): Function | null {
const stateNode = inst.stateNode;
if (stateNode === null) {
// Work in progress (ex: onload events in incremental mode).
return null;
}
const props = getFiberCurrentPropsFromNode(stateNode);
if (props === null) {
// Work in progress.
return null;
}
const listener = props[registrationName];

if (listener && typeof listener !== 'function') {
throw new Error(
`Expected \`${registrationName}\` listener to be a function, instead got a value of \`${typeof listener}\` type.`,
);
}

return listener;
}
Loading

0 comments on commit d49e0e0

Please sign in to comment.