Skip to content

Commit 4a3f779

Browse files
trueadmtrueadm
authored
Remove event pooling in the modern system (facebook#18969)
1 parent 22f7663 commit 4a3f779

File tree

7 files changed

+412
-361
lines changed

7 files changed

+412
-361
lines changed

‎packages/legacy-events/SyntheticEvent.js

Lines changed: 92 additions & 71 deletions
Original file line numberDiff line numberDiff line change
@@ -157,73 +157,79 @@ Object.assign(SyntheticEvent.prototype, {
157157
* won't be added back into the pool.
158158
*/
159159
persist: function() {
160-
this.isPersistent = functionThatReturnsTrue;
160+
// Modern event system doesn't use pooling.
161+
if (!enableModernEventSystem) {
162+
this.isPersistent = functionThatReturnsTrue;
163+
}
161164
},
162165

163166
/**
164167
* Checks if this event should be released back into the pool.
165168
*
166169
* @return {boolean} True if this should not be released, false otherwise.
167170
*/
168-
isPersistent: functionThatReturnsFalse,
171+
isPersistent: enableModernEventSystem
172+
? functionThatReturnsTrue
173+
: functionThatReturnsFalse,
169174

170175
/**
171176
* `PooledClass` looks for `destructor` on each instance it releases.
172177
*/
173178
destructor: function() {
174-
const Interface = this.constructor.Interface;
175-
for (const propName in Interface) {
179+
// Modern event system doesn't use pooling.
180+
if (!enableModernEventSystem) {
181+
const Interface = this.constructor.Interface;
182+
for (const propName in Interface) {
183+
if (__DEV__) {
184+
Object.defineProperty(
185+
this,
186+
propName,
187+
getPooledWarningPropertyDefinition(propName, Interface[propName]),
188+
);
189+
} else {
190+
this[propName] = null;
191+
}
192+
}
193+
this.dispatchConfig = null;
194+
this._targetInst = null;
195+
this.nativeEvent = null;
196+
this.isDefaultPrevented = functionThatReturnsFalse;
197+
this.isPropagationStopped = functionThatReturnsFalse;
198+
this._dispatchListeners = null;
199+
this._dispatchInstances = null;
176200
if (__DEV__) {
177201
Object.defineProperty(
178202
this,
179-
propName,
180-
getPooledWarningPropertyDefinition(propName, Interface[propName]),
203+
'nativeEvent',
204+
getPooledWarningPropertyDefinition('nativeEvent', null),
181205
);
182-
} else {
183-
this[propName] = null;
184-
}
185-
}
186-
this.dispatchConfig = null;
187-
this._targetInst = null;
188-
this.nativeEvent = null;
189-
this.isDefaultPrevented = functionThatReturnsFalse;
190-
this.isPropagationStopped = functionThatReturnsFalse;
191-
if (!enableModernEventSystem) {
192-
this._dispatchListeners = null;
193-
this._dispatchInstances = null;
194-
}
195-
if (__DEV__) {
196-
Object.defineProperty(
197-
this,
198-
'nativeEvent',
199-
getPooledWarningPropertyDefinition('nativeEvent', null),
200-
);
201-
Object.defineProperty(
202-
this,
203-
'isDefaultPrevented',
204-
getPooledWarningPropertyDefinition(
206+
Object.defineProperty(
207+
this,
205208
'isDefaultPrevented',
206-
functionThatReturnsFalse,
207-
),
208-
);
209-
Object.defineProperty(
210-
this,
211-
'isPropagationStopped',
212-
getPooledWarningPropertyDefinition(
209+
getPooledWarningPropertyDefinition(
210+
'isDefaultPrevented',
211+
functionThatReturnsFalse,
212+
),
213+
);
214+
Object.defineProperty(
215+
this,
213216
'isPropagationStopped',
214-
functionThatReturnsFalse,
215-
),
216-
);
217-
Object.defineProperty(
218-
this,
219-
'preventDefault',
220-
getPooledWarningPropertyDefinition('preventDefault', () => {}),
221-
);
222-
Object.defineProperty(
223-
this,
224-
'stopPropagation',
225-
getPooledWarningPropertyDefinition('stopPropagation', () => {}),
226-
);
217+
getPooledWarningPropertyDefinition(
218+
'isPropagationStopped',
219+
functionThatReturnsFalse,
220+
),
221+
);
222+
Object.defineProperty(
223+
this,
224+
'preventDefault',
225+
getPooledWarningPropertyDefinition('preventDefault', () => {}),
226+
);
227+
Object.defineProperty(
228+
this,
229+
'stopPropagation',
230+
getPooledWarningPropertyDefinition('stopPropagation', () => {}),
231+
);
232+
}
227233
}
228234
},
229235
});
@@ -303,18 +309,26 @@ function getPooledWarningPropertyDefinition(propName, getVal) {
303309
}
304310
}
305311

306-
function getPooledEvent(dispatchConfig, targetInst, nativeEvent, nativeInst) {
312+
function createOrGetPooledEvent(
313+
dispatchConfig,
314+
targetInst,
315+
nativeEvent,
316+
nativeInst,
317+
) {
307318
const EventConstructor = this;
308-
if (EventConstructor.eventPool.length) {
309-
const instance = EventConstructor.eventPool.pop();
310-
EventConstructor.call(
311-
instance,
312-
dispatchConfig,
313-
targetInst,
314-
nativeEvent,
315-
nativeInst,
316-
);
317-
return instance;
319+
// Modern event system doesn't use pooling.
320+
if (!enableModernEventSystem) {
321+
if (EventConstructor.eventPool.length) {
322+
const instance = EventConstructor.eventPool.pop();
323+
EventConstructor.call(
324+
instance,
325+
dispatchConfig,
326+
targetInst,
327+
nativeEvent,
328+
nativeInst,
329+
);
330+
return instance;
331+
}
318332
}
319333
return new EventConstructor(
320334
dispatchConfig,
@@ -325,21 +339,28 @@ function getPooledEvent(dispatchConfig, targetInst, nativeEvent, nativeInst) {
325339
}
326340

327341
function releasePooledEvent(event) {
328-
const EventConstructor = this;
329-
invariant(
330-
event instanceof EventConstructor,
331-
'Trying to release an event instance into a pool of a different type.',
332-
);
333-
event.destructor();
334-
if (EventConstructor.eventPool.length < EVENT_POOL_SIZE) {
335-
EventConstructor.eventPool.push(event);
342+
// Modern event system doesn't use pooling.
343+
if (!enableModernEventSystem) {
344+
const EventConstructor = this;
345+
invariant(
346+
event instanceof EventConstructor,
347+
'Trying to release an event instance into a pool of a different type.',
348+
);
349+
event.destructor();
350+
if (EventConstructor.eventPool.length < EVENT_POOL_SIZE) {
351+
EventConstructor.eventPool.push(event);
352+
}
336353
}
337354
}
338355

339356
function addEventPoolingTo(EventConstructor) {
340-
EventConstructor.eventPool = [];
341-
EventConstructor.getPooled = getPooledEvent;
342-
EventConstructor.release = releasePooledEvent;
357+
EventConstructor.getPooled = createOrGetPooledEvent;
358+
359+
// Modern event system doesn't use pooling.
360+
if (!enableModernEventSystem) {
361+
EventConstructor.eventPool = [];
362+
EventConstructor.release = releasePooledEvent;
363+
}
343364
}
344365

345366
export default SyntheticEvent;

‎packages/react-dom/src/events/DOMModernPluginEventSystem.js

Lines changed: 1 addition & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -207,10 +207,7 @@ export function dispatchEventsInBatch(dispatchQueue: DispatchQueue): void {
207207
const dispatchQueueItem: DispatchQueueItem = dispatchQueue[i];
208208
const {event, capture, bubble} = dispatchQueueItem;
209209
executeDispatchesInOrder(event, capture, bubble);
210-
// Release the event from the pool if needed
211-
if (!event.isPersistent()) {
212-
event.constructor.release(event);
213-
}
210+
// Modern event system doesn't use pooling.
214211
}
215212
// This would be a good time to rethrow if any of the event handlers threw.
216213
rethrowCaughtError();

‎packages/react-dom/src/events/__tests__/DOMModernPluginEventSystem-test.internal.js

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -78,6 +78,27 @@ describe('DOMModernPluginEventSystem', () => {
7878
endNativeEventListenerClearDown();
7979
});
8080

81+
it('does not pool events', () => {
82+
const buttonRef = React.createRef();
83+
const log = [];
84+
const onClick = jest.fn(e => log.push(e));
85+
86+
function Test() {
87+
return <button ref={buttonRef} onClick={onClick} />;
88+
}
89+
90+
ReactDOM.render(<Test />, container);
91+
92+
let buttonElement = buttonRef.current;
93+
dispatchClickEvent(buttonElement);
94+
expect(onClick).toHaveBeenCalledTimes(1);
95+
dispatchClickEvent(buttonElement);
96+
expect(onClick).toHaveBeenCalledTimes(2);
97+
expect(log[0]).not.toBe(log[1]);
98+
expect(log[0].type).toBe('click');
99+
expect(log[1].type).toBe('click');
100+
});
101+
81102
it('handle propagation of click events', () => {
82103
const buttonRef = React.createRef();
83104
const divRef = React.createRef();

‎packages/react-dom/src/events/__tests__/SyntheticClipboardEvent-test.js

Lines changed: 39 additions & 36 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,7 @@
1111

1212
let React;
1313
let ReactDOM;
14+
const ReactFeatureFlags = require('shared/ReactFeatureFlags');
1415

1516
describe('SyntheticClipboardEvent', () => {
1617
let container;
@@ -117,41 +118,43 @@ describe('SyntheticClipboardEvent', () => {
117118
expect(expectedCount).toBe(3);
118119
});
119120

120-
it('is able to `persist`', () => {
121-
const persistentEvents = [];
122-
const eventHandler = event => {
123-
expect(event.isPersistent()).toBe(false);
124-
event.persist();
125-
expect(event.isPersistent()).toBe(true);
126-
persistentEvents.push(event);
127-
};
128-
129-
const div = ReactDOM.render(
130-
<div
131-
onCopy={eventHandler}
132-
onCut={eventHandler}
133-
onPaste={eventHandler}
134-
/>,
135-
container,
136-
);
137-
138-
let event;
139-
event = document.createEvent('Event');
140-
event.initEvent('copy', true, true);
141-
div.dispatchEvent(event);
142-
143-
event = document.createEvent('Event');
144-
event.initEvent('cut', true, true);
145-
div.dispatchEvent(event);
146-
147-
event = document.createEvent('Event');
148-
event.initEvent('paste', true, true);
149-
div.dispatchEvent(event);
150-
151-
expect(persistentEvents.length).toBe(3);
152-
expect(persistentEvents[0].type).toBe('copy');
153-
expect(persistentEvents[1].type).toBe('cut');
154-
expect(persistentEvents[2].type).toBe('paste');
155-
});
121+
if (!ReactFeatureFlags.enableModernEventSystem) {
122+
it('is able to `persist`', () => {
123+
const persistentEvents = [];
124+
const eventHandler = event => {
125+
expect(event.isPersistent()).toBe(false);
126+
event.persist();
127+
expect(event.isPersistent()).toBe(true);
128+
persistentEvents.push(event);
129+
};
130+
131+
const div = ReactDOM.render(
132+
<div
133+
onCopy={eventHandler}
134+
onCut={eventHandler}
135+
onPaste={eventHandler}
136+
/>,
137+
container,
138+
);
139+
140+
let event;
141+
event = document.createEvent('Event');
142+
event.initEvent('copy', true, true);
143+
div.dispatchEvent(event);
144+
145+
event = document.createEvent('Event');
146+
event.initEvent('cut', true, true);
147+
div.dispatchEvent(event);
148+
149+
event = document.createEvent('Event');
150+
event.initEvent('paste', true, true);
151+
div.dispatchEvent(event);
152+
153+
expect(persistentEvents.length).toBe(3);
154+
expect(persistentEvents[0].type).toBe('copy');
155+
expect(persistentEvents[1].type).toBe('cut');
156+
expect(persistentEvents[2].type).toBe('paste');
157+
});
158+
}
156159
});
157160
});

0 commit comments

Comments
 (0)