Skip to content

Commit

Permalink
[react-events] Ensure updateEventListeners updates in commit phase (#…
Browse files Browse the repository at this point in the history
  • Loading branch information
trueadm authored Aug 22, 2019
1 parent 0f6e3cd commit fc80772
Show file tree
Hide file tree
Showing 10 changed files with 261 additions and 249 deletions.
1 change: 0 additions & 1 deletion packages/react-art/src/ReactARTHostConfig.js
Original file line number Diff line number Diff line change
Expand Up @@ -432,7 +432,6 @@ export function mountResponderInstance(
props: Object,
state: Object,
instance: Object,
rootContainerInstance: Object,
) {
throw new Error('Not yet implemented.');
}
Expand Down
3 changes: 1 addition & 2 deletions packages/react-dom/src/client/ReactDOMHostConfig.js
Original file line number Diff line number Diff line change
Expand Up @@ -824,10 +824,9 @@ export function mountResponderInstance(
responderProps: Object,
responderState: Object,
instance: Instance,
rootContainerInstance: Container,
): ReactDOMEventResponderInstance {
// Listen to events
const doc = rootContainerInstance.ownerDocument;
const doc = instance.ownerDocument;
const documentBody = doc.body || doc;
const {
rootEventTypes,
Expand Down
14 changes: 9 additions & 5 deletions packages/react-dom/src/events/DOMEventResponderSystem.js
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ import {
PASSIVE_NOT_SUPPORTED,
} from 'legacy-events/EventSystemFlags';
import type {AnyNativeEvent} from 'legacy-events/PluginModuleType';
import {HostComponent} from 'shared/ReactWorkTags';
import {HostComponent, SuspenseComponent} from 'shared/ReactWorkTags';
import type {EventPriority} from 'shared/ReactTypes';
import type {
ReactDOMEventResponder,
Expand All @@ -32,10 +32,6 @@ import type {Fiber} from 'react-reconciler/src/ReactFiber';
import warning from 'shared/warning';
import {enableFlareAPI} from 'shared/ReactFeatureFlags';
import invariant from 'shared/invariant';
import {
isFiberSuspenseAndTimedOut,
getSuspenseFallbackChild,
} from 'react-reconciler/src/ReactFiberEvents';

import {getClosestInstanceFromNode} from '../client/ReactDOMComponentTree';
import {
Expand Down Expand Up @@ -630,6 +626,14 @@ function validateResponderContext(): void {
);
}

function isFiberSuspenseAndTimedOut(fiber: Fiber): boolean {
return fiber.tag === SuspenseComponent && fiber.memoizedState !== null;
}

function getSuspenseFallbackChild(fiber: Fiber): Fiber | null {
return ((((fiber.child: any): Fiber).sibling: any): Fiber).child;
}

export function dispatchEventForResponderEventSystem(
topLevelType: string,
targetFiber: null | Fiber,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ let ReactFeatureFlags;
let ReactDOM;
let ReactDOMServer;
let ReactTestRenderer;
let Scheduler;

// FIXME: What should the public API be for setting an event's priority? Right
// now it's an enum but is that what we want? Hard coding this for now.
Expand Down Expand Up @@ -72,6 +73,7 @@ describe('DOMEventResponderSystem', () => {
React = require('react');
ReactDOM = require('react-dom');
ReactDOMServer = require('react-dom/server');
Scheduler = require('scheduler');
container = document.createElement('div');
document.body.appendChild(container);
});
Expand Down Expand Up @@ -811,8 +813,8 @@ describe('DOMEventResponderSystem', () => {

it('the event responder system should warn on accessing invalid properties', () => {
const TestResponder = createEventResponder({
rootEventTypes: ['click'],
onRootEvent: (event, context, props) => {
targetEventTypes: ['click'],
onEvent: (event, context, props) => {
const syntheticEvent = {
target: event.target,
type: 'click',
Expand All @@ -823,19 +825,24 @@ describe('DOMEventResponderSystem', () => {
});

let handler;
let buttonRef = React.createRef();
const Test = () => {
const listener = React.unstable_useResponder(TestResponder, {
onClick: handler,
});

return <button listeners={listener}>Click me!</button>;
return (
<button listeners={listener} ref={buttonRef}>
Click me!
</button>
);
};
expect(() => {
handler = event => {
event.preventDefault();
};
ReactDOM.render(<Test />, container);
dispatchClickEvent(document.body);
dispatchClickEvent(buttonRef.current);
}).toWarnDev(
'Warning: preventDefault() is not available on event objects created from event responder modules ' +
'(React Flare).' +
Expand All @@ -847,7 +854,7 @@ describe('DOMEventResponderSystem', () => {
event.stopPropagation();
};
ReactDOM.render(<Test />, container);
dispatchClickEvent(document.body);
dispatchClickEvent(buttonRef.current);
}).toWarnDev(
'Warning: stopPropagation() is not available on event objects created from event responder modules ' +
'(React Flare).' +
Expand All @@ -859,7 +866,7 @@ describe('DOMEventResponderSystem', () => {
event.isDefaultPrevented();
};
ReactDOM.render(<Test />, container);
dispatchClickEvent(document.body);
dispatchClickEvent(buttonRef.current);
}).toWarnDev(
'Warning: isDefaultPrevented() is not available on event objects created from event responder modules ' +
'(React Flare).' +
Expand All @@ -871,7 +878,7 @@ describe('DOMEventResponderSystem', () => {
event.isPropagationStopped();
};
ReactDOM.render(<Test />, container);
dispatchClickEvent(document.body);
dispatchClickEvent(buttonRef.current);
}).toWarnDev(
'Warning: isPropagationStopped() is not available on event objects created from event responder modules ' +
'(React Flare).' +
Expand All @@ -883,7 +890,7 @@ describe('DOMEventResponderSystem', () => {
return event.nativeEvent;
};
ReactDOM.render(<Test />, container);
dispatchClickEvent(document.body);
dispatchClickEvent(buttonRef.current);
}).toWarnDev(
'Warning: nativeEvent is not available on event objects created from event responder modules ' +
'(React Flare).' +
Expand Down Expand Up @@ -934,4 +941,57 @@ describe('DOMEventResponderSystem', () => {
ReactDOM.render(<Test2 />, container);
buttonRef.current.dispatchEvent(createEvent('foobar'));
});

it('should work with concurrent mode updates', async () => {
const log = [];
const TestResponder = createEventResponder({
targetEventTypes: ['click'],
onEvent(event, context, props) {
log.push(props);
},
});
const ref = React.createRef();

function Test({counter}) {
const listener = React.unstable_useResponder(TestResponder, {counter});

return (
<button listeners={listener} ref={ref}>
Press me
</button>
);
}

let root = ReactDOM.unstable_createRoot(container);
let batch = root.createBatch();
batch.render(<Test counter={0} />);
Scheduler.unstable_flushAll();
jest.runAllTimers();
batch.commit();

// Click the button
dispatchClickEvent(ref.current);
expect(log).toEqual([{counter: 0}]);

// Clear log
log.length = 0;

// Increase counter
batch = root.createBatch();
batch.render(<Test counter={1} />);
Scheduler.unstable_flushAll();
jest.runAllTimers();

// Click the button again
dispatchClickEvent(ref.current);
expect(log).toEqual([{counter: 0}]);

// Clear log
log.length = 0;

// Commit
batch.commit();
dispatchClickEvent(ref.current);
expect(log).toEqual([{counter: 1}]);
});
});
Original file line number Diff line number Diff line change
Expand Up @@ -449,7 +449,6 @@ export function mountResponderInstance(
props: Object,
state: Object,
instance: Instance,
rootContainerInstance: Container,
) {
if (enableFlareAPI) {
const {rootEventTypes} = responder;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -501,7 +501,6 @@ export function mountResponderInstance(
props: Object,
state: Object,
instance: Instance,
rootContainerInstance: Container,
) {
throw new Error('Not yet implemented.');
}
Expand Down
8 changes: 8 additions & 0 deletions packages/react-reconciler/src/ReactFiberCommitWork.js
Original file line number Diff line number Diff line change
Expand Up @@ -118,6 +118,7 @@ import {
} from './ReactHookEffectTags';
import {didWarnAboutReassigningProps} from './ReactFiberBeginWork';
import {runWithPriority, NormalPriority} from './SchedulerWithReactIntegration';
import {updateEventListeners} from './ReactFiberEvents';

let didWarnAboutUndefinedSnapshotBeforeUpdate: Set<mixed> | null = null;
if (__DEV__) {
Expand Down Expand Up @@ -1331,6 +1332,13 @@ function commitWork(current: Fiber | null, finishedWork: Fiber): void {
finishedWork,
);
}
if (enableFlareAPI) {
const prevListeners = oldProps.listeners;
const nextListeners = newProps.listeners;
if (prevListeners !== nextListeners) {
updateEventListeners(nextListeners, instance, finishedWork);
}
}
}
return;
}
Expand Down
Loading

0 comments on commit fc80772

Please sign in to comment.