Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Keep onTouchStart, onTouchMove, and onWheel passive #19654

Merged
merged 2 commits into from
Aug 19, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
20 changes: 18 additions & 2 deletions packages/react-dom/src/events/DOMPluginEventSystem.js
Original file line number Diff line number Diff line change
Expand Up @@ -53,6 +53,7 @@ import {
enableLegacyFBSupport,
enableCreateEventHandleAPI,
enableScopeAPI,
enablePassiveEventIntervention,
} from 'shared/ReactFeatureFlags';
import {
invokeGuardedCallbackAndCatchFirstError,
Expand Down Expand Up @@ -342,6 +343,21 @@ export function listenToNativeEvent(
if (domEventName === 'selectionchange') {
target = (rootContainerElement: any).ownerDocument;
}
if (enablePassiveEventIntervention && isPassiveListener === undefined) {
// Browsers introduced an intervention, making these events
// passive by default on document. React doesn't bind them
// to document anymore, but changing this now would undo
// the performance wins from the change. So we emulate
// the existing behavior manually on the roots now.
// https://github.com/facebook/react/issues/19651
if (
domEventName === 'touchstart' ||
domEventName === 'touchmove' ||
domEventName === 'wheel'
) {
isPassiveListener = true;
}
}
// If the event can be delegated (or is capture phase), we can
// register it to the root container. Otherwise, we should
// register the event to the target element and mark it as
Expand Down Expand Up @@ -506,7 +522,7 @@ function addTrappedEventListener(
};
}
if (isCapturePhaseListener) {
if (enableCreateEventHandleAPI && isPassiveListener !== undefined) {
if (isPassiveListener !== undefined) {
unsubscribeListener = addEventCaptureListenerWithPassiveFlag(
targetContainer,
domEventName,
Expand All @@ -521,7 +537,7 @@ function addTrappedEventListener(
);
}
} else {
if (enableCreateEventHandleAPI && isPassiveListener !== undefined) {
if (isPassiveListener !== undefined) {
unsubscribeListener = addEventBubbleListenerWithPassiveFlag(
targetContainer,
domEventName,
Expand Down
3 changes: 1 addition & 2 deletions packages/react-dom/src/events/checkPassiveEvents.js
Original file line number Diff line number Diff line change
Expand Up @@ -8,13 +8,12 @@
*/

import {canUseDOM} from 'shared/ExecutionEnvironment';
import {enableCreateEventHandleAPI} from 'shared/ReactFeatureFlags';

export let passiveBrowserEventsSupported = false;

// Check if browser support events with passive listeners
// https://developer.mozilla.org/en-US/docs/Web/API/EventTarget/addEventListener#Safely_detecting_option_support
if (enableCreateEventHandleAPI && canUseDOM) {
if (canUseDOM) {
try {
const options = {};
// $FlowFixMe: Ignore Flow complaining about needing a value
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -504,5 +504,44 @@ describe('SimpleEventPlugin', function() {

expect(onClick).toHaveBeenCalledTimes(0);
});

it('registers passive handlers for events affected by the intervention', () => {
container = document.createElement('div');

const passiveEvents = [];
const nativeAddEventListener = container.addEventListener;
container.addEventListener = function(type, fn, options) {
if (options !== null && typeof options === 'object') {
if (options.passive) {
passiveEvents.push(type);
}
}
return nativeAddEventListener.apply(this, arguments);
};

ReactDOM.render(
<div
// Affected by the intervention:
// https://github.com/facebook/react/issues/19651
onTouchStart={() => {}}
onTouchMove={() => {}}
onWheel={() => {}}
// A few events that should be unaffected:
onClick={() => {}}
onScroll={() => {}}
onTouchEnd={() => {}}
onChange={() => {}}
onPointerDown={() => {}}
onPointerMove={() => {}}
/>,
container,
);

if (gate(flags => flags.enablePassiveEventIntervention)) {
expect(passiveEvents).toEqual(['touchstart', 'touchmove', 'wheel']);
} else {
expect(passiveEvents).toEqual([]);
}
});
});
});
3 changes: 3 additions & 0 deletions packages/shared/ReactFeatureFlags.js
Original file line number Diff line number Diff line change
Expand Up @@ -132,3 +132,6 @@ export const deferRenderPhaseUpdateToNextBatch = true;
export const decoupleUpdatePriorityFromScheduler = false;

export const enableDiscreteEventFlushingChange = false;

// https://github.com/facebook/react/pull/19654
export const enablePassiveEventIntervention = true;
1 change: 1 addition & 0 deletions packages/shared/forks/ReactFeatureFlags.native-fb.js
Original file line number Diff line number Diff line change
Expand Up @@ -49,6 +49,7 @@ export const enableNewReconciler = false;
export const deferRenderPhaseUpdateToNextBatch = true;
export const decoupleUpdatePriorityFromScheduler = false;
export const enableDiscreteEventFlushingChange = false;
export const enablePassiveEventIntervention = true;

// Flow magic to verify the exports of this file match the original version.
// eslint-disable-next-line no-unused-vars
Expand Down
1 change: 1 addition & 0 deletions packages/shared/forks/ReactFeatureFlags.native-oss.js
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,7 @@ export const enableNewReconciler = false;
export const deferRenderPhaseUpdateToNextBatch = true;
export const decoupleUpdatePriorityFromScheduler = false;
export const enableDiscreteEventFlushingChange = false;
export const enablePassiveEventIntervention = true;

// Flow magic to verify the exports of this file match the original version.
// eslint-disable-next-line no-unused-vars
Expand Down
1 change: 1 addition & 0 deletions packages/shared/forks/ReactFeatureFlags.test-renderer.js
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,7 @@ export const enableNewReconciler = false;
export const deferRenderPhaseUpdateToNextBatch = true;
export const decoupleUpdatePriorityFromScheduler = false;
export const enableDiscreteEventFlushingChange = false;
export const enablePassiveEventIntervention = true;

// Flow magic to verify the exports of this file match the original version.
// eslint-disable-next-line no-unused-vars
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,7 @@ export const enableNewReconciler = false;
export const deferRenderPhaseUpdateToNextBatch = true;
export const decoupleUpdatePriorityFromScheduler = false;
export const enableDiscreteEventFlushingChange = false;
export const enablePassiveEventIntervention = true;

// Flow magic to verify the exports of this file match the original version.
// eslint-disable-next-line no-unused-vars
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,7 @@ export const enableNewReconciler = false;
export const deferRenderPhaseUpdateToNextBatch = true;
export const decoupleUpdatePriorityFromScheduler = false;
export const enableDiscreteEventFlushingChange = false;
export const enablePassiveEventIntervention = true;

// Flow magic to verify the exports of this file match the original version.
// eslint-disable-next-line no-unused-vars
Expand Down
1 change: 1 addition & 0 deletions packages/shared/forks/ReactFeatureFlags.testing.js
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,7 @@ export const enableNewReconciler = false;
export const deferRenderPhaseUpdateToNextBatch = true;
export const decoupleUpdatePriorityFromScheduler = false;
export const enableDiscreteEventFlushingChange = false;
export const enablePassiveEventIntervention = true;

// Flow magic to verify the exports of this file match the original version.
// eslint-disable-next-line no-unused-vars
Expand Down
1 change: 1 addition & 0 deletions packages/shared/forks/ReactFeatureFlags.testing.www.js
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,7 @@ export const enableNewReconciler = false;
export const deferRenderPhaseUpdateToNextBatch = true;
export const decoupleUpdatePriorityFromScheduler = false;
export const enableDiscreteEventFlushingChange = true;
export const enablePassiveEventIntervention = true;

// Flow magic to verify the exports of this file match the original version.
// eslint-disable-next-line no-unused-vars
Expand Down
1 change: 1 addition & 0 deletions packages/shared/forks/ReactFeatureFlags.www-dynamic.js
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ export const enableFilterEmptyStringAttributesDOM = __VARIANT__;
export const enableLegacyFBSupport = __VARIANT__;
export const decoupleUpdatePriorityFromScheduler = __VARIANT__;
export const skipUnmountedBoundaries = __VARIANT__;
export const enablePassiveEventIntervention = __VARIANT__;

// Enable this flag to help with concurrent mode debugging.
// It logs information to the console about React scheduling, rendering, and commit phases.
Expand Down
1 change: 1 addition & 0 deletions packages/shared/forks/ReactFeatureFlags.www.js
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@ export const {
decoupleUpdatePriorityFromScheduler,
enableDebugTracing,
skipUnmountedBoundaries,
enablePassiveEventIntervention,
} = dynamicFeatureFlags;

// On WWW, __EXPERIMENTAL__ is used for a new modern build.
Expand Down