Skip to content
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

Refactor DOM plugin system to fewer modules #18025

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
176 changes: 0 additions & 176 deletions packages/legacy-events/EventPluginHub.js

This file was deleted.

10 changes: 3 additions & 7 deletions packages/legacy-events/EventPluginRegistry.js
Original file line number Diff line number Diff line change
Expand Up @@ -89,7 +89,7 @@ function publishEventForPlugin(
): boolean {
invariant(
!eventNameDispatchConfigs.hasOwnProperty(eventName),
'EventPluginHub: More than one plugin attempted to publish the same ' +
'EventPluginRegistry: More than one plugin attempted to publish the same ' +
'event name, `%s`.',
eventName,
);
Expand Down Expand Up @@ -133,7 +133,7 @@ function publishRegistrationName(
): void {
invariant(
!registrationNameModules[registrationName],
'EventPluginHub: More than one plugin attempted to publish the same ' +
'EventPluginRegistry: More than one plugin attempted to publish the same ' +
'registration name, `%s`.',
registrationName,
);
Expand All @@ -153,8 +153,6 @@ function publishRegistrationName(

/**
* Registers plugins so that they can extract and dispatch events.
*
* @see {EventPluginHub}
*/

/**
Expand Down Expand Up @@ -193,7 +191,6 @@ export const possibleRegistrationNames = __DEV__ ? {} : (null: any);
*
* @param {array} InjectedEventPluginOrder
* @internal
* @see {EventPluginHub.injection.injectEventPluginOrder}
*/
export function injectEventPluginOrder(
injectedEventPluginOrder: EventPluginOrder,
Expand All @@ -209,14 +206,13 @@ export function injectEventPluginOrder(
}

/**
* Injects plugins to be used by `EventPluginHub`. The plugin names must be
* Injects plugins to be used by plugin event system. The plugin names must be
* in the ordering injected by `injectEventPluginOrder`.
*
* Plugins can be injected as part of page initialization or on-the-fly.
*
* @param {object} injectedNamesToPlugins Map from names to plugin modules.
* @internal
* @see {EventPluginHub.injection.injectEventPluginsByName}
*/
export function injectEventPluginsByName(
injectedNamesToPlugins: NamesToPlugins,
Expand Down
2 changes: 1 addition & 1 deletion packages/legacy-events/EventPropagators.js
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ import {
traverseEnterLeave,
} from 'shared/ReactTreeTraversal';

import {getListener} from './EventPluginHub';
import getListener from 'legacy-events/getListener';
import accumulateInto from './accumulateInto';
import forEachAccumulated from './forEachAccumulated';

Expand Down
16 changes: 8 additions & 8 deletions packages/legacy-events/ResponderEventPlugin.js
Original file line number Diff line number Diff line change
Expand Up @@ -282,7 +282,7 @@ to return true:wantsResponderID| |
+ + */

/**
* A note about event ordering in the `EventPluginHub`.
* A note about event ordering in the `EventPluginRegistry`.
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We inject into the EventPluginRegistry now, so the comment changes are to reflect that.

*
* Suppose plugins are injected in the following order:
*
Expand All @@ -301,7 +301,7 @@ to return true:wantsResponderID| |
* - When returned from `extractEvents`, deferred-dispatched events contain an
* "accumulation" of deferred dispatches.
* - These deferred dispatches are accumulated/collected before they are
* returned, but processed at a later time by the `EventPluginHub` (hence the
* returned, but processed at a later time by the `EventPluginRegistry` (hence the
* name deferred).
*
* In the process of returning their deferred-dispatched events, event plugins
Expand All @@ -325,9 +325,9 @@ to return true:wantsResponderID| |
* - `R`s on-demand events (if any) (dispatched by `R` on-demand)
* - `S`s on-demand events (if any) (dispatched by `S` on-demand)
* - `C`s on-demand events (if any) (dispatched by `C` on-demand)
* - `R`s extracted events (if any) (dispatched by `EventPluginHub`)
* - `S`s extracted events (if any) (dispatched by `EventPluginHub`)
* - `C`s extracted events (if any) (dispatched by `EventPluginHub`)
* - `R`s extracted events (if any) (dispatched by `EventPluginRegistry`)
* - `S`s extracted events (if any) (dispatched by `EventPluginRegistry`)
* - `C`s extracted events (if any) (dispatched by `EventPluginRegistry`)
*
* In the case of `ResponderEventPlugin`: If the `startShouldSetResponder`
* on-demand dispatch returns `true` (and some other details are satisfied) the
Expand All @@ -336,9 +336,9 @@ to return true:wantsResponderID| |
* will appear as follows:
*
* - `startShouldSetResponder` (`ResponderEventPlugin` dispatches on-demand)
* - `touchStartCapture` (`EventPluginHub` dispatches as usual)
* - `touchStart` (`EventPluginHub` dispatches as usual)
* - `responderGrant/Reject` (`EventPluginHub` dispatches as usual)
* - `touchStartCapture` (`EventPluginRegistry` dispatches as usual)
* - `touchStart` (`EventPluginRegistry` dispatches as usual)
* - `responderGrant/Reject` (`EventPluginRegistry` dispatches as usual)
*/

function setResponderAndExtractTransfer(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -211,7 +211,7 @@ describe('EventPluginRegistry', () => {
expect(function() {
EventPluginRegistry.injectEventPluginOrder(['one', 'two']);
}).toThrowError(
'EventPluginHub: More than one plugin attempted to publish the same ' +
'EventPluginRegistry: More than one plugin attempted to publish the same ' +
'registration name, `onPhotoCapture`.',
);
});
Expand Down
74 changes: 74 additions & 0 deletions packages/legacy-events/getListener.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,74 @@
/**
* Copyright (c) Facebook, Inc. and its 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/ReactFiber';

import invariant from 'shared/invariant';

import {getFiberCurrentPropsFromNode} from './EventPluginUtils';

function isInteractive(tag) {
return (
tag === 'button' ||
tag === 'input' ||
tag === 'select' ||
tag === 'textarea'
);
}

function shouldPreventMouseEvent(name, type, props) {
switch (name) {
case 'onClick':
case 'onClickCapture':
case 'onDoubleClick':
case 'onDoubleClickCapture':
case 'onMouseDown':
case 'onMouseDownCapture':
case 'onMouseMove':
case 'onMouseMoveCapture':
case 'onMouseUp':
case 'onMouseUpCapture':
case 'onMouseEnter':
return !!(props.disabled && isInteractive(type));
default:
return false;
}
}

/**
* @param {object} inst The instance, which is the source of events.
* @param {string} registrationName Name of listener (e.g. `onClick`).
* @return {?function} The stored callback.
*/
export default function getListener(inst: Fiber, registrationName: string) {
let listener;

// TODO: shouldPreventMouseEvent is DOM-specific and definitely should not
// live here; needs to be moved to a better place soon
const stateNode = inst.stateNode;
if (!stateNode) {
// Work in progress (ex: onload events in incremental mode).
return null;
}
const props = getFiberCurrentPropsFromNode(stateNode);
if (!props) {
// Work in progress.
return null;
}
listener = props[registrationName];
if (shouldPreventMouseEvent(registrationName, inst.type, props)) {
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's a shame that this runs for Fabric and RN too. This is definitely something we should remove once we get to clean up the existing DOM event system.

return null;
}
invariant(
!listener || typeof listener === 'function',
'Expected `%s` listener to be a function, instead got a value of `%s` type.',
registrationName,
typeof listener,
);
return listener;
}
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@

jest.mock('../events/isEventSupported');

describe('EventPluginHub', () => {
describe('InvalidEventListeners', () => {
let React;
let ReactTestUtils;

Expand Down
Loading