From b5b3a8757474c4f9dcd7466c81d2681875909455 Mon Sep 17 00:00:00 2001 From: Joey Arhar Date: Wed, 8 Dec 2021 18:44:02 -0800 Subject: [PATCH 01/25] Bypass react event system for custom elements --- .../__tests__/DOMPropertyOperations-test.js | 88 ++++++++++++++++++- .../src/client/DOMPropertyOperations.js | 9 ++ .../react-dom/src/client/ReactDOMComponent.js | 17 +++- .../src/events/DOMPluginEventSystem.js | 11 +++ 4 files changed, 121 insertions(+), 4 deletions(-) diff --git a/packages/react-dom/src/__tests__/DOMPropertyOperations-test.js b/packages/react-dom/src/__tests__/DOMPropertyOperations-test.js index 28776b1eb62..df6994cf051 100644 --- a/packages/react-dom/src/__tests__/DOMPropertyOperations-test.js +++ b/packages/react-dom/src/__tests__/DOMPropertyOperations-test.js @@ -256,6 +256,7 @@ describe('DOMPropertyOperations', () => { customElement.dispatchEvent(new Event('false')); }); + // @gate enableCustomElementPropertySupport it('custom elements should still have onClick treated like regular elements', () => { let syntheticClickEvent = null; const syntheticEventHandler = jest.fn( @@ -277,7 +278,92 @@ describe('DOMPropertyOperations', () => { expect(nativeEventHandler).toHaveBeenCalledTimes(1); expect(syntheticEventHandler).toHaveBeenCalledTimes(1); - expect(syntheticClickEvent.nativeEvent).toBe(nativeClickEvent); + expect(syntheticClickEvent).toBe(nativeClickEvent); + }); + + // @gate enableCustomElementPropertySupport + it('custom elements should have working onChange event listeners', () => { + let reactChangeEvent = null; + const eventHandler = jest.fn(event => (reactChangeEvent = event)); + const container = document.createElement('div'); + document.body.appendChild(container); + ReactDOM.render( + console.log('customevent')} + />, + container, + ); + + const customElement = container.querySelector('my-custom-element'); + const changeEvent = new Event('change'); + customElement.dispatchEvent(changeEvent); + + expect(eventHandler).toHaveBeenCalledTimes(1); + expect(reactChangeEvent).toBe(changeEvent); + + // Also make sure that removing and re-adding the event listener works + + ReactDOM.render(, container); + customElement.dispatchEvent(new Event('change')); + expect(eventHandler).toHaveBeenCalledTimes(1); + + ReactDOM.render(, container); + customElement.dispatchEvent(new Event('change')); + expect(eventHandler).toHaveBeenCalledTimes(2); + }); + + // @gate enableCustomElementPropertySupport + it('custom elements should have working onInput event listeners', () => { + let reactInputEvent = null; + const eventHandler = jest.fn(event => (reactInputEvent = event)); + const container = document.createElement('div'); + document.body.appendChild(container); + ReactDOM.render(, container); + + const customElement = container.querySelector('my-custom-element'); + const inputEvent = new Event('input'); + customElement.dispatchEvent(inputEvent); + + expect(eventHandler).toHaveBeenCalledTimes(1); + expect(reactInputEvent).toBe(inputEvent); + + // Also make sure that removing and re-adding the event listener works + + ReactDOM.render(, container); + customElement.dispatchEvent(new Event('input')); + expect(eventHandler).toHaveBeenCalledTimes(1); + + ReactDOM.render(, container); + customElement.dispatchEvent(new Event('input')); + expect(eventHandler).toHaveBeenCalledTimes(2); + }); + + // @gate enableCustomElementPropertySupport + it('custom elements should be able to remove and re-add custom event listeners', () => { + const container = document.createElement('div'); + document.body.appendChild(container); + const eventHandler = jest.fn(); + ReactDOM.render( + , + container, + ); + + const customElement = container.querySelector('my-custom-element'); + customElement.dispatchEvent(new Event('customevent')); + expect(eventHandler).toHaveBeenCalledTimes(1); + + ReactDOM.render(, container); + customElement.dispatchEvent(new Event('customevent')); + expect(eventHandler).toHaveBeenCalledTimes(1); + + ReactDOM.render( + , + container, + ); + customElement.dispatchEvent(new Event('customevent')); + expect(eventHandler).toHaveBeenCalledTimes(2); }); // @gate enableCustomElementPropertySupport diff --git a/packages/react-dom/src/client/DOMPropertyOperations.js b/packages/react-dom/src/client/DOMPropertyOperations.js index 71b53d703c6..7d5a5bc4123 100644 --- a/packages/react-dom/src/client/DOMPropertyOperations.js +++ b/packages/react-dom/src/client/DOMPropertyOperations.js @@ -158,8 +158,17 @@ export function setValueForProperty( name[0] === 'o' && name[1] === 'n' ) { + // Detect and remove trailing "Capture" let eventName = name.replace(/Capture$/, ''); const useCapture = name !== eventName; + + // Infer correct casing for DOM built-in events + // TODO maybe this should be pulled from a static list instead...? + if (eventName.toLowerCase() in node) { + eventName = eventName.toLowerCase(); + } + + // Remove the leading "on" eventName = eventName.slice(2); const prevProps = getFiberCurrentPropsFromNode(node); diff --git a/packages/react-dom/src/client/ReactDOMComponent.js b/packages/react-dom/src/client/ReactDOMComponent.js index ffd90c128d4..7345727030c 100644 --- a/packages/react-dom/src/client/ReactDOMComponent.js +++ b/packages/react-dom/src/client/ReactDOMComponent.js @@ -321,7 +321,10 @@ function setInitialDOMProperties( // We could have excluded it in the property list instead of // adding a special case here, but then it wouldn't be emitted // on server rendering (but we *do* want to emit it in SSR). - } else if (registrationNameDependencies.hasOwnProperty(propKey)) { + } else if ( + registrationNameDependencies.hasOwnProperty(propKey) && + (!enableCustomElementPropertySupport || !isCustomComponentTag) + ) { if (nextProp != null) { if (__DEV__ && typeof nextProp !== 'function') { warnForInvalidEventListener(propKey, nextProp); @@ -675,7 +678,11 @@ export function diffProperties( // Noop } else if (propKey === AUTOFOCUS) { // Noop. It doesn't work on updates anyway. - } else if (registrationNameDependencies.hasOwnProperty(propKey)) { + } else if ( + registrationNameDependencies.hasOwnProperty(propKey) && + (!enableCustomElementPropertySupport || + !isCustomComponent(domElement.tagName, lastRawProps)) + ) { // This is a special case. If any listener updates we need to ensure // that the "current" fiber pointer gets updated so we need a commit // to update this element. @@ -761,7 +768,11 @@ export function diffProperties( propKey === SUPPRESS_HYDRATION_WARNING ) { // Noop - } else if (registrationNameDependencies.hasOwnProperty(propKey)) { + } else if ( + registrationNameDependencies.hasOwnProperty(propKey) && + (!enableCustomElementPropertySupport || + !isCustomComponent(domElement.tagName, lastRawProps)) + ) { if (nextProp != null) { // We eagerly listen to this even though we haven't committed yet. if (__DEV__ && typeof nextProp !== 'function') { diff --git a/packages/react-dom/src/events/DOMPluginEventSystem.js b/packages/react-dom/src/events/DOMPluginEventSystem.js index e52b128c11d..e177335ffe1 100644 --- a/packages/react-dom/src/events/DOMPluginEventSystem.js +++ b/packages/react-dom/src/events/DOMPluginEventSystem.js @@ -52,6 +52,7 @@ import { enableLegacyFBSupport, enableCreateEventHandleAPI, enableScopeAPI, + enableCustomElementPropertySupport, } from 'shared/ReactFeatureFlags'; import { invokeGuardedCallbackAndCatchFirstError, @@ -71,6 +72,7 @@ import * as ChangeEventPlugin from './plugins/ChangeEventPlugin'; import * as EnterLeaveEventPlugin from './plugins/EnterLeaveEventPlugin'; import * as SelectEventPlugin from './plugins/SelectEventPlugin'; import * as SimpleEventPlugin from './plugins/SimpleEventPlugin'; +import isCustomComponent from '../shared/isCustomComponent'; type DispatchListener = {| instance: null | Fiber, @@ -540,6 +542,15 @@ export function dispatchEventForPluginEventSystem( targetInst: null | Fiber, targetContainer: EventTarget, ): void { + if ( + enableCustomElementPropertySupport && + targetInst && + isCustomComponent(targetInst.elementType, targetInst.pendingProps) + ) { + // Don't fire events on custom elements, they have a separate event system. + return; + } + let ancestorInst = targetInst; if ( (eventSystemFlags & IS_EVENT_HANDLE_NON_MANAGED_NODE) === 0 && From e2bbc6a62c052da26810f9c6996d31da3d225ec8 Mon Sep 17 00:00:00 2001 From: Joey Arhar Date: Tue, 14 Dec 2021 19:11:26 -0800 Subject: [PATCH 02/25] Going to try fixing react event system instead --- .../__tests__/DOMPropertyOperations-test.js | 19 +++------ .../src/client/DOMPropertyOperations.js | 8 ---- .../react-dom/src/client/ReactDOMComponent.js | 39 +++++++++++-------- .../src/events/DOMPluginEventSystem.js | 22 +++++------ 4 files changed, 39 insertions(+), 49 deletions(-) diff --git a/packages/react-dom/src/__tests__/DOMPropertyOperations-test.js b/packages/react-dom/src/__tests__/DOMPropertyOperations-test.js index df6994cf051..3bc55259c66 100644 --- a/packages/react-dom/src/__tests__/DOMPropertyOperations-test.js +++ b/packages/react-dom/src/__tests__/DOMPropertyOperations-test.js @@ -256,7 +256,6 @@ describe('DOMPropertyOperations', () => { customElement.dispatchEvent(new Event('false')); }); - // @gate enableCustomElementPropertySupport it('custom elements should still have onClick treated like regular elements', () => { let syntheticClickEvent = null; const syntheticEventHandler = jest.fn( @@ -278,7 +277,7 @@ describe('DOMPropertyOperations', () => { expect(nativeEventHandler).toHaveBeenCalledTimes(1); expect(syntheticEventHandler).toHaveBeenCalledTimes(1); - expect(syntheticClickEvent).toBe(nativeClickEvent); + expect(syntheticClickEvent.nativeEvent).toBe(nativeClickEvent); }); // @gate enableCustomElementPropertySupport @@ -287,21 +286,15 @@ describe('DOMPropertyOperations', () => { const eventHandler = jest.fn(event => (reactChangeEvent = event)); const container = document.createElement('div'); document.body.appendChild(container); - ReactDOM.render( - console.log('customevent')} - />, - container, - ); + ReactDOM.render(, container); const customElement = container.querySelector('my-custom-element'); - const changeEvent = new Event('change'); + // TODO const changeEvent = new Event('change'); + const changeEvent = new Event('change', {bubbles: true}); customElement.dispatchEvent(changeEvent); expect(eventHandler).toHaveBeenCalledTimes(1); - expect(reactChangeEvent).toBe(changeEvent); + expect(reactChangeEvent.nativeEvent).toBe(changeEvent); // Also make sure that removing and re-adding the event listener works @@ -327,7 +320,7 @@ describe('DOMPropertyOperations', () => { customElement.dispatchEvent(inputEvent); expect(eventHandler).toHaveBeenCalledTimes(1); - expect(reactInputEvent).toBe(inputEvent); + expect(reactInputEvent.nativeEvent).toBe(inputEvent); // Also make sure that removing and re-adding the event listener works diff --git a/packages/react-dom/src/client/DOMPropertyOperations.js b/packages/react-dom/src/client/DOMPropertyOperations.js index 7d5a5bc4123..4a5f3c7aeba 100644 --- a/packages/react-dom/src/client/DOMPropertyOperations.js +++ b/packages/react-dom/src/client/DOMPropertyOperations.js @@ -161,14 +161,6 @@ export function setValueForProperty( // Detect and remove trailing "Capture" let eventName = name.replace(/Capture$/, ''); const useCapture = name !== eventName; - - // Infer correct casing for DOM built-in events - // TODO maybe this should be pulled from a static list instead...? - if (eventName.toLowerCase() in node) { - eventName = eventName.toLowerCase(); - } - - // Remove the leading "on" eventName = eventName.slice(2); const prevProps = getFiberCurrentPropsFromNode(node); diff --git a/packages/react-dom/src/client/ReactDOMComponent.js b/packages/react-dom/src/client/ReactDOMComponent.js index 7345727030c..caaa5640231 100644 --- a/packages/react-dom/src/client/ReactDOMComponent.js +++ b/packages/react-dom/src/client/ReactDOMComponent.js @@ -76,6 +76,7 @@ import { import { mediaEventTypes, listenToNonDelegatedEvent, + listenToNonDelegatedEventForCustomElement, } from '../events/DOMPluginEventSystem'; let didWarnInvalidHydration = false; @@ -321,15 +322,18 @@ function setInitialDOMProperties( // We could have excluded it in the property list instead of // adding a special case here, but then it wouldn't be emitted // on server rendering (but we *do* want to emit it in SSR). - } else if ( - registrationNameDependencies.hasOwnProperty(propKey) && - (!enableCustomElementPropertySupport || !isCustomComponentTag) - ) { + } else if (registrationNameDependencies.hasOwnProperty(propKey)) { if (nextProp != null) { if (__DEV__ && typeof nextProp !== 'function') { warnForInvalidEventListener(propKey, nextProp); } - if (propKey === 'onScroll') { + if (enableCustomElementPropertySupport && isCustomComponentTag) { + console.log('doing the thing!' + + '\n registrationName...: ' + registrationNameDependencies[propKey]); + for (const eventName of registrationNameDependencies[propKey]) { + listenToNonDelegatedEventForCustomElement(eventName, domElement); + } + } else if (propKey === 'onScroll') { listenToNonDelegatedEvent('scroll', domElement); } } @@ -678,11 +682,7 @@ export function diffProperties( // Noop } else if (propKey === AUTOFOCUS) { // Noop. It doesn't work on updates anyway. - } else if ( - registrationNameDependencies.hasOwnProperty(propKey) && - (!enableCustomElementPropertySupport || - !isCustomComponent(domElement.tagName, lastRawProps)) - ) { + } else if (registrationNameDependencies.hasOwnProperty(propKey)) { // This is a special case. If any listener updates we need to ensure // that the "current" fiber pointer gets updated so we need a commit // to update this element. @@ -768,17 +768,18 @@ export function diffProperties( propKey === SUPPRESS_HYDRATION_WARNING ) { // Noop - } else if ( - registrationNameDependencies.hasOwnProperty(propKey) && - (!enableCustomElementPropertySupport || - !isCustomComponent(domElement.tagName, lastRawProps)) - ) { + } else if (registrationNameDependencies.hasOwnProperty(propKey)) { if (nextProp != null) { // We eagerly listen to this even though we haven't committed yet. if (__DEV__ && typeof nextProp !== 'function') { warnForInvalidEventListener(propKey, nextProp); } - if (propKey === 'onScroll') { + if (enableCustomElementPropertySupport && + isCustomComponent(domElement.tagName, lastRawProps)) { + for (const eventName of registrationNameDependencies[propKey]) { + listenToNonDelegatedEventForCustomElement(eventName, domElement); + } + } else if (propKey === 'onScroll') { listenToNonDelegatedEvent('scroll', domElement); } } @@ -1001,7 +1002,11 @@ export function diffHydratedProperties( if (__DEV__ && typeof nextProp !== 'function') { warnForInvalidEventListener(propKey, nextProp); } - if (propKey === 'onScroll') { + if (enableCustomElementPropertySupport && isCustomComponentTag) { + for (const eventName of registrationNameDependencies[propKey]) { + listenToNonDelegatedEventForCustomElement(eventName, domElement); + } + } else if (propKey === 'onScroll') { listenToNonDelegatedEvent('scroll', domElement); } } diff --git a/packages/react-dom/src/events/DOMPluginEventSystem.js b/packages/react-dom/src/events/DOMPluginEventSystem.js index e177335ffe1..91fc930eafa 100644 --- a/packages/react-dom/src/events/DOMPluginEventSystem.js +++ b/packages/react-dom/src/events/DOMPluginEventSystem.js @@ -52,7 +52,6 @@ import { enableLegacyFBSupport, enableCreateEventHandleAPI, enableScopeAPI, - enableCustomElementPropertySupport, } from 'shared/ReactFeatureFlags'; import { invokeGuardedCallbackAndCatchFirstError, @@ -72,7 +71,6 @@ import * as ChangeEventPlugin from './plugins/ChangeEventPlugin'; import * as EnterLeaveEventPlugin from './plugins/EnterLeaveEventPlugin'; import * as SelectEventPlugin from './plugins/SelectEventPlugin'; import * as SimpleEventPlugin from './plugins/SimpleEventPlugin'; -import isCustomComponent from '../shared/isCustomComponent'; type DispatchListener = {| instance: null | Fiber, @@ -308,6 +306,13 @@ export function listenToNonDelegatedEvent( ); } } + listenToNonDelegatedEventForCustomElement(domEventName, targetElement); +} + +export function listenToNonDelegatedEventForCustomElement( + domEventName: DOMEventName, + targetElement: Element, +): void { const isCapturePhaseListener = false; const listenerSet = getEventListenerSet(targetElement); const listenerSetKey = getListenerSetKey( @@ -420,6 +425,10 @@ function addTrappedEventListener( isCapturePhaseListener: boolean, isDeferredListenerForLegacyFBSupport?: boolean, ) { + /*console.log('addTrappedEventListener' + + '\n targetContainer: ' + targetContainer + + '\n domEventName: ' + domEventName + + '\n isCapturePhaseListener: ' + isCapturePhaseListener);*/ let listener = createEventListenerWrapperWithPriority( targetContainer, domEventName, @@ -542,15 +551,6 @@ export function dispatchEventForPluginEventSystem( targetInst: null | Fiber, targetContainer: EventTarget, ): void { - if ( - enableCustomElementPropertySupport && - targetInst && - isCustomComponent(targetInst.elementType, targetInst.pendingProps) - ) { - // Don't fire events on custom elements, they have a separate event system. - return; - } - let ancestorInst = targetInst; if ( (eventSystemFlags & IS_EVENT_HANDLE_NON_MANAGED_NODE) === 0 && From b194a40715f467a56ca9151a2477940a280b041a Mon Sep 17 00:00:00 2001 From: Joey Arhar Date: Wed, 15 Dec 2021 19:08:58 -0800 Subject: [PATCH 03/25] finally got it to call onChange, but perhaps too many times --- .../src/events/DOMEventProperties.js | 1 + .../src/events/DOMPluginEventSystem.js | 27 ++++++++++++++++--- .../src/events/plugins/ChangeEventPlugin.js | 10 +++++-- .../src/events/plugins/SimpleEventPlugin.js | 1 + 4 files changed, 34 insertions(+), 5 deletions(-) diff --git a/packages/react-dom/src/events/DOMEventProperties.js b/packages/react-dom/src/events/DOMEventProperties.js index a4ed6fcd575..461ab4fdf49 100644 --- a/packages/react-dom/src/events/DOMEventProperties.js +++ b/packages/react-dom/src/events/DOMEventProperties.js @@ -44,6 +44,7 @@ const simpleEventPluginEvents = [ 'contextMenu', 'copy', 'cut', + // no this makes everything blow up 'change', 'drag', 'dragEnd', 'dragEnter', diff --git a/packages/react-dom/src/events/DOMPluginEventSystem.js b/packages/react-dom/src/events/DOMPluginEventSystem.js index 91fc930eafa..a5e0bfda33f 100644 --- a/packages/react-dom/src/events/DOMPluginEventSystem.js +++ b/packages/react-dom/src/events/DOMPluginEventSystem.js @@ -52,6 +52,7 @@ import { enableLegacyFBSupport, enableCreateEventHandleAPI, enableScopeAPI, + enableCustomElementPropertySupport, } from 'shared/ReactFeatureFlags'; import { invokeGuardedCallbackAndCatchFirstError, @@ -71,6 +72,7 @@ import * as ChangeEventPlugin from './plugins/ChangeEventPlugin'; import * as EnterLeaveEventPlugin from './plugins/EnterLeaveEventPlugin'; import * as SelectEventPlugin from './plugins/SelectEventPlugin'; import * as SimpleEventPlugin from './plugins/SimpleEventPlugin'; +import isCustomComponent from '../shared/isCustomComponent'; type DispatchListener = {| instance: null | Fiber, @@ -135,7 +137,12 @@ function extractEvents( // could alter all these plugins to work in such ways, but // that might cause other unknown side-effects that we // can't foresee right now. - if (shouldProcessPolyfillPlugins) { + // i think this should be true for custom elements + let proceed = shouldProcessPolyfillPlugins; + if (enableCustomElementPropertySupport && targetInst) { + proceed = proceed || isCustomComponent(targetInst.elementType, targetInst.pendingProps); + } + if (proceed) { EnterLeaveEventPlugin.extractEvents( dispatchQueue, domEventName, @@ -306,7 +313,21 @@ export function listenToNonDelegatedEvent( ); } } - listenToNonDelegatedEventForCustomElement(domEventName, targetElement); + const isCapturePhaseListener = false; + const listenerSet = getEventListenerSet(targetElement); + const listenerSetKey = getListenerSetKey( + domEventName, + isCapturePhaseListener, + ); + if (!listenerSet.has(listenerSetKey)) { + addTrappedEventListener( + targetElement, + domEventName, + IS_NON_DELEGATED, + isCapturePhaseListener, + ); + listenerSet.add(listenerSetKey); + } } export function listenToNonDelegatedEventForCustomElement( @@ -323,7 +344,7 @@ export function listenToNonDelegatedEventForCustomElement( addTrappedEventListener( targetElement, domEventName, - IS_NON_DELEGATED, + IS_NON_DELEGATED & SHOULD_NOT_PROCESS_POLYFILL_EVENT_PLUGINS, isCapturePhaseListener, ); listenerSet.add(listenerSetKey); diff --git a/packages/react-dom/src/events/plugins/ChangeEventPlugin.js b/packages/react-dom/src/events/plugins/ChangeEventPlugin.js index 43d338a0cc5..2df511085ef 100644 --- a/packages/react-dom/src/events/plugins/ChangeEventPlugin.js +++ b/packages/react-dom/src/events/plugins/ChangeEventPlugin.js @@ -23,12 +23,13 @@ import {updateValueIfChanged} from '../../client/inputValueTracking'; import {setDefaultValue} from '../../client/ReactDOMInput'; import {enqueueStateRestore} from '../ReactDOMControlledComponent'; -import {disableInputAttributeSyncing} from 'shared/ReactFeatureFlags'; +import {disableInputAttributeSyncing, enableCustomElementPropertySupport} from 'shared/ReactFeatureFlags'; import {batchedUpdates} from '../ReactDOMUpdateBatching'; import { processDispatchQueue, accumulateTwoPhaseListeners, } from '../DOMPluginEventSystem'; +import isCustomComponent from '../../shared/isCustomComponent'; function registerEvents() { registerTwoPhaseEvent('onChange', [ @@ -280,8 +281,13 @@ function extractEvents( ) { const targetNode = targetInst ? getNodeFromInstance(targetInst) : window; + let isCustomComponentTag = false; + if (enableCustomElementPropertySupport && targetInst) { + isCustomComponentTag = isCustomComponent(targetInst.elementType, targetInst.pendingProps); + } + let getTargetInstFunc, handleEventFunc; - if (shouldUseChangeEvent(targetNode)) { + if (shouldUseChangeEvent(targetNode) || isCustomComponentTag) { getTargetInstFunc = getTargetInstForChangeEvent; } else if (isTextInputElement(((targetNode: any): HTMLElement))) { if (isInputEventSupported) { diff --git a/packages/react-dom/src/events/plugins/SimpleEventPlugin.js b/packages/react-dom/src/events/plugins/SimpleEventPlugin.js index 03bf9484b2c..806df35bd0f 100644 --- a/packages/react-dom/src/events/plugins/SimpleEventPlugin.js +++ b/packages/react-dom/src/events/plugins/SimpleEventPlugin.js @@ -60,6 +60,7 @@ function extractEvents( ): void { const reactName = topLevelEventsToReactNames.get(domEventName); if (reactName === undefined) { + // onChange handler isn't getting called because of this check? but also ChangeEventPlugin isn't getting called either.... return; } let SyntheticEventCtor = SyntheticEvent; From ace371d8d40a2068edc48144979667322bf68771 Mon Sep 17 00:00:00 2001 From: Joey Arhar Date: Wed, 15 Dec 2021 19:47:16 -0800 Subject: [PATCH 04/25] update test --- .../__tests__/DOMPropertyOperations-test.js | 23 +++++++++++-------- 1 file changed, 14 insertions(+), 9 deletions(-) diff --git a/packages/react-dom/src/__tests__/DOMPropertyOperations-test.js b/packages/react-dom/src/__tests__/DOMPropertyOperations-test.js index 3bc55259c66..e186d145bd7 100644 --- a/packages/react-dom/src/__tests__/DOMPropertyOperations-test.js +++ b/packages/react-dom/src/__tests__/DOMPropertyOperations-test.js @@ -287,24 +287,29 @@ describe('DOMPropertyOperations', () => { const container = document.createElement('div'); document.body.appendChild(container); ReactDOM.render(, container); - const customElement = container.querySelector('my-custom-element'); - // TODO const changeEvent = new Event('change'); - const changeEvent = new Event('change', {bubbles: true}); - customElement.dispatchEvent(changeEvent); - expect(eventHandler).toHaveBeenCalledTimes(1); + let expectedHandlerCallCount = 0; + const changeEvent = new Event('change'); + customElement.dispatchEvent(changeEvent); + expectedHandlerCallCount++; + expect(eventHandler).toHaveBeenCalledTimes(expectedHandlerCallCount); expect(reactChangeEvent.nativeEvent).toBe(changeEvent); // Also make sure that removing and re-adding the event listener works - ReactDOM.render(, container); customElement.dispatchEvent(new Event('change')); - expect(eventHandler).toHaveBeenCalledTimes(1); - + expect(eventHandler).toHaveBeenCalledTimes(expectedHandlerCallCount); ReactDOM.render(, container); customElement.dispatchEvent(new Event('change')); - expect(eventHandler).toHaveBeenCalledTimes(2); + expectedHandlerCallCount++; + expect(eventHandler).toHaveBeenCalledTimes(expectedHandlerCallCount); + + // Also make sure that when the event bubbles, the event handler is only called once + const changeEventBubble = new Event('change', {bubbles: true}); + customElement.dispatchEvent(changeEventBubble); + expectedHandlerCallCount++; + expect(eventHandler).toHaveBeenCalledTimes(expectedHandlerCallCount); }); // @gate enableCustomElementPropertySupport From 0f35e74692e8abf38ed3a7754b746d0f38af3b68 Mon Sep 17 00:00:00 2001 From: Joey Arhar Date: Wed, 15 Dec 2021 19:57:52 -0800 Subject: [PATCH 05/25] Removed ReactDOMComponent changes, now works but still doubles for bubbles --- .../react-dom/src/client/ReactDOMComponent.js | 21 +++---------------- 1 file changed, 3 insertions(+), 18 deletions(-) diff --git a/packages/react-dom/src/client/ReactDOMComponent.js b/packages/react-dom/src/client/ReactDOMComponent.js index caaa5640231..23c618fa70c 100644 --- a/packages/react-dom/src/client/ReactDOMComponent.js +++ b/packages/react-dom/src/client/ReactDOMComponent.js @@ -327,13 +327,7 @@ function setInitialDOMProperties( if (__DEV__ && typeof nextProp !== 'function') { warnForInvalidEventListener(propKey, nextProp); } - if (enableCustomElementPropertySupport && isCustomComponentTag) { - console.log('doing the thing!' - + '\n registrationName...: ' + registrationNameDependencies[propKey]); - for (const eventName of registrationNameDependencies[propKey]) { - listenToNonDelegatedEventForCustomElement(eventName, domElement); - } - } else if (propKey === 'onScroll') { + if (propKey === 'onScroll') { listenToNonDelegatedEvent('scroll', domElement); } } @@ -774,12 +768,7 @@ export function diffProperties( if (__DEV__ && typeof nextProp !== 'function') { warnForInvalidEventListener(propKey, nextProp); } - if (enableCustomElementPropertySupport && - isCustomComponent(domElement.tagName, lastRawProps)) { - for (const eventName of registrationNameDependencies[propKey]) { - listenToNonDelegatedEventForCustomElement(eventName, domElement); - } - } else if (propKey === 'onScroll') { + if (propKey === 'onScroll') { listenToNonDelegatedEvent('scroll', domElement); } } @@ -1002,11 +991,7 @@ export function diffHydratedProperties( if (__DEV__ && typeof nextProp !== 'function') { warnForInvalidEventListener(propKey, nextProp); } - if (enableCustomElementPropertySupport && isCustomComponentTag) { - for (const eventName of registrationNameDependencies[propKey]) { - listenToNonDelegatedEventForCustomElement(eventName, domElement); - } - } else if (propKey === 'onScroll') { + if (propKey === 'onScroll') { listenToNonDelegatedEvent('scroll', domElement); } } From 82f2ac421206048cde578c2c7d064b1bee6cc60b Mon Sep 17 00:00:00 2001 From: Joey Arhar Date: Thu, 23 Dec 2021 13:05:10 -0800 Subject: [PATCH 06/25] Maybe i should only support bubbling events --- .../__tests__/DOMPropertyOperations-test.js | 34 ++++++++----------- .../src/events/plugins/ChangeEventPlugin.js | 6 ++-- .../src/events/plugins/SimpleEventPlugin.js | 34 +++++++++++++++++-- 3 files changed, 48 insertions(+), 26 deletions(-) diff --git a/packages/react-dom/src/__tests__/DOMPropertyOperations-test.js b/packages/react-dom/src/__tests__/DOMPropertyOperations-test.js index e186d145bd7..2871a5ceeb6 100644 --- a/packages/react-dom/src/__tests__/DOMPropertyOperations-test.js +++ b/packages/react-dom/src/__tests__/DOMPropertyOperations-test.js @@ -288,9 +288,9 @@ describe('DOMPropertyOperations', () => { document.body.appendChild(container); ReactDOM.render(, container); const customElement = container.querySelector('my-custom-element'); - let expectedHandlerCallCount = 0; - const changeEvent = new Event('change'); + + const changeEvent = new Event('change', {bubbles: true}); customElement.dispatchEvent(changeEvent); expectedHandlerCallCount++; expect(eventHandler).toHaveBeenCalledTimes(expectedHandlerCallCount); @@ -298,16 +298,10 @@ describe('DOMPropertyOperations', () => { // Also make sure that removing and re-adding the event listener works ReactDOM.render(, container); - customElement.dispatchEvent(new Event('change')); + customElement.dispatchEvent(new Event('change', {bubbles: true})); expect(eventHandler).toHaveBeenCalledTimes(expectedHandlerCallCount); ReactDOM.render(, container); - customElement.dispatchEvent(new Event('change')); - expectedHandlerCallCount++; - expect(eventHandler).toHaveBeenCalledTimes(expectedHandlerCallCount); - - // Also make sure that when the event bubbles, the event handler is only called once - const changeEventBubble = new Event('change', {bubbles: true}); - customElement.dispatchEvent(changeEventBubble); + customElement.dispatchEvent(new Event('change', {bubbles: true})); expectedHandlerCallCount++; expect(eventHandler).toHaveBeenCalledTimes(expectedHandlerCallCount); }); @@ -319,23 +313,23 @@ describe('DOMPropertyOperations', () => { const container = document.createElement('div'); document.body.appendChild(container); ReactDOM.render(, container); - const customElement = container.querySelector('my-custom-element'); - const inputEvent = new Event('input'); - customElement.dispatchEvent(inputEvent); + let expectedHandlerCallCount = 0; - expect(eventHandler).toHaveBeenCalledTimes(1); + const inputEvent = new Event('input', {bubbles: true}); + customElement.dispatchEvent(inputEvent); + expectedHandlerCallCount++; + expect(eventHandler).toHaveBeenCalledTimes(expectedHandlerCallCount); expect(reactInputEvent.nativeEvent).toBe(inputEvent); // Also make sure that removing and re-adding the event listener works - ReactDOM.render(, container); - customElement.dispatchEvent(new Event('input')); - expect(eventHandler).toHaveBeenCalledTimes(1); - + customElement.dispatchEvent(new Event('input', {bubbles: true})); + expect(eventHandler).toHaveBeenCalledTimes(expectedHandlerCallCount); ReactDOM.render(, container); - customElement.dispatchEvent(new Event('input')); - expect(eventHandler).toHaveBeenCalledTimes(2); + customElement.dispatchEvent(new Event('input', {bubbles: true})); + expectedHandlerCallCount++; + expect(eventHandler).toHaveBeenCalledTimes(expectedHandlerCallCount); }); // @gate enableCustomElementPropertySupport diff --git a/packages/react-dom/src/events/plugins/ChangeEventPlugin.js b/packages/react-dom/src/events/plugins/ChangeEventPlugin.js index 2df511085ef..10f417bf2ad 100644 --- a/packages/react-dom/src/events/plugins/ChangeEventPlugin.js +++ b/packages/react-dom/src/events/plugins/ChangeEventPlugin.js @@ -282,12 +282,12 @@ function extractEvents( const targetNode = targetInst ? getNodeFromInstance(targetInst) : window; let isCustomComponentTag = false; - if (enableCustomElementPropertySupport && targetInst) { + /*if (enableCustomElementPropertySupport && targetInst) { isCustomComponentTag = isCustomComponent(targetInst.elementType, targetInst.pendingProps); - } + }*/ let getTargetInstFunc, handleEventFunc; - if (shouldUseChangeEvent(targetNode) || isCustomComponentTag) { + if (shouldUseChangeEvent(targetNode) /*|| isCustomComponentTag*/) { getTargetInstFunc = getTargetInstForChangeEvent; } else if (isTextInputElement(((targetNode: any): HTMLElement))) { if (isInputEventSupported) { diff --git a/packages/react-dom/src/events/plugins/SimpleEventPlugin.js b/packages/react-dom/src/events/plugins/SimpleEventPlugin.js index 806df35bd0f..3a80aa19f6b 100644 --- a/packages/react-dom/src/events/plugins/SimpleEventPlugin.js +++ b/packages/react-dom/src/events/plugins/SimpleEventPlugin.js @@ -26,6 +26,7 @@ import { SyntheticWheelEvent, SyntheticClipboardEvent, SyntheticPointerEvent, + SyntheticCustomElementEvent, } from '../../events/SyntheticEvent'; import { @@ -47,7 +48,20 @@ import {IS_EVENT_HANDLE_NON_MANAGED_NODE} from '../EventSystemFlags'; import getEventCharCode from '../getEventCharCode'; import {IS_CAPTURE_PHASE} from '../EventSystemFlags'; -import {enableCreateEventHandleAPI} from 'shared/ReactFeatureFlags'; +import { + enableCreateEventHandleAPI, + enableCustomElementPropertySupport, +} from 'shared/ReactFeatureFlags'; + +import isCustomComponent from '../../shared/isCustomComponent'; + +const customElementTopLevelEventsToReactNames: Map< + DOMEventName, + string | null, +> = new Map([ + ['change', 'onChange'], + //['input', 'onInput'], +]); function extractEvents( dispatchQueue: DispatchQueue, @@ -58,11 +72,23 @@ function extractEvents( eventSystemFlags: EventSystemFlags, targetContainer: EventTarget, ): void { - const reactName = topLevelEventsToReactNames.get(domEventName); + let debug = false; + let reactName = topLevelEventsToReactNames.get(domEventName); + if (reactName === undefined) { + if (enableCustomElementPropertySupport && targetInst && + isCustomComponent(targetInst.elementType, targetInst.pendingProps)) { + debug = true; + // This will fix it if the change event is targeting the custom element, + // but if a change event bubbles through a custom element that is + // targeted at something inside, then this won't work... + reactName = customElementTopLevelEventsToReactNames.get(domEventName); + console.log('set react name for custom element: ' + reactName); + } + } if (reactName === undefined) { - // onChange handler isn't getting called because of this check? but also ChangeEventPlugin isn't getting called either.... return; } + let SyntheticEventCtor = SyntheticEvent; let reactEventType: string = domEventName; switch (domEventName) { @@ -171,6 +197,7 @@ function extractEvents( targetContainer, inCapturePhase, ); + if (debug) console.log('listeners.length: ' + listeners.length); if (listeners.length > 0) { // Intentionally create event lazily. const event = new SyntheticEventCtor( @@ -203,6 +230,7 @@ function extractEvents( accumulateTargetOnly, nativeEvent, ); + if (debug) console.log('2listeners.length: ' + listeners.length); if (listeners.length > 0) { // Intentionally create event lazily. const event = new SyntheticEventCtor( From 2cbbce3914a18fe8336b9de719f568faabb0f785 Mon Sep 17 00:00:00 2001 From: Joey Arhar Date: Thu, 23 Dec 2021 17:45:29 -0800 Subject: [PATCH 07/25] removed some old stuff --- .../src/client/DOMPropertyOperations.js | 1 - .../react-dom/src/client/ReactDOMComponent.js | 1 - .../src/events/DOMEventProperties.js | 1 - .../src/events/DOMPluginEventSystem.js | 25 ------------------- .../src/events/plugins/SimpleEventPlugin.js | 17 +------------ 5 files changed, 1 insertion(+), 44 deletions(-) diff --git a/packages/react-dom/src/client/DOMPropertyOperations.js b/packages/react-dom/src/client/DOMPropertyOperations.js index 4a5f3c7aeba..71b53d703c6 100644 --- a/packages/react-dom/src/client/DOMPropertyOperations.js +++ b/packages/react-dom/src/client/DOMPropertyOperations.js @@ -158,7 +158,6 @@ export function setValueForProperty( name[0] === 'o' && name[1] === 'n' ) { - // Detect and remove trailing "Capture" let eventName = name.replace(/Capture$/, ''); const useCapture = name !== eventName; eventName = eventName.slice(2); diff --git a/packages/react-dom/src/client/ReactDOMComponent.js b/packages/react-dom/src/client/ReactDOMComponent.js index 23c618fa70c..ffd90c128d4 100644 --- a/packages/react-dom/src/client/ReactDOMComponent.js +++ b/packages/react-dom/src/client/ReactDOMComponent.js @@ -76,7 +76,6 @@ import { import { mediaEventTypes, listenToNonDelegatedEvent, - listenToNonDelegatedEventForCustomElement, } from '../events/DOMPluginEventSystem'; let didWarnInvalidHydration = false; diff --git a/packages/react-dom/src/events/DOMEventProperties.js b/packages/react-dom/src/events/DOMEventProperties.js index 461ab4fdf49..a4ed6fcd575 100644 --- a/packages/react-dom/src/events/DOMEventProperties.js +++ b/packages/react-dom/src/events/DOMEventProperties.js @@ -44,7 +44,6 @@ const simpleEventPluginEvents = [ 'contextMenu', 'copy', 'cut', - // no this makes everything blow up 'change', 'drag', 'dragEnd', 'dragEnter', diff --git a/packages/react-dom/src/events/DOMPluginEventSystem.js b/packages/react-dom/src/events/DOMPluginEventSystem.js index a5e0bfda33f..9ee2aa5d7b5 100644 --- a/packages/react-dom/src/events/DOMPluginEventSystem.js +++ b/packages/react-dom/src/events/DOMPluginEventSystem.js @@ -330,27 +330,6 @@ export function listenToNonDelegatedEvent( } } -export function listenToNonDelegatedEventForCustomElement( - domEventName: DOMEventName, - targetElement: Element, -): void { - const isCapturePhaseListener = false; - const listenerSet = getEventListenerSet(targetElement); - const listenerSetKey = getListenerSetKey( - domEventName, - isCapturePhaseListener, - ); - if (!listenerSet.has(listenerSetKey)) { - addTrappedEventListener( - targetElement, - domEventName, - IS_NON_DELEGATED & SHOULD_NOT_PROCESS_POLYFILL_EVENT_PLUGINS, - isCapturePhaseListener, - ); - listenerSet.add(listenerSetKey); - } -} - export function listenToNativeEvent( domEventName: DOMEventName, isCapturePhaseListener: boolean, @@ -446,10 +425,6 @@ function addTrappedEventListener( isCapturePhaseListener: boolean, isDeferredListenerForLegacyFBSupport?: boolean, ) { - /*console.log('addTrappedEventListener' - + '\n targetContainer: ' + targetContainer - + '\n domEventName: ' + domEventName - + '\n isCapturePhaseListener: ' + isCapturePhaseListener);*/ let listener = createEventListenerWrapperWithPriority( targetContainer, domEventName, diff --git a/packages/react-dom/src/events/plugins/SimpleEventPlugin.js b/packages/react-dom/src/events/plugins/SimpleEventPlugin.js index 3a80aa19f6b..0c06467cb8c 100644 --- a/packages/react-dom/src/events/plugins/SimpleEventPlugin.js +++ b/packages/react-dom/src/events/plugins/SimpleEventPlugin.js @@ -26,7 +26,6 @@ import { SyntheticWheelEvent, SyntheticClipboardEvent, SyntheticPointerEvent, - SyntheticCustomElementEvent, } from '../../events/SyntheticEvent'; import { @@ -55,14 +54,6 @@ import { import isCustomComponent from '../../shared/isCustomComponent'; -const customElementTopLevelEventsToReactNames: Map< - DOMEventName, - string | null, -> = new Map([ - ['change', 'onChange'], - //['input', 'onInput'], -]); - function extractEvents( dispatchQueue: DispatchQueue, domEventName: DOMEventName, @@ -72,23 +63,19 @@ function extractEvents( eventSystemFlags: EventSystemFlags, targetContainer: EventTarget, ): void { - let debug = false; let reactName = topLevelEventsToReactNames.get(domEventName); if (reactName === undefined) { - if (enableCustomElementPropertySupport && targetInst && + if (domEventName === 'change' && enableCustomElementPropertySupport && targetInst && isCustomComponent(targetInst.elementType, targetInst.pendingProps)) { - debug = true; // This will fix it if the change event is targeting the custom element, // but if a change event bubbles through a custom element that is // targeted at something inside, then this won't work... reactName = customElementTopLevelEventsToReactNames.get(domEventName); - console.log('set react name for custom element: ' + reactName); } } if (reactName === undefined) { return; } - let SyntheticEventCtor = SyntheticEvent; let reactEventType: string = domEventName; switch (domEventName) { @@ -197,7 +184,6 @@ function extractEvents( targetContainer, inCapturePhase, ); - if (debug) console.log('listeners.length: ' + listeners.length); if (listeners.length > 0) { // Intentionally create event lazily. const event = new SyntheticEventCtor( @@ -230,7 +216,6 @@ function extractEvents( accumulateTargetOnly, nativeEvent, ); - if (debug) console.log('2listeners.length: ' + listeners.length); if (listeners.length > 0) { // Intentionally create event lazily. const event = new SyntheticEventCtor( From 952398d7b2328e9aeacc241a816051289566c0fd Mon Sep 17 00:00:00 2001 From: Joey Arhar Date: Fri, 24 Dec 2021 18:22:57 -0800 Subject: [PATCH 08/25] cleaned up changeeventplugin stuff --- .../react-dom/src/events/DOMPluginEventSystem.js | 12 ++++++------ .../src/events/plugins/ChangeEventPlugin.js | 6 +++--- .../src/events/plugins/SimpleEventPlugin.js | 3 --- 3 files changed, 9 insertions(+), 12 deletions(-) diff --git a/packages/react-dom/src/events/DOMPluginEventSystem.js b/packages/react-dom/src/events/DOMPluginEventSystem.js index 9ee2aa5d7b5..e51c208099b 100644 --- a/packages/react-dom/src/events/DOMPluginEventSystem.js +++ b/packages/react-dom/src/events/DOMPluginEventSystem.js @@ -137,12 +137,7 @@ function extractEvents( // could alter all these plugins to work in such ways, but // that might cause other unknown side-effects that we // can't foresee right now. - // i think this should be true for custom elements - let proceed = shouldProcessPolyfillPlugins; - if (enableCustomElementPropertySupport && targetInst) { - proceed = proceed || isCustomComponent(targetInst.elementType, targetInst.pendingProps); - } - if (proceed) { + if (shouldProcessPolyfillPlugins) { EnterLeaveEventPlugin.extractEvents( dispatchQueue, domEventName, @@ -152,6 +147,9 @@ function extractEvents( eventSystemFlags, targetContainer, ); + } + if (shouldProcessPolyfillPlugins || + (enableCustomElementPropertySupport && targetInst && isCustomComponent(targetInst.elementType, targetInst.pendingProps))) { ChangeEventPlugin.extractEvents( dispatchQueue, domEventName, @@ -161,6 +159,8 @@ function extractEvents( eventSystemFlags, targetContainer, ); + } + if (shouldProcessPolyfillPlugins) { SelectEventPlugin.extractEvents( dispatchQueue, domEventName, diff --git a/packages/react-dom/src/events/plugins/ChangeEventPlugin.js b/packages/react-dom/src/events/plugins/ChangeEventPlugin.js index 10f417bf2ad..2df511085ef 100644 --- a/packages/react-dom/src/events/plugins/ChangeEventPlugin.js +++ b/packages/react-dom/src/events/plugins/ChangeEventPlugin.js @@ -282,12 +282,12 @@ function extractEvents( const targetNode = targetInst ? getNodeFromInstance(targetInst) : window; let isCustomComponentTag = false; - /*if (enableCustomElementPropertySupport && targetInst) { + if (enableCustomElementPropertySupport && targetInst) { isCustomComponentTag = isCustomComponent(targetInst.elementType, targetInst.pendingProps); - }*/ + } let getTargetInstFunc, handleEventFunc; - if (shouldUseChangeEvent(targetNode) /*|| isCustomComponentTag*/) { + if (shouldUseChangeEvent(targetNode) || isCustomComponentTag) { getTargetInstFunc = getTargetInstForChangeEvent; } else if (isTextInputElement(((targetNode: any): HTMLElement))) { if (isInputEventSupported) { diff --git a/packages/react-dom/src/events/plugins/SimpleEventPlugin.js b/packages/react-dom/src/events/plugins/SimpleEventPlugin.js index 0c06467cb8c..19a9960935c 100644 --- a/packages/react-dom/src/events/plugins/SimpleEventPlugin.js +++ b/packages/react-dom/src/events/plugins/SimpleEventPlugin.js @@ -67,9 +67,6 @@ function extractEvents( if (reactName === undefined) { if (domEventName === 'change' && enableCustomElementPropertySupport && targetInst && isCustomComponent(targetInst.elementType, targetInst.pendingProps)) { - // This will fix it if the change event is targeting the custom element, - // but if a change event bubbles through a custom element that is - // targeted at something inside, then this won't work... reactName = customElementTopLevelEventsToReactNames.get(domEventName); } } From eb315d70ad00e19270d009740fbb94f70b7f5907 Mon Sep 17 00:00:00 2001 From: Joey Arhar Date: Fri, 24 Dec 2021 18:35:28 -0800 Subject: [PATCH 09/25] prettier, lint --- packages/react-dom/src/events/DOMPluginEventSystem.js | 8 ++++++-- .../react-dom/src/events/plugins/ChangeEventPlugin.js | 10 ++++++++-- .../react-dom/src/events/plugins/SimpleEventPlugin.js | 10 +++++++--- 3 files changed, 21 insertions(+), 7 deletions(-) diff --git a/packages/react-dom/src/events/DOMPluginEventSystem.js b/packages/react-dom/src/events/DOMPluginEventSystem.js index e51c208099b..3f5469c533e 100644 --- a/packages/react-dom/src/events/DOMPluginEventSystem.js +++ b/packages/react-dom/src/events/DOMPluginEventSystem.js @@ -148,8 +148,12 @@ function extractEvents( targetContainer, ); } - if (shouldProcessPolyfillPlugins || - (enableCustomElementPropertySupport && targetInst && isCustomComponent(targetInst.elementType, targetInst.pendingProps))) { + if ( + shouldProcessPolyfillPlugins || + (enableCustomElementPropertySupport && + targetInst && + isCustomComponent(targetInst.elementType, targetInst.pendingProps)) + ) { ChangeEventPlugin.extractEvents( dispatchQueue, domEventName, diff --git a/packages/react-dom/src/events/plugins/ChangeEventPlugin.js b/packages/react-dom/src/events/plugins/ChangeEventPlugin.js index 2df511085ef..fbdfa995f86 100644 --- a/packages/react-dom/src/events/plugins/ChangeEventPlugin.js +++ b/packages/react-dom/src/events/plugins/ChangeEventPlugin.js @@ -23,7 +23,10 @@ import {updateValueIfChanged} from '../../client/inputValueTracking'; import {setDefaultValue} from '../../client/ReactDOMInput'; import {enqueueStateRestore} from '../ReactDOMControlledComponent'; -import {disableInputAttributeSyncing, enableCustomElementPropertySupport} from 'shared/ReactFeatureFlags'; +import { + disableInputAttributeSyncing, + enableCustomElementPropertySupport, +} from 'shared/ReactFeatureFlags'; import {batchedUpdates} from '../ReactDOMUpdateBatching'; import { processDispatchQueue, @@ -283,7 +286,10 @@ function extractEvents( let isCustomComponentTag = false; if (enableCustomElementPropertySupport && targetInst) { - isCustomComponentTag = isCustomComponent(targetInst.elementType, targetInst.pendingProps); + isCustomComponentTag = isCustomComponent( + targetInst.elementType, + targetInst.pendingProps, + ); } let getTargetInstFunc, handleEventFunc; diff --git a/packages/react-dom/src/events/plugins/SimpleEventPlugin.js b/packages/react-dom/src/events/plugins/SimpleEventPlugin.js index 19a9960935c..3b5144d23e9 100644 --- a/packages/react-dom/src/events/plugins/SimpleEventPlugin.js +++ b/packages/react-dom/src/events/plugins/SimpleEventPlugin.js @@ -65,9 +65,13 @@ function extractEvents( ): void { let reactName = topLevelEventsToReactNames.get(domEventName); if (reactName === undefined) { - if (domEventName === 'change' && enableCustomElementPropertySupport && targetInst && - isCustomComponent(targetInst.elementType, targetInst.pendingProps)) { - reactName = customElementTopLevelEventsToReactNames.get(domEventName); + if ( + domEventName === 'change' && + enableCustomElementPropertySupport && + targetInst && + isCustomComponent(targetInst.elementType, targetInst.pendingProps) + ) { + reactName = 'onChange'; } } if (reactName === undefined) { From 9c999022ecfb18979c040ac42bdb2234715ac981 Mon Sep 17 00:00:00 2001 From: Joey Arhar Date: Fri, 24 Dec 2021 18:37:41 -0800 Subject: [PATCH 10/25] removed changeeventplugin stuff --- .../react-dom/src/events/DOMPluginEventSystem.js | 11 ----------- .../src/events/plugins/ChangeEventPlugin.js | 16 ++-------------- 2 files changed, 2 insertions(+), 25 deletions(-) diff --git a/packages/react-dom/src/events/DOMPluginEventSystem.js b/packages/react-dom/src/events/DOMPluginEventSystem.js index 3f5469c533e..e52b128c11d 100644 --- a/packages/react-dom/src/events/DOMPluginEventSystem.js +++ b/packages/react-dom/src/events/DOMPluginEventSystem.js @@ -52,7 +52,6 @@ import { enableLegacyFBSupport, enableCreateEventHandleAPI, enableScopeAPI, - enableCustomElementPropertySupport, } from 'shared/ReactFeatureFlags'; import { invokeGuardedCallbackAndCatchFirstError, @@ -72,7 +71,6 @@ import * as ChangeEventPlugin from './plugins/ChangeEventPlugin'; import * as EnterLeaveEventPlugin from './plugins/EnterLeaveEventPlugin'; import * as SelectEventPlugin from './plugins/SelectEventPlugin'; import * as SimpleEventPlugin from './plugins/SimpleEventPlugin'; -import isCustomComponent from '../shared/isCustomComponent'; type DispatchListener = {| instance: null | Fiber, @@ -147,13 +145,6 @@ function extractEvents( eventSystemFlags, targetContainer, ); - } - if ( - shouldProcessPolyfillPlugins || - (enableCustomElementPropertySupport && - targetInst && - isCustomComponent(targetInst.elementType, targetInst.pendingProps)) - ) { ChangeEventPlugin.extractEvents( dispatchQueue, domEventName, @@ -163,8 +154,6 @@ function extractEvents( eventSystemFlags, targetContainer, ); - } - if (shouldProcessPolyfillPlugins) { SelectEventPlugin.extractEvents( dispatchQueue, domEventName, diff --git a/packages/react-dom/src/events/plugins/ChangeEventPlugin.js b/packages/react-dom/src/events/plugins/ChangeEventPlugin.js index fbdfa995f86..43d338a0cc5 100644 --- a/packages/react-dom/src/events/plugins/ChangeEventPlugin.js +++ b/packages/react-dom/src/events/plugins/ChangeEventPlugin.js @@ -23,16 +23,12 @@ import {updateValueIfChanged} from '../../client/inputValueTracking'; import {setDefaultValue} from '../../client/ReactDOMInput'; import {enqueueStateRestore} from '../ReactDOMControlledComponent'; -import { - disableInputAttributeSyncing, - enableCustomElementPropertySupport, -} from 'shared/ReactFeatureFlags'; +import {disableInputAttributeSyncing} from 'shared/ReactFeatureFlags'; import {batchedUpdates} from '../ReactDOMUpdateBatching'; import { processDispatchQueue, accumulateTwoPhaseListeners, } from '../DOMPluginEventSystem'; -import isCustomComponent from '../../shared/isCustomComponent'; function registerEvents() { registerTwoPhaseEvent('onChange', [ @@ -284,16 +280,8 @@ function extractEvents( ) { const targetNode = targetInst ? getNodeFromInstance(targetInst) : window; - let isCustomComponentTag = false; - if (enableCustomElementPropertySupport && targetInst) { - isCustomComponentTag = isCustomComponent( - targetInst.elementType, - targetInst.pendingProps, - ); - } - let getTargetInstFunc, handleEventFunc; - if (shouldUseChangeEvent(targetNode) || isCustomComponentTag) { + if (shouldUseChangeEvent(targetNode)) { getTargetInstFunc = getTargetInstForChangeEvent; } else if (isTextInputElement(((targetNode: any): HTMLElement))) { if (isInputEventSupported) { From 5d1e6da6c825ea449fa753d83194ab49be29330f Mon Sep 17 00:00:00 2001 From: Joey Arhar Date: Fri, 24 Dec 2021 18:50:10 -0800 Subject: [PATCH 11/25] remove unneeded gate for onInput test --- packages/react-dom/src/__tests__/DOMPropertyOperations-test.js | 1 - 1 file changed, 1 deletion(-) diff --git a/packages/react-dom/src/__tests__/DOMPropertyOperations-test.js b/packages/react-dom/src/__tests__/DOMPropertyOperations-test.js index 2871a5ceeb6..68f53761a2d 100644 --- a/packages/react-dom/src/__tests__/DOMPropertyOperations-test.js +++ b/packages/react-dom/src/__tests__/DOMPropertyOperations-test.js @@ -306,7 +306,6 @@ describe('DOMPropertyOperations', () => { expect(eventHandler).toHaveBeenCalledTimes(expectedHandlerCallCount); }); - // @gate enableCustomElementPropertySupport it('custom elements should have working onInput event listeners', () => { let reactInputEvent = null; const eventHandler = jest.fn(event => (reactInputEvent = event)); From a5db602266ccdf49ba6b9d495a1b8c435a7e6bc6 Mon Sep 17 00:00:00 2001 From: Joey Arhar Date: Wed, 12 Jan 2022 17:36:08 -0700 Subject: [PATCH 12/25] Go back to using ChangeEventPlugin --- .../src/events/DOMPluginEventSystem.js | 2 ++ .../src/events/plugins/ChangeEventPlugin.js | 16 ++++++++++++++-- .../src/events/plugins/SimpleEventPlugin.js | 19 ++----------------- 3 files changed, 18 insertions(+), 19 deletions(-) diff --git a/packages/react-dom/src/events/DOMPluginEventSystem.js b/packages/react-dom/src/events/DOMPluginEventSystem.js index e52b128c11d..408263460b6 100644 --- a/packages/react-dom/src/events/DOMPluginEventSystem.js +++ b/packages/react-dom/src/events/DOMPluginEventSystem.js @@ -52,6 +52,7 @@ import { enableLegacyFBSupport, enableCreateEventHandleAPI, enableScopeAPI, + enableCustomElementPropertySupport, } from 'shared/ReactFeatureFlags'; import { invokeGuardedCallbackAndCatchFirstError, @@ -71,6 +72,7 @@ import * as ChangeEventPlugin from './plugins/ChangeEventPlugin'; import * as EnterLeaveEventPlugin from './plugins/EnterLeaveEventPlugin'; import * as SelectEventPlugin from './plugins/SelectEventPlugin'; import * as SimpleEventPlugin from './plugins/SimpleEventPlugin'; +import isCustomComponent from '../shared/isCustomComponent'; type DispatchListener = {| instance: null | Fiber, diff --git a/packages/react-dom/src/events/plugins/ChangeEventPlugin.js b/packages/react-dom/src/events/plugins/ChangeEventPlugin.js index 43d338a0cc5..fbdfa995f86 100644 --- a/packages/react-dom/src/events/plugins/ChangeEventPlugin.js +++ b/packages/react-dom/src/events/plugins/ChangeEventPlugin.js @@ -23,12 +23,16 @@ import {updateValueIfChanged} from '../../client/inputValueTracking'; import {setDefaultValue} from '../../client/ReactDOMInput'; import {enqueueStateRestore} from '../ReactDOMControlledComponent'; -import {disableInputAttributeSyncing} from 'shared/ReactFeatureFlags'; +import { + disableInputAttributeSyncing, + enableCustomElementPropertySupport, +} from 'shared/ReactFeatureFlags'; import {batchedUpdates} from '../ReactDOMUpdateBatching'; import { processDispatchQueue, accumulateTwoPhaseListeners, } from '../DOMPluginEventSystem'; +import isCustomComponent from '../../shared/isCustomComponent'; function registerEvents() { registerTwoPhaseEvent('onChange', [ @@ -280,8 +284,16 @@ function extractEvents( ) { const targetNode = targetInst ? getNodeFromInstance(targetInst) : window; + let isCustomComponentTag = false; + if (enableCustomElementPropertySupport && targetInst) { + isCustomComponentTag = isCustomComponent( + targetInst.elementType, + targetInst.pendingProps, + ); + } + let getTargetInstFunc, handleEventFunc; - if (shouldUseChangeEvent(targetNode)) { + if (shouldUseChangeEvent(targetNode) || isCustomComponentTag) { getTargetInstFunc = getTargetInstForChangeEvent; } else if (isTextInputElement(((targetNode: any): HTMLElement))) { if (isInputEventSupported) { diff --git a/packages/react-dom/src/events/plugins/SimpleEventPlugin.js b/packages/react-dom/src/events/plugins/SimpleEventPlugin.js index 3b5144d23e9..03bf9484b2c 100644 --- a/packages/react-dom/src/events/plugins/SimpleEventPlugin.js +++ b/packages/react-dom/src/events/plugins/SimpleEventPlugin.js @@ -47,12 +47,7 @@ import {IS_EVENT_HANDLE_NON_MANAGED_NODE} from '../EventSystemFlags'; import getEventCharCode from '../getEventCharCode'; import {IS_CAPTURE_PHASE} from '../EventSystemFlags'; -import { - enableCreateEventHandleAPI, - enableCustomElementPropertySupport, -} from 'shared/ReactFeatureFlags'; - -import isCustomComponent from '../../shared/isCustomComponent'; +import {enableCreateEventHandleAPI} from 'shared/ReactFeatureFlags'; function extractEvents( dispatchQueue: DispatchQueue, @@ -63,17 +58,7 @@ function extractEvents( eventSystemFlags: EventSystemFlags, targetContainer: EventTarget, ): void { - let reactName = topLevelEventsToReactNames.get(domEventName); - if (reactName === undefined) { - if ( - domEventName === 'change' && - enableCustomElementPropertySupport && - targetInst && - isCustomComponent(targetInst.elementType, targetInst.pendingProps) - ) { - reactName = 'onChange'; - } - } + const reactName = topLevelEventsToReactNames.get(domEventName); if (reactName === undefined) { return; } From 9b77933a6d283472cc2dbaf2f061b26b0303d1e6 Mon Sep 17 00:00:00 2001 From: Joey Arhar Date: Wed, 12 Jan 2022 17:58:38 -0700 Subject: [PATCH 13/25] Add input+change test --- .../__tests__/DOMPropertyOperations-test.js | 20 +++++++++++++++++++ 1 file changed, 20 insertions(+) diff --git a/packages/react-dom/src/__tests__/DOMPropertyOperations-test.js b/packages/react-dom/src/__tests__/DOMPropertyOperations-test.js index 68f53761a2d..3631fb4c93d 100644 --- a/packages/react-dom/src/__tests__/DOMPropertyOperations-test.js +++ b/packages/react-dom/src/__tests__/DOMPropertyOperations-test.js @@ -331,6 +331,26 @@ describe('DOMPropertyOperations', () => { expect(eventHandler).toHaveBeenCalledTimes(expectedHandlerCallCount); }); + // @gate enableCustomElementPropertySupport + it('custom elements should have separate onInput and onChange handling', () => { + const container = document.createElement('div'); + document.body.appendChild(container); + const inputEventHandler = jest.fn(); + const changeEventHandler = jest.fn(); + ReactDOM.render(, container); + const customElement = container.querySelector('my-custom-element'); + + customElement.dispatchEvent(new Event('input', {bubbles: true})); + expect(inputEventHandler).toHaveBeenCalledTimes(1); + expect(changeEventHandler).toHaveBeenCalledTimes(0); + + customElement.dispatchEvent(new Event('change', {bubbles: true})); + expect(inputEventHandler).toHaveBeenCalledTimes(1); + expect(changeEventHandler).toHaveBeenCalledTimes(1); + }); + // @gate enableCustomElementPropertySupport it('custom elements should be able to remove and re-add custom event listeners', () => { const container = document.createElement('div'); From dd6c930c92eddeac76da06850fb84b51745035a1 Mon Sep 17 00:00:00 2001 From: Joey Arhar Date: Wed, 12 Jan 2022 18:05:09 -0700 Subject: [PATCH 14/25] lint --- .../src/__tests__/DOMPropertyOperations-test.js | 10 +++++++--- packages/react-dom/src/events/DOMPluginEventSystem.js | 2 -- 2 files changed, 7 insertions(+), 5 deletions(-) diff --git a/packages/react-dom/src/__tests__/DOMPropertyOperations-test.js b/packages/react-dom/src/__tests__/DOMPropertyOperations-test.js index 5c5d40d5f3f..5a2a5dcef2f 100644 --- a/packages/react-dom/src/__tests__/DOMPropertyOperations-test.js +++ b/packages/react-dom/src/__tests__/DOMPropertyOperations-test.js @@ -337,9 +337,13 @@ describe('DOMPropertyOperations', () => { document.body.appendChild(container); const inputEventHandler = jest.fn(); const changeEventHandler = jest.fn(); - ReactDOM.render(, container); + ReactDOM.render( + , + container, + ); const customElement = container.querySelector('my-custom-element'); customElement.dispatchEvent(new Event('input', {bubbles: true})); diff --git a/packages/react-dom/src/events/DOMPluginEventSystem.js b/packages/react-dom/src/events/DOMPluginEventSystem.js index 408263460b6..e52b128c11d 100644 --- a/packages/react-dom/src/events/DOMPluginEventSystem.js +++ b/packages/react-dom/src/events/DOMPluginEventSystem.js @@ -52,7 +52,6 @@ import { enableLegacyFBSupport, enableCreateEventHandleAPI, enableScopeAPI, - enableCustomElementPropertySupport, } from 'shared/ReactFeatureFlags'; import { invokeGuardedCallbackAndCatchFirstError, @@ -72,7 +71,6 @@ import * as ChangeEventPlugin from './plugins/ChangeEventPlugin'; import * as EnterLeaveEventPlugin from './plugins/EnterLeaveEventPlugin'; import * as SelectEventPlugin from './plugins/SelectEventPlugin'; import * as SimpleEventPlugin from './plugins/SimpleEventPlugin'; -import isCustomComponent from '../shared/isCustomComponent'; type DispatchListener = {| instance: null | Fiber, From c4e2dbe97914afe87c5abab17ed66f7450b22451 Mon Sep 17 00:00:00 2001 From: Joey Arhar Date: Thu, 13 Jan 2022 19:07:05 -0700 Subject: [PATCH 15/25] Move logic to shouldUseChangeEvent --- .../src/events/plugins/ChangeEventPlugin.js | 19 +++++++++---------- 1 file changed, 9 insertions(+), 10 deletions(-) diff --git a/packages/react-dom/src/events/plugins/ChangeEventPlugin.js b/packages/react-dom/src/events/plugins/ChangeEventPlugin.js index fbdfa995f86..7d79fbabe88 100644 --- a/packages/react-dom/src/events/plugins/ChangeEventPlugin.js +++ b/packages/react-dom/src/events/plugins/ChangeEventPlugin.js @@ -76,7 +76,14 @@ let activeElementInst = null; /** * SECTION: handle `change` event */ -function shouldUseChangeEvent(elem) { +function shouldUseChangeEvent(elem, targetInst) { + if ( + enableCustomElementPropertySupport && + targetInst && + isCustomComponent(targetInst.elementType, targetInst.pendingProps) + ) { + return true; + } const nodeName = elem.nodeName && elem.nodeName.toLowerCase(); return ( nodeName === 'select' || @@ -284,16 +291,8 @@ function extractEvents( ) { const targetNode = targetInst ? getNodeFromInstance(targetInst) : window; - let isCustomComponentTag = false; - if (enableCustomElementPropertySupport && targetInst) { - isCustomComponentTag = isCustomComponent( - targetInst.elementType, - targetInst.pendingProps, - ); - } - let getTargetInstFunc, handleEventFunc; - if (shouldUseChangeEvent(targetNode) || isCustomComponentTag) { + if (shouldUseChangeEvent(targetNode, targetInst)) { getTargetInstFunc = getTargetInstForChangeEvent; } else if (isTextInputElement(((targetNode: any): HTMLElement))) { if (isInputEventSupported) { From 741db5278d780294b873b4ec03fcc036ddd4c520 Mon Sep 17 00:00:00 2001 From: Joey Arhar Date: Fri, 14 Jan 2022 19:24:46 -0800 Subject: [PATCH 16/25] use memoizedProps instead of pendingProps --- packages/react-dom/src/events/plugins/ChangeEventPlugin.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/react-dom/src/events/plugins/ChangeEventPlugin.js b/packages/react-dom/src/events/plugins/ChangeEventPlugin.js index 7d79fbabe88..d6bf9ebb301 100644 --- a/packages/react-dom/src/events/plugins/ChangeEventPlugin.js +++ b/packages/react-dom/src/events/plugins/ChangeEventPlugin.js @@ -80,7 +80,7 @@ function shouldUseChangeEvent(elem, targetInst) { if ( enableCustomElementPropertySupport && targetInst && - isCustomComponent(targetInst.elementType, targetInst.pendingProps) + isCustomComponent(targetInst.elementType, targetInst.memoizedProps) ) { return true; } From 2d17c9746bb7452a437c51fb6f14ffa3de68184a Mon Sep 17 00:00:00 2001 From: Joey Arhar Date: Tue, 18 Jan 2022 09:38:13 -0800 Subject: [PATCH 17/25] Run form control behavior before custom element behavior --- .../__tests__/DOMPropertyOperations-test.js | 158 ++++++++++++++++++ .../src/events/plugins/ChangeEventPlugin.js | 17 +- 2 files changed, 166 insertions(+), 9 deletions(-) diff --git a/packages/react-dom/src/__tests__/DOMPropertyOperations-test.js b/packages/react-dom/src/__tests__/DOMPropertyOperations-test.js index 5a2a5dcef2f..36bbd22a42e 100644 --- a/packages/react-dom/src/__tests__/DOMPropertyOperations-test.js +++ b/packages/react-dom/src/__tests__/DOMPropertyOperations-test.js @@ -381,6 +381,164 @@ describe('DOMPropertyOperations', () => { expect(eventHandler).toHaveBeenCalledTimes(2); }); + it(' should have the same onChange/onInput/onClick behavior as ', () => { + const container = document.createElement('div'); + document.body.appendChild(container); + const regularOnInputHandler = jest.fn(); + const regularOnChangeHandler = jest.fn(); + const regularOnClickHandler = jest.fn(); + const customOnInputHandler = jest.fn(); + const customOnChangeHandler = jest.fn(); + const customOnClickHandler = jest.fn(); + ReactDOM.render( +
+ + +
, + container, + ); + + const regularInput = container.querySelector( + 'input:not([is=my-custom-element])', + ); + const customInput = container.querySelector( + 'input[is=my-custom-element]', + ); + expect(regularInput).not.toBe(customInput); + + regularInput.dispatchEvent(new Event('input', {bubbles: true})); + regularInput.dispatchEvent(new Event('change', {bubbles: true})); + regularInput.dispatchEvent(new Event('click', {bubbles: true})); + customInput.dispatchEvent(new Event('input', {bubbles: true})); + customInput.dispatchEvent(new Event('change', {bubbles: true})); + customInput.dispatchEvent(new Event('click', {bubbles: true})); + + expect(customOnInputHandler).toHaveBeenCalledTimes( + regularOnInputHandler.mock.calls.length, + ); + expect(customOnChangeHandler).toHaveBeenCalledTimes( + regularOnChangeHandler.mock.calls.length, + ); + expect(customOnClickHandler).toHaveBeenCalledTimes( + regularOnClickHandler.mock.calls.length, + ); + }); + + it(' should have the same onChange/onInput/onClick behavior as ', () => { + const container = document.createElement('div'); + document.body.appendChild(container); + const regularOnInputHandler = jest.fn(); + const regularOnChangeHandler = jest.fn(); + const regularOnClickHandler = jest.fn(); + const customOnInputHandler = jest.fn(); + const customOnChangeHandler = jest.fn(); + const customOnClickHandler = jest.fn(); + ReactDOM.render( +
+ + +
, + container, + ); + + const regularInput = container.querySelector( + 'input:not([is=my-custom-element])', + ); + const customInput = container.querySelector( + 'input[is=my-custom-element]', + ); + expect(regularInput).not.toBe(customInput); + + regularInput.dispatchEvent(new Event('input', {bubbles: true})); + regularInput.dispatchEvent(new Event('change', {bubbles: true})); + regularInput.dispatchEvent(new Event('click', {bubbles: true})); + customInput.dispatchEvent(new Event('input', {bubbles: true})); + customInput.dispatchEvent(new Event('change', {bubbles: true})); + customInput.dispatchEvent(new Event('click', {bubbles: true})); + + expect(customOnInputHandler).toHaveBeenCalledTimes( + regularOnInputHandler.mock.calls.length, + ); + expect(customOnChangeHandler).toHaveBeenCalledTimes( + regularOnChangeHandler.mock.calls.length, + ); + expect(customOnClickHandler).toHaveBeenCalledTimes( + regularOnClickHandler.mock.calls.length, + ); + }); + + it('', () => { + const container = document.createElement('div'); + document.body.appendChild(container); + const regularOnInputHandler = jest.fn(); + const regularOnChangeHandler = jest.fn(); + const regularOnClickHandler = jest.fn(); + const customOnInputHandler = jest.fn(); + const customOnChangeHandler = jest.fn(); + const customOnClickHandler = jest.fn(); + ReactDOM.render( +
+ +
, + container, + ); + + const regularSelect = container.querySelector( + 'select:not([is=my-custom-element])', + ); + const customSelect = container.querySelector( + 'select[is=my-custom-element]', + ); + expect(regularSelect).not.toBe(customSelect); + + regularSelect.dispatchEvent(new Event('input', {bubbles: true})); + regularSelect.dispatchEvent(new Event('change', {bubbles: true})); + regularSelect.dispatchEvent(new Event('click', {bubbles: true})); + customSelect.dispatchEvent(new Event('input', {bubbles: true})); + customSelect.dispatchEvent(new Event('change', {bubbles: true})); + customSelect.dispatchEvent(new Event('click', {bubbles: true})); + + expect(customOnInputHandler).toHaveBeenCalledTimes( + regularOnInputHandler.mock.calls.length, + ); + expect(customOnChangeHandler).toHaveBeenCalledTimes( + regularOnChangeHandler.mock.calls.length, + ); + expect(customOnClickHandler).toHaveBeenCalledTimes( + regularOnClickHandler.mock.calls.length, + ); + }); + // @gate enableCustomElementPropertySupport it('custom elements should allow custom events with capture event listeners', () => { const oncustomeventCapture = jest.fn(); diff --git a/packages/react-dom/src/events/plugins/ChangeEventPlugin.js b/packages/react-dom/src/events/plugins/ChangeEventPlugin.js index d6bf9ebb301..8985c50c71f 100644 --- a/packages/react-dom/src/events/plugins/ChangeEventPlugin.js +++ b/packages/react-dom/src/events/plugins/ChangeEventPlugin.js @@ -76,14 +76,7 @@ let activeElementInst = null; /** * SECTION: handle `change` event */ -function shouldUseChangeEvent(elem, targetInst) { - if ( - enableCustomElementPropertySupport && - targetInst && - isCustomComponent(targetInst.elementType, targetInst.memoizedProps) - ) { - return true; - } +function shouldUseChangeEvent(elem) { const nodeName = elem.nodeName && elem.nodeName.toLowerCase(); return ( nodeName === 'select' || @@ -292,7 +285,7 @@ function extractEvents( const targetNode = targetInst ? getNodeFromInstance(targetInst) : window; let getTargetInstFunc, handleEventFunc; - if (shouldUseChangeEvent(targetNode, targetInst)) { + if (shouldUseChangeEvent(targetNode)) { getTargetInstFunc = getTargetInstForChangeEvent; } else if (isTextInputElement(((targetNode: any): HTMLElement))) { if (isInputEventSupported) { @@ -303,6 +296,12 @@ function extractEvents( } } else if (shouldUseClickEvent(targetNode)) { getTargetInstFunc = getTargetInstForClickEvent; + } else if ( + enableCustomElementPropertySupport && + targetInst && + isCustomComponent(targetInst.elementType, targetInst.memoizedProps) + ) { + getTargetInstFunc = getTargetInstForChangeEvent; } if (getTargetInstFunc) { From 3f345c8398d622ebf961b866bfb4b1411b170500 Mon Sep 17 00:00:00 2001 From: Joey Arhar Date: Tue, 18 Jan 2022 11:00:11 -0800 Subject: [PATCH 18/25] add bubbling test --- .../__tests__/DOMPropertyOperations-test.js | 66 +++++++++++++++++++ 1 file changed, 66 insertions(+) diff --git a/packages/react-dom/src/__tests__/DOMPropertyOperations-test.js b/packages/react-dom/src/__tests__/DOMPropertyOperations-test.js index 36bbd22a42e..fa54124f221 100644 --- a/packages/react-dom/src/__tests__/DOMPropertyOperations-test.js +++ b/packages/react-dom/src/__tests__/DOMPropertyOperations-test.js @@ -539,6 +539,72 @@ describe('DOMPropertyOperations', () => { ); }); + // @gate enableCustomElementPropertySupport + it('onChange/onInput/onClick on div with various types of children', () => { + const container = document.createElement('div'); + const onChangeHandler = jest.fn(); + const onInputHandler = jest.fn(); + const onClickHandler = jest.fn(); + ReactDOM.render( +
+ + + +
, + container, + ); + const customElement = container.querySelector('my-custom-element'); + const regularInput = container.querySelector( + 'input:not([is="my-custom-element"])', + ); + const customInput = container.querySelector( + 'input[is="my-custom-element"]', + ); + expect(regularInput).not.toBe(customInput); + + customElement.dispatchEvent(new Event('change', {bubbles: true})); + expect(onChangeHandler).toBeCalledTimes(1); + expect(onInputHandler).toBeCalledTimes(0); + expect(onClickHandler).toBeCalledTimes(0); + customElement.dispatchEvent(new Event('input', {bubbles: true})); + expect(onChangeHandler).toBeCalledTimes(1); + expect(onInputHandler).toBeCalledTimes(1); + expect(onClickHandler).toBeCalledTimes(0); + customElement.dispatchEvent(new Event('click', {bubbles: true})); + expect(onChangeHandler).toBeCalledTimes(1); + expect(onInputHandler).toBeCalledTimes(1); + expect(onClickHandler).toBeCalledTimes(0); + + regularInput.dispatchEvent(new Event('change', {bubbles: true})); + expect(onChangeHandler).toBeCalledTimes(1); + expect(onInputHandler).toBeCalledTimes(1); + expect(onClickHandler).toBeCalledTimes(0); + regularInput.dispatchEvent(new Event('input', {bubbles: true})); + expect(onChangeHandler).toBeCalledTimes(1); + expect(onInputHandler).toBeCalledTimes(2); + expect(onClickHandler).toBeCalledTimes(0); + regularInput.dispatchEvent(new Event('click', {bubbles: true})); + expect(onChangeHandler).toBeCalledTimes(1); + expect(onInputHandler).toBeCalledTimes(2); + expect(onClickHandler).toBeCalledTimes(0); + + customInput.dispatchEvent(new Event('change', {bubbles: true})); + expect(onChangeHandler).toBeCalledTimes(1); + expect(onInputHandler).toBeCalledTimes(2); + expect(onClickHandler).toBeCalledTimes(0); + customInput.dispatchEvent(new Event('input', {bubbles: true})); + expect(onChangeHandler).toBeCalledTimes(1); + expect(onInputHandler).toBeCalledTimes(3); + expect(onClickHandler).toBeCalledTimes(0); + customInput.dispatchEvent(new Event('click', {bubbles: true})); + expect(onChangeHandler).toBeCalledTimes(1); + expect(onInputHandler).toBeCalledTimes(3); + expect(onClickHandler).toBeCalledTimes(0); + }); + // @gate enableCustomElementPropertySupport it('custom elements should allow custom events with capture event listeners', () => { const oncustomeventCapture = jest.fn(); From 41a9ad76fc238247e25ea8f84c04345c0ac3c66b Mon Sep 17 00:00:00 2001 From: Joey Arhar Date: Tue, 18 Jan 2022 11:06:37 -0800 Subject: [PATCH 19/25] forgot to append container to body --- .../src/__tests__/DOMPropertyOperations-test.js | 15 ++++++++------- 1 file changed, 8 insertions(+), 7 deletions(-) diff --git a/packages/react-dom/src/__tests__/DOMPropertyOperations-test.js b/packages/react-dom/src/__tests__/DOMPropertyOperations-test.js index fa54124f221..286c65b360a 100644 --- a/packages/react-dom/src/__tests__/DOMPropertyOperations-test.js +++ b/packages/react-dom/src/__tests__/DOMPropertyOperations-test.js @@ -542,6 +542,7 @@ describe('DOMPropertyOperations', () => { // @gate enableCustomElementPropertySupport it('onChange/onInput/onClick on div with various types of children', () => { const container = document.createElement('div'); + document.body.appendChild(container); const onChangeHandler = jest.fn(); const onInputHandler = jest.fn(); const onClickHandler = jest.fn(); @@ -576,33 +577,33 @@ describe('DOMPropertyOperations', () => { customElement.dispatchEvent(new Event('click', {bubbles: true})); expect(onChangeHandler).toBeCalledTimes(1); expect(onInputHandler).toBeCalledTimes(1); - expect(onClickHandler).toBeCalledTimes(0); + expect(onClickHandler).toBeCalledTimes(1); regularInput.dispatchEvent(new Event('change', {bubbles: true})); expect(onChangeHandler).toBeCalledTimes(1); expect(onInputHandler).toBeCalledTimes(1); - expect(onClickHandler).toBeCalledTimes(0); + expect(onClickHandler).toBeCalledTimes(1); regularInput.dispatchEvent(new Event('input', {bubbles: true})); expect(onChangeHandler).toBeCalledTimes(1); expect(onInputHandler).toBeCalledTimes(2); - expect(onClickHandler).toBeCalledTimes(0); + expect(onClickHandler).toBeCalledTimes(1); regularInput.dispatchEvent(new Event('click', {bubbles: true})); expect(onChangeHandler).toBeCalledTimes(1); expect(onInputHandler).toBeCalledTimes(2); - expect(onClickHandler).toBeCalledTimes(0); + expect(onClickHandler).toBeCalledTimes(2); customInput.dispatchEvent(new Event('change', {bubbles: true})); expect(onChangeHandler).toBeCalledTimes(1); expect(onInputHandler).toBeCalledTimes(2); - expect(onClickHandler).toBeCalledTimes(0); + expect(onClickHandler).toBeCalledTimes(2); customInput.dispatchEvent(new Event('input', {bubbles: true})); expect(onChangeHandler).toBeCalledTimes(1); expect(onInputHandler).toBeCalledTimes(3); - expect(onClickHandler).toBeCalledTimes(0); + expect(onClickHandler).toBeCalledTimes(2); customInput.dispatchEvent(new Event('click', {bubbles: true})); expect(onChangeHandler).toBeCalledTimes(1); expect(onInputHandler).toBeCalledTimes(3); - expect(onClickHandler).toBeCalledTimes(0); + expect(onClickHandler).toBeCalledTimes(3); }); // @gate enableCustomElementPropertySupport From 81d4517852f177c3df99584d330f5743cd150247 Mon Sep 17 00:00:00 2001 From: Joey Arhar Date: Tue, 18 Jan 2022 11:12:11 -0800 Subject: [PATCH 20/25] add child event target test --- .../__tests__/DOMPropertyOperations-test.js | 31 +++++++++++++++++++ 1 file changed, 31 insertions(+) diff --git a/packages/react-dom/src/__tests__/DOMPropertyOperations-test.js b/packages/react-dom/src/__tests__/DOMPropertyOperations-test.js index 286c65b360a..d2eecc37697 100644 --- a/packages/react-dom/src/__tests__/DOMPropertyOperations-test.js +++ b/packages/react-dom/src/__tests__/DOMPropertyOperations-test.js @@ -606,6 +606,37 @@ describe('DOMPropertyOperations', () => { expect(onClickHandler).toBeCalledTimes(3); }); + it('custom element with event target child onChange/onInput/onClick', () => { + const container = document.createElement('div'); + document.body.appendChild(container); + const onChangeHandler = jest.fn(); + const onInputHandler = jest.fn(); + const onClickHandler = jest.fn(); + ReactDOM.render( + +
+ , + container, + ); + + const div = container.querySelector('div'); + div.dispatchEvent(new Event('change', {bubbles: true})); + expect(onChangeHandler).toBeCalledTimes(0); + expect(onInputHandler).toBeCalledTimes(0); + expect(onClickHandler).toBeCalledTimes(0); + div.dispatchEvent(new Event('input', {bubbles: true})); + expect(onChangeHandler).toBeCalledTimes(0); + expect(onInputHandler).toBeCalledTimes(1); + expect(onClickHandler).toBeCalledTimes(0); + div.dispatchEvent(new Event('click', {bubbles: true})); + expect(onChangeHandler).toBeCalledTimes(0); + expect(onInputHandler).toBeCalledTimes(1); + expect(onClickHandler).toBeCalledTimes(1); + }); + // @gate enableCustomElementPropertySupport it('custom elements should allow custom events with capture event listeners', () => { const oncustomeventCapture = jest.fn(); From 69eb020c6a30ce0e48477287a04227ae43aa2bec Mon Sep 17 00:00:00 2001 From: Joey Arhar Date: Tue, 18 Jan 2022 12:35:13 -0800 Subject: [PATCH 21/25] expand test expectations --- .../__tests__/DOMPropertyOperations-test.js | 87 ++++++++++++------- 1 file changed, 57 insertions(+), 30 deletions(-) diff --git a/packages/react-dom/src/__tests__/DOMPropertyOperations-test.js b/packages/react-dom/src/__tests__/DOMPropertyOperations-test.js index d2eecc37697..d434befb60c 100644 --- a/packages/react-dom/src/__tests__/DOMPropertyOperations-test.js +++ b/packages/react-dom/src/__tests__/DOMPropertyOperations-test.js @@ -416,21 +416,30 @@ describe('DOMPropertyOperations', () => { expect(regularInput).not.toBe(customInput); regularInput.dispatchEvent(new Event('input', {bubbles: true})); + expect(regularOnInputHandler).toHaveBeenCalledTimes(1); + expect(regularOnChangeHandler).toHaveBeenCalledTimes(0); + expect(regularOnClickHandler).toHaveBeenCalledTimes(0); regularInput.dispatchEvent(new Event('change', {bubbles: true})); + expect(regularOnInputHandler).toHaveBeenCalledTimes(1); + expect(regularOnChangeHandler).toHaveBeenCalledTimes(0); + expect(regularOnClickHandler).toHaveBeenCalledTimes(0); regularInput.dispatchEvent(new Event('click', {bubbles: true})); + expect(regularOnInputHandler).toHaveBeenCalledTimes(1); + expect(regularOnChangeHandler).toHaveBeenCalledTimes(0); + expect(regularOnClickHandler).toHaveBeenCalledTimes(1); + customInput.dispatchEvent(new Event('input', {bubbles: true})); + expect(customOnInputHandler).toHaveBeenCalledTimes(1); + expect(customOnChangeHandler).toHaveBeenCalledTimes(0); + expect(customOnClickHandler).toHaveBeenCalledTimes(0); customInput.dispatchEvent(new Event('change', {bubbles: true})); + expect(customOnInputHandler).toHaveBeenCalledTimes(1); + expect(customOnChangeHandler).toHaveBeenCalledTimes(0); + expect(customOnClickHandler).toHaveBeenCalledTimes(0); customInput.dispatchEvent(new Event('click', {bubbles: true})); - - expect(customOnInputHandler).toHaveBeenCalledTimes( - regularOnInputHandler.mock.calls.length, - ); - expect(customOnChangeHandler).toHaveBeenCalledTimes( - regularOnChangeHandler.mock.calls.length, - ); - expect(customOnClickHandler).toHaveBeenCalledTimes( - regularOnClickHandler.mock.calls.length, - ); + expect(customOnInputHandler).toHaveBeenCalledTimes(1); + expect(customOnChangeHandler).toHaveBeenCalledTimes(0); + expect(customOnClickHandler).toHaveBeenCalledTimes(1); }); it(' should have the same onChange/onInput/onClick behavior as ', () => { @@ -470,21 +479,30 @@ describe('DOMPropertyOperations', () => { expect(regularInput).not.toBe(customInput); regularInput.dispatchEvent(new Event('input', {bubbles: true})); + expect(regularOnInputHandler).toHaveBeenCalledTimes(1); + expect(regularOnChangeHandler).toHaveBeenCalledTimes(0); + expect(regularOnClickHandler).toHaveBeenCalledTimes(0); regularInput.dispatchEvent(new Event('change', {bubbles: true})); + expect(regularOnInputHandler).toHaveBeenCalledTimes(1); + expect(regularOnChangeHandler).toHaveBeenCalledTimes(0); + expect(regularOnClickHandler).toHaveBeenCalledTimes(0); regularInput.dispatchEvent(new Event('click', {bubbles: true})); + expect(regularOnInputHandler).toHaveBeenCalledTimes(1); + expect(regularOnChangeHandler).toHaveBeenCalledTimes(0); + expect(regularOnClickHandler).toHaveBeenCalledTimes(1); + customInput.dispatchEvent(new Event('input', {bubbles: true})); + expect(customOnInputHandler).toHaveBeenCalledTimes(1); + expect(customOnChangeHandler).toHaveBeenCalledTimes(0); + expect(customOnClickHandler).toHaveBeenCalledTimes(0); customInput.dispatchEvent(new Event('change', {bubbles: true})); + expect(customOnInputHandler).toHaveBeenCalledTimes(1); + expect(customOnChangeHandler).toHaveBeenCalledTimes(0); + expect(customOnClickHandler).toHaveBeenCalledTimes(0); customInput.dispatchEvent(new Event('click', {bubbles: true})); - - expect(customOnInputHandler).toHaveBeenCalledTimes( - regularOnInputHandler.mock.calls.length, - ); - expect(customOnChangeHandler).toHaveBeenCalledTimes( - regularOnChangeHandler.mock.calls.length, - ); - expect(customOnClickHandler).toHaveBeenCalledTimes( - regularOnClickHandler.mock.calls.length, - ); + expect(customOnInputHandler).toHaveBeenCalledTimes(1); + expect(customOnChangeHandler).toHaveBeenCalledTimes(0); + expect(customOnClickHandler).toHaveBeenCalledTimes(1); }); it('', () => { @@ -522,21 +540,30 @@ describe('DOMPropertyOperations', () => { expect(regularSelect).not.toBe(customSelect); regularSelect.dispatchEvent(new Event('input', {bubbles: true})); + expect(regularOnInputHandler).toHaveBeenCalledTimes(1); + expect(regularOnChangeHandler).toHaveBeenCalledTimes(0); + expect(regularOnClickHandler).toHaveBeenCalledTimes(0); regularSelect.dispatchEvent(new Event('change', {bubbles: true})); + expect(regularOnInputHandler).toHaveBeenCalledTimes(1); + expect(regularOnChangeHandler).toHaveBeenCalledTimes(1); + expect(regularOnClickHandler).toHaveBeenCalledTimes(0); regularSelect.dispatchEvent(new Event('click', {bubbles: true})); + expect(regularOnInputHandler).toHaveBeenCalledTimes(1); + expect(regularOnChangeHandler).toHaveBeenCalledTimes(1); + expect(regularOnClickHandler).toHaveBeenCalledTimes(1); + customSelect.dispatchEvent(new Event('input', {bubbles: true})); + expect(customOnInputHandler).toHaveBeenCalledTimes(1); + expect(customOnChangeHandler).toHaveBeenCalledTimes(0); + expect(customOnClickHandler).toHaveBeenCalledTimes(0); customSelect.dispatchEvent(new Event('change', {bubbles: true})); + expect(customOnInputHandler).toHaveBeenCalledTimes(1); + expect(customOnChangeHandler).toHaveBeenCalledTimes(1); + expect(customOnClickHandler).toHaveBeenCalledTimes(0); customSelect.dispatchEvent(new Event('click', {bubbles: true})); - - expect(customOnInputHandler).toHaveBeenCalledTimes( - regularOnInputHandler.mock.calls.length, - ); - expect(customOnChangeHandler).toHaveBeenCalledTimes( - regularOnChangeHandler.mock.calls.length, - ); - expect(customOnClickHandler).toHaveBeenCalledTimes( - regularOnClickHandler.mock.calls.length, - ); + expect(customOnInputHandler).toHaveBeenCalledTimes(1); + expect(customOnChangeHandler).toHaveBeenCalledTimes(1); + expect(customOnClickHandler).toHaveBeenCalledTimes(1); }); // @gate enableCustomElementPropertySupport From 6eaf85ce8d0e917218b5d0e48e5e0db64137d64a Mon Sep 17 00:00:00 2001 From: Dan Abramov Date: Wed, 19 Jan 2022 19:12:19 +0000 Subject: [PATCH 22/25] Make tests more realistic --- .../__tests__/DOMPropertyOperations-test.js | 263 +++++++++++++----- 1 file changed, 194 insertions(+), 69 deletions(-) diff --git a/packages/react-dom/src/__tests__/DOMPropertyOperations-test.js b/packages/react-dom/src/__tests__/DOMPropertyOperations-test.js index d434befb60c..314fdd6b3cc 100644 --- a/packages/react-dom/src/__tests__/DOMPropertyOperations-test.js +++ b/packages/react-dom/src/__tests__/DOMPropertyOperations-test.js @@ -22,6 +22,17 @@ describe('DOMPropertyOperations', () => { ReactDOM = require('react-dom'); }); + // Sets a value in a way that React doesn't see, + // so that a subsequent "change" event will trigger the event handler. + const setUntrackedValue = Object.getOwnPropertyDescriptor( + HTMLInputElement.prototype, + 'value', + ).set; + const setUntrackedChecked = Object.getOwnPropertyDescriptor( + HTMLInputElement.prototype, + 'checked', + ).set; + describe('setValueForProperty', () => { it('should set values as properties by default', () => { const container = document.createElement('div'); @@ -390,6 +401,14 @@ describe('DOMPropertyOperations', () => { const customOnInputHandler = jest.fn(); const customOnChangeHandler = jest.fn(); const customOnClickHandler = jest.fn(); + function clearMocks() { + regularOnInputHandler.mockClear(); + regularOnChangeHandler.mockClear(); + regularOnClickHandler.mockClear(); + customOnInputHandler.mockClear(); + customOnChangeHandler.mockClear(); + customOnClickHandler.mockClear(); + } ReactDOM.render(
{ ); expect(regularInput).not.toBe(customInput); + // Typing should trigger onInput and onChange for both kinds of inputs. + clearMocks(); + setUntrackedValue.call(regularInput, 'hello'); regularInput.dispatchEvent(new Event('input', {bubbles: true})); expect(regularOnInputHandler).toHaveBeenCalledTimes(1); - expect(regularOnChangeHandler).toHaveBeenCalledTimes(0); + expect(regularOnChangeHandler).toHaveBeenCalledTimes(1); expect(regularOnClickHandler).toHaveBeenCalledTimes(0); + setUntrackedValue.call(customInput, 'hello'); + customInput.dispatchEvent(new Event('input', {bubbles: true})); + expect(customOnInputHandler).toHaveBeenCalledTimes(1); + expect(customOnChangeHandler).toHaveBeenCalledTimes(1); + expect(customOnClickHandler).toHaveBeenCalledTimes(0); + + // The native change event itself does not produce extra React events. + clearMocks(); regularInput.dispatchEvent(new Event('change', {bubbles: true})); - expect(regularOnInputHandler).toHaveBeenCalledTimes(1); + expect(regularOnInputHandler).toHaveBeenCalledTimes(0); expect(regularOnChangeHandler).toHaveBeenCalledTimes(0); expect(regularOnClickHandler).toHaveBeenCalledTimes(0); + customInput.dispatchEvent(new Event('change', {bubbles: true})); + expect(customOnInputHandler).toHaveBeenCalledTimes(0); + expect(customOnChangeHandler).toHaveBeenCalledTimes(0); + expect(customOnClickHandler).toHaveBeenCalledTimes(0); + + // The click event is handled by both inputs. + clearMocks(); regularInput.dispatchEvent(new Event('click', {bubbles: true})); - expect(regularOnInputHandler).toHaveBeenCalledTimes(1); + expect(regularOnInputHandler).toHaveBeenCalledTimes(0); expect(regularOnChangeHandler).toHaveBeenCalledTimes(0); expect(regularOnClickHandler).toHaveBeenCalledTimes(1); + customInput.dispatchEvent(new Event('click', {bubbles: true})); + expect(customOnInputHandler).toHaveBeenCalledTimes(0); + expect(customOnChangeHandler).toHaveBeenCalledTimes(0); + expect(customOnClickHandler).toHaveBeenCalledTimes(1); + // Typing again should trigger onInput and onChange for both kinds of inputs. + clearMocks(); + setUntrackedValue.call(regularInput, 'goodbye'); + regularInput.dispatchEvent(new Event('input', {bubbles: true})); + expect(regularOnInputHandler).toHaveBeenCalledTimes(1); + expect(regularOnChangeHandler).toHaveBeenCalledTimes(1); + expect(regularOnClickHandler).toHaveBeenCalledTimes(0); + setUntrackedValue.call(customInput, 'goodbye'); customInput.dispatchEvent(new Event('input', {bubbles: true})); expect(customOnInputHandler).toHaveBeenCalledTimes(1); - expect(customOnChangeHandler).toHaveBeenCalledTimes(0); - expect(customOnClickHandler).toHaveBeenCalledTimes(0); - customInput.dispatchEvent(new Event('change', {bubbles: true})); - expect(customOnInputHandler).toHaveBeenCalledTimes(1); - expect(customOnChangeHandler).toHaveBeenCalledTimes(0); + expect(customOnChangeHandler).toHaveBeenCalledTimes(1); expect(customOnClickHandler).toHaveBeenCalledTimes(0); - customInput.dispatchEvent(new Event('click', {bubbles: true})); - expect(customOnInputHandler).toHaveBeenCalledTimes(1); - expect(customOnChangeHandler).toHaveBeenCalledTimes(0); - expect(customOnClickHandler).toHaveBeenCalledTimes(1); }); it(' should have the same onChange/onInput/onClick behavior as ', () => { @@ -451,6 +492,14 @@ describe('DOMPropertyOperations', () => { const customOnInputHandler = jest.fn(); const customOnChangeHandler = jest.fn(); const customOnClickHandler = jest.fn(); + function clearMocks() { + regularOnInputHandler.mockClear(); + regularOnChangeHandler.mockClear(); + regularOnClickHandler.mockClear(); + customOnInputHandler.mockClear(); + customOnChangeHandler.mockClear(); + customOnClickHandler.mockClear(); + } ReactDOM.render(
{ ); expect(regularInput).not.toBe(customInput); + // Clicking should trigger onClick and onChange on both inputs. + clearMocks(); + setUntrackedChecked.call(regularInput, true); + regularInput.dispatchEvent(new Event('click', {bubbles: true})); + expect(regularOnInputHandler).toHaveBeenCalledTimes(0); + expect(regularOnChangeHandler).toHaveBeenCalledTimes(1); + expect(regularOnClickHandler).toHaveBeenCalledTimes(1); + setUntrackedChecked.call(customInput, true); + customInput.dispatchEvent(new Event('click', {bubbles: true})); + expect(customOnInputHandler).toHaveBeenCalledTimes(0); + expect(customOnChangeHandler).toHaveBeenCalledTimes(1); + expect(customOnClickHandler).toHaveBeenCalledTimes(1); + + // The native input event only produces a React onInput event. + clearMocks(); regularInput.dispatchEvent(new Event('input', {bubbles: true})); expect(regularOnInputHandler).toHaveBeenCalledTimes(1); expect(regularOnChangeHandler).toHaveBeenCalledTimes(0); expect(regularOnClickHandler).toHaveBeenCalledTimes(0); - regularInput.dispatchEvent(new Event('change', {bubbles: true})); - expect(regularOnInputHandler).toHaveBeenCalledTimes(1); - expect(regularOnChangeHandler).toHaveBeenCalledTimes(0); - expect(regularOnClickHandler).toHaveBeenCalledTimes(0); - regularInput.dispatchEvent(new Event('click', {bubbles: true})); - expect(regularOnInputHandler).toHaveBeenCalledTimes(1); - expect(regularOnChangeHandler).toHaveBeenCalledTimes(0); - expect(regularOnClickHandler).toHaveBeenCalledTimes(1); - customInput.dispatchEvent(new Event('input', {bubbles: true})); expect(customOnInputHandler).toHaveBeenCalledTimes(1); expect(customOnChangeHandler).toHaveBeenCalledTimes(0); expect(customOnClickHandler).toHaveBeenCalledTimes(0); - customInput.dispatchEvent(new Event('change', {bubbles: true})); - expect(customOnInputHandler).toHaveBeenCalledTimes(1); - expect(customOnChangeHandler).toHaveBeenCalledTimes(0); - expect(customOnClickHandler).toHaveBeenCalledTimes(0); + + // Clicking again should trigger onClick and onChange on both inputs. + clearMocks(); + setUntrackedChecked.call(regularInput, false); + regularInput.dispatchEvent(new Event('click', {bubbles: true})); + expect(regularOnInputHandler).toHaveBeenCalledTimes(0); + expect(regularOnChangeHandler).toHaveBeenCalledTimes(1); + expect(regularOnClickHandler).toHaveBeenCalledTimes(1); + setUntrackedChecked.call(customInput, false); customInput.dispatchEvent(new Event('click', {bubbles: true})); - expect(customOnInputHandler).toHaveBeenCalledTimes(1); - expect(customOnChangeHandler).toHaveBeenCalledTimes(0); + expect(customOnInputHandler).toHaveBeenCalledTimes(0); + expect(customOnChangeHandler).toHaveBeenCalledTimes(1); expect(customOnClickHandler).toHaveBeenCalledTimes(1); }); @@ -514,6 +574,14 @@ describe('DOMPropertyOperations', () => { const customOnInputHandler = jest.fn(); const customOnChangeHandler = jest.fn(); const customOnClickHandler = jest.fn(); + function clearMocks() { + regularOnInputHandler.mockClear(); + regularOnChangeHandler.mockClear(); + regularOnClickHandler.mockClear(); + customOnInputHandler.mockClear(); + customOnChangeHandler.mockClear(); + customOnClickHandler.mockClear(); + } ReactDOM.render(
, container, ); - const div = container.querySelector('div'); - div.dispatchEvent(new Event('change', {bubbles: true})); - expect(onChangeHandler).toBeCalledTimes(0); - expect(onInputHandler).toBeCalledTimes(0); + const input = container.querySelector('input'); + setUntrackedValue.call(input, 'hello'); + input.dispatchEvent(new Event('input', {bubbles: true})); + // Simulated onChange from the child's input event + // bubbles to the parent custom element. + expect(onChangeHandler).toBeCalledTimes(1); + expect(onInputHandler).toBeCalledTimes(1); expect(onClickHandler).toBeCalledTimes(0); - div.dispatchEvent(new Event('input', {bubbles: true})); - expect(onChangeHandler).toBeCalledTimes(0); + // Consequently, the native change event is ignored. + input.dispatchEvent(new Event('change', {bubbles: true})); + expect(onChangeHandler).toBeCalledTimes(1); expect(onInputHandler).toBeCalledTimes(1); expect(onClickHandler).toBeCalledTimes(0); - div.dispatchEvent(new Event('click', {bubbles: true})); + input.dispatchEvent(new Event('click', {bubbles: true})); + expect(onChangeHandler).toBeCalledTimes(1); + expect(onInputHandler).toBeCalledTimes(1); + expect(onClickHandler).toBeCalledTimes(1); + }); + + it('custom element onChange/onInput/onClick with event target custom element child', () => { + const container = document.createElement('div'); + document.body.appendChild(container); + const onChangeHandler = jest.fn(); + const onInputHandler = jest.fn(); + const onClickHandler = jest.fn(); + ReactDOM.render( + + + , + container, + ); + + const customChild = container.querySelector('other-custom-element'); + customChild.dispatchEvent(new Event('input', {bubbles: true})); + // There is no simulated onChange, only raw onInput is dispatched. expect(onChangeHandler).toBeCalledTimes(0); expect(onInputHandler).toBeCalledTimes(1); + expect(onClickHandler).toBeCalledTimes(0); + // The native change event propagates to the parent as onChange. + customChild.dispatchEvent(new Event('change', {bubbles: true})); + expect(onChangeHandler).toBeCalledTimes(1); + expect(onInputHandler).toBeCalledTimes(1); + expect(onClickHandler).toBeCalledTimes(0); + customChild.dispatchEvent(new Event('click', {bubbles: true})); + expect(onChangeHandler).toBeCalledTimes(1); + expect(onInputHandler).toBeCalledTimes(1); expect(onClickHandler).toBeCalledTimes(1); }); From c58a742dd085d351c572a85acb62a777592f2d7e Mon Sep 17 00:00:00 2001 From: Dan Abramov Date: Wed, 19 Jan 2022 19:19:32 +0000 Subject: [PATCH 23/25] Add extra test --- .../__tests__/DOMPropertyOperations-test.js | 70 +++++++++++++++++++ 1 file changed, 70 insertions(+) diff --git a/packages/react-dom/src/__tests__/DOMPropertyOperations-test.js b/packages/react-dom/src/__tests__/DOMPropertyOperations-test.js index 314fdd6b3cc..8d27f0ce340 100644 --- a/packages/react-dom/src/__tests__/DOMPropertyOperations-test.js +++ b/packages/react-dom/src/__tests__/DOMPropertyOperations-test.js @@ -756,6 +756,76 @@ describe('DOMPropertyOperations', () => { expect(onClickHandler).toBeCalledTimes(1); }); + it('custom element onChange/onInput/onClick with event target div child', () => { + const container = document.createElement('div'); + document.body.appendChild(container); + const onChangeHandler = jest.fn(); + const onInputHandler = jest.fn(); + const onClickHandler = jest.fn(); + ReactDOM.render( + +
+ , + container, + ); + + const div = container.querySelector('div'); + div.dispatchEvent(new Event('input', {bubbles: true})); + expect(onChangeHandler).toBeCalledTimes(0); + expect(onInputHandler).toBeCalledTimes(1); + expect(onClickHandler).toBeCalledTimes(0); + + div.dispatchEvent(new Event('change', {bubbles: true})); + // React always ignores change event invoked on non-custom and non-input targets. + // So change event emitted on a div does not propagate upwards. + expect(onChangeHandler).toBeCalledTimes(0); + expect(onInputHandler).toBeCalledTimes(1); + expect(onClickHandler).toBeCalledTimes(0); + + div.dispatchEvent(new Event('click', {bubbles: true})); + expect(onChangeHandler).toBeCalledTimes(0); + expect(onInputHandler).toBeCalledTimes(1); + expect(onClickHandler).toBeCalledTimes(1); + }); + + it('div onChange/onInput/onClick with event target div child', () => { + const container = document.createElement('div'); + document.body.appendChild(container); + const onChangeHandler = jest.fn(); + const onInputHandler = jest.fn(); + const onClickHandler = jest.fn(); + ReactDOM.render( +
+
+
, + container, + ); + + const div = container.querySelector('div > div'); + div.dispatchEvent(new Event('input', {bubbles: true})); + expect(onChangeHandler).toBeCalledTimes(0); + expect(onInputHandler).toBeCalledTimes(1); + expect(onClickHandler).toBeCalledTimes(0); + + div.dispatchEvent(new Event('change', {bubbles: true})); + // React always ignores change event invoked on non-custom and non-input targets. + // So change event emitted on a div does not propagate upwards. + expect(onChangeHandler).toBeCalledTimes(0); + expect(onInputHandler).toBeCalledTimes(1); + expect(onClickHandler).toBeCalledTimes(0); + + div.dispatchEvent(new Event('click', {bubbles: true})); + expect(onChangeHandler).toBeCalledTimes(0); + expect(onInputHandler).toBeCalledTimes(1); + expect(onClickHandler).toBeCalledTimes(1); + }); + it('custom element onChange/onInput/onClick with event target custom element child', () => { const container = document.createElement('div'); document.body.appendChild(container); From 2ab52dfd1b28e188f6533d4ca1b8eb681ca20b42 Mon Sep 17 00:00:00 2001 From: Dan Abramov Date: Wed, 19 Jan 2022 19:21:19 +0000 Subject: [PATCH 24/25] Add missing gating --- packages/react-dom/src/__tests__/DOMPropertyOperations-test.js | 2 ++ 1 file changed, 2 insertions(+) diff --git a/packages/react-dom/src/__tests__/DOMPropertyOperations-test.js b/packages/react-dom/src/__tests__/DOMPropertyOperations-test.js index 8d27f0ce340..3b7fc75681b 100644 --- a/packages/react-dom/src/__tests__/DOMPropertyOperations-test.js +++ b/packages/react-dom/src/__tests__/DOMPropertyOperations-test.js @@ -721,6 +721,7 @@ describe('DOMPropertyOperations', () => { expect(onClickHandler).toBeCalledTimes(1); }); + // @gate enableCustomElementPropertySupport it('custom element onChange/onInput/onClick with event target input child', () => { const container = document.createElement('div'); document.body.appendChild(container); @@ -756,6 +757,7 @@ describe('DOMPropertyOperations', () => { expect(onClickHandler).toBeCalledTimes(1); }); + // @gate enableCustomElementPropertySupport it('custom element onChange/onInput/onClick with event target div child', () => { const container = document.createElement('div'); document.body.appendChild(container); From 93c9db1890431d2b0ada18dd240559a886f8817d Mon Sep 17 00:00:00 2001 From: Dan Abramov Date: Wed, 19 Jan 2022 19:24:18 +0000 Subject: [PATCH 25/25] Actually fix gating --- packages/react-dom/src/__tests__/DOMPropertyOperations-test.js | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/packages/react-dom/src/__tests__/DOMPropertyOperations-test.js b/packages/react-dom/src/__tests__/DOMPropertyOperations-test.js index 3b7fc75681b..ab5ca9a5495 100644 --- a/packages/react-dom/src/__tests__/DOMPropertyOperations-test.js +++ b/packages/react-dom/src/__tests__/DOMPropertyOperations-test.js @@ -721,7 +721,6 @@ describe('DOMPropertyOperations', () => { expect(onClickHandler).toBeCalledTimes(1); }); - // @gate enableCustomElementPropertySupport it('custom element onChange/onInput/onClick with event target input child', () => { const container = document.createElement('div'); document.body.appendChild(container); @@ -757,7 +756,6 @@ describe('DOMPropertyOperations', () => { expect(onClickHandler).toBeCalledTimes(1); }); - // @gate enableCustomElementPropertySupport it('custom element onChange/onInput/onClick with event target div child', () => { const container = document.createElement('div'); document.body.appendChild(container); @@ -828,6 +826,7 @@ describe('DOMPropertyOperations', () => { expect(onClickHandler).toBeCalledTimes(1); }); + // @gate enableCustomElementPropertySupport it('custom element onChange/onInput/onClick with event target custom element child', () => { const container = document.createElement('div'); document.body.appendChild(container);