Skip to content

Commit

Permalink
Remove deleteListener, make setListener take null/void to clear
Browse files Browse the repository at this point in the history
  • Loading branch information
trueadm committed Dec 22, 2019
1 parent 85409f5 commit 87c7c7d
Show file tree
Hide file tree
Showing 10 changed files with 41 additions and 82 deletions.
6 changes: 1 addition & 5 deletions packages/react-art/src/ReactARTHostConfig.js
Original file line number Diff line number Diff line change
Expand Up @@ -485,10 +485,6 @@ export function detachListenerFromInstance(listener: any): any {
// noop
}

export function validateReactListenerDeleteListener(instance): void {
// noop
}

export function validateReactListenerMapListener(instance, listener): void {
export function validateReactListenerMapSetListener(instance, listener): void {
// noop
}
61 changes: 23 additions & 38 deletions packages/react-dom/src/client/ReactDOMHostConfig.js
Original file line number Diff line number Diff line change
Expand Up @@ -1102,52 +1102,37 @@ export function detachListenerFromInstance(listener: ReactDOMListener): void {
}
}

function validateListenerInstance(instance, methodString): boolean {
if (
instance &&
(instance === window || getClosestInstanceFromNode(instance))
) {
return true;
}
if (__DEV__) {
if (instance && (instance: any).nodeType === DOCUMENT_NODE) {
console.warn(
'Event listener method %s() from useEvent() hook requires the first argument to be a valid' +
' DOM node that was rendered and managed by React or a "window" object. It looks like' +
' you supplied a "document" node, instead use the "window" object.',
methodString,
);
} else {
console.warn(
'Event listener method %s() from useEvent() hook requires the first argument to be a valid' +
' DOM node that was rendered and managed by React or a "window" object. If this is' +
' from a ref, ensure the ref value has been set before attaching.',
methodString,
);
}
}
return false;
}

export function validateReactListenerDeleteListener(
export function validateReactListenerMapSetListener(
instance: EventTarget,
): boolean {
return validateListenerInstance(instance, 'deleteListener');
}

export function validateReactListenerMapListener(
instance: EventTarget,
listener: Event => void,
listener: ?(Event) => void,
): boolean {
if (enableListenerAPI) {
if (validateListenerInstance(instance, 'setListener')) {
if (typeof listener === 'function') {
if (
instance &&
(instance === window || getClosestInstanceFromNode(instance))
) {
if (listener == null || typeof listener === 'function') {
return true;
}
if (__DEV__) {
console.warn(
'Event listener method setListener() from useEvent() hook requires the second argument' +
' to be valid function callback.',
' to be either a valid function callback or null/undefined.',
);
}
}
if (__DEV__) {
if (instance && (instance: any).nodeType === DOCUMENT_NODE) {
console.warn(
'Event listener method setListener() from useEvent() hook requires the first argument to be a valid' +
' DOM node that was rendered and managed by React or a "window" object. It looks like' +
' you supplied a "document" node, instead use the "window" object.',
);
} else {
console.warn(
'Event listener method setListener() from useEvent() hook requires the first argument to be a valid' +
' DOM node that was rendered and managed by React or a "window" object. If this is' +
' from a ref, ensure the ref value has been set before attaching.',
);
}
}
Expand Down
1 change: 0 additions & 1 deletion packages/react-dom/src/server/ReactPartialRendererHooks.js
Original file line number Diff line number Diff line change
Expand Up @@ -476,7 +476,6 @@ function useTransition(
function useEvent(event: any): any {
return {
clear: noop,
deleteListener: noop,
setListener: noop,
};
}
Expand Down
6 changes: 1 addition & 5 deletions packages/react-native-renderer/src/ReactFabricHostConfig.js
Original file line number Diff line number Diff line change
Expand Up @@ -486,10 +486,6 @@ export function detachListenerFromInstance(instance, event, callback): any {
// noop
}

export function validateReactListenerDeleteListener(instance): void {
// noop
}

export function validateReactListenerMapListener(instance, listener): void {
export function validateReactListenerMapSetListener(instance, listener): void {
// noop
}
6 changes: 1 addition & 5 deletions packages/react-native-renderer/src/ReactNativeHostConfig.js
Original file line number Diff line number Diff line change
Expand Up @@ -537,10 +537,6 @@ export function detachListenerFromInstance(instance, event, callback): any {
// noop
}

export function validateReactListenerDeleteListener(instance): void {
// noop
}

export function validateReactListenerMapListener(instance, listener): void {
export function validateReactListenerMapSetListener(instance, listener): void {
// noop
}
27 changes: 11 additions & 16 deletions packages/react-reconciler/src/ReactFiberHooks.js
Original file line number Diff line number Diff line change
Expand Up @@ -65,8 +65,7 @@ import {
registerListenerEvent,
attachListenerToInstance,
detachListenerFromInstance,
validateReactListenerMapListener,
validateReactListenerDeleteListener,
validateReactListenerMapSetListener,
} from './ReactFiberHostConfig';
import {enableListenerAPI} from 'shared/ReactFeatureFlags';

Expand Down Expand Up @@ -1303,28 +1302,24 @@ export function mountEventListener(

const reactListenerMap: ReactListenerMap = {
clear,
deleteListener(instance: EventTarget): void {
setListener(instance: EventTarget, callback: ?(Event) => void): void {
if (
validateNotInFunctionRender() &&
validateReactListenerDeleteListener(instance)
) {
const listener = listenerMap.get(instance);
if (listener !== undefined) {
listenerMap.delete(instance);
detachListenerFromInstance(listener);
}
}
},
setListener(instance: EventTarget, callback: Event => void): void {
if (
validateNotInFunctionRender() &&
validateReactListenerMapListener(instance, callback)
validateReactListenerMapSetListener(instance, callback)
) {
let listener = listenerMap.get(instance);
if (listener === undefined) {
if (callback == null) {
return;
}
listener = createReactListener(event, callback, instance, destroy);
listenerMap.set(instance, listener);
} else {
if (callback == null) {
listenerMap.delete(instance);
detachListenerFromInstance(listener);
return;
}
listener.callback = callback;
}
attachListenerToInstance(listener);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -77,10 +77,8 @@ export const shouldUpdateFundamentalComponent =
export const getInstanceFromNode = $$$hostConfig.getInstanceFromNode;
export const beforeRemoveInstance = $$$hostConfig.beforeRemoveInstance;
export const registerListenerEvent = $$$hostConfig.registerListenerEvent;
export const validateReactListenerDeleteListener =
$$$hostConfig.validateReactListenerDeleteListener;
export const validateReactListenerMapListener =
$$$hostConfig.validateReactListenerMapListener;
export const validateReactListenerMapSetListener =
$$$hostConfig.validateReactListenerMapSetListener;

// -------------------
// Mutation
Expand Down
1 change: 0 additions & 1 deletion packages/react-test-renderer/src/ReactShallowRenderer.js
Original file line number Diff line number Diff line change
Expand Up @@ -398,7 +398,6 @@ class ReactShallowRenderer {
const useEvent = () => {
return {
clear: noOp,
deleteListener: noOp,
setListener: noOp,
};
};
Expand Down
6 changes: 1 addition & 5 deletions packages/react-test-renderer/src/ReactTestHostConfig.js
Original file line number Diff line number Diff line change
Expand Up @@ -390,10 +390,6 @@ export function detachListenerFromInstance(instance, event, callback): any {
// noop
}

export function validateReactListenerDeleteListener(instance): void {
// noop
}

export function validateReactListenerMapListener(instance, listener): void {
export function validateReactListenerMapSetListener(instance, listener): void {
// noop
}
3 changes: 1 addition & 2 deletions packages/shared/ReactDOMTypes.js
Original file line number Diff line number Diff line change
Expand Up @@ -85,8 +85,7 @@ export type ReactDOMListenerEvent = {|

export type ReactDOMListenerMap = {|
clear: () => void,
setListener: (instance: EventTarget, callback: (Event) => void) => void,
deleteListener: (instance: EventTarget) => void,
setListener: (instance: EventTarget, callback: ?(Event) => void) => void,
|};

export type ReactDOMListener = {|
Expand Down

0 comments on commit 87c7c7d

Please sign in to comment.