Skip to content

Commit dcf4360

Browse files
jackpopejackpope
committed
[DOM] Scope Fragment once listeners to the fragment, not each child
{once: true} was forwarded to every host child, so each child could fire independently and newly inserted children re-armed the listener from _eventListeners. Wrap once listeners so the first fire removes them from the fragment and all children. Also, removes dead comment about HostText in RN.
1 parent 4240abd commit dcf4360

3 files changed

Lines changed: 169 additions & 14 deletions

File tree

‎packages/react-dom-bindings/src/client/ReactFiberConfigDOM.js‎

Lines changed: 75 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -2988,6 +2988,9 @@ type StoredEventListener = {
29882988
type: string,
29892989
listener: EventListener,
29902990
optionsOrUseCapture: void | EventListenerOptionsOrUseCapture,
2991+
// When once:true, a wrapper that removes the fragment listener after the
2992+
// first fire. Otherwise the same as listener.
2993+
attachedListener: EventListener,
29912994
};
29922995

29932996
export type FragmentInstanceType = {
@@ -3042,13 +3045,37 @@ FragmentInstance.prototype.addEventListener = function (
30423045
const isNewEventListener =
30433046
indexOfEventListener(listeners, type, listener, optionsOrUseCapture) === -1;
30443047
if (isNewEventListener) {
3045-
listeners.push({type, listener, optionsOrUseCapture});
3048+
const fragmentInstance = this;
3049+
let attachedListener = listener;
3050+
if (isOnceOption(optionsOrUseCapture)) {
3051+
// once is fragment-scoped: the first fire on any child removes this
3052+
// listener from the fragment and every host child.
3053+
attachedListener = function (this: EventTarget, event: Event) {
3054+
fragmentInstance.removeEventListener(
3055+
type,
3056+
listener,
3057+
optionsOrUseCapture,
3058+
);
3059+
if (typeof listener === 'function') {
3060+
listener.call(this, event);
3061+
} else {
3062+
listener.handleEvent(event);
3063+
}
3064+
};
3065+
}
3066+
const attachOptions = getAttachOptions(optionsOrUseCapture);
3067+
listeners.push({
3068+
type,
3069+
listener,
3070+
optionsOrUseCapture,
3071+
attachedListener,
3072+
});
30463073
traverseFragmentInstancesAndTextInstances(
30473074
this._fragmentFiber,
30483075
addEventListenerToChild,
30493076
type,
3050-
listener,
3051-
optionsOrUseCapture,
3077+
attachedListener,
3078+
attachOptions,
30523079
);
30533080
}
30543081
this._eventListeners = listeners;
@@ -3083,12 +3110,15 @@ FragmentInstance.prototype.removeEventListener = function (
30833110
if (index === -1) {
30843111
return;
30853112
}
3113+
const {attachedListener, optionsOrUseCapture: storedOptions} =
3114+
listeners[index];
3115+
const attachOptions = getAttachOptions(storedOptions);
30863116
traverseFragmentInstancesAndTextInstances(
30873117
this._fragmentFiber,
30883118
removeEventListenerFromChild,
30893119
type,
3090-
listener,
3091-
optionsOrUseCapture,
3120+
attachedListener,
3121+
attachOptions,
30923122
);
30933123
listeners.splice(index, 1);
30943124
};
@@ -3102,6 +3132,22 @@ function removeEventListenerFromChild(
31023132
instance.removeEventListener(type, listener, optionsOrUseCapture);
31033133
return false;
31043134
}
3135+
function isOnceOption(opts: ?EventListenerOptionsOrUseCapture): boolean {
3136+
return opts != null && typeof opts !== 'boolean' && opts.once === true;
3137+
}
3138+
function getAttachOptions(
3139+
opts: void | EventListenerOptionsOrUseCapture,
3140+
): void | EventListenerOptionsOrUseCapture {
3141+
// Strip once when attaching to host children; Fragment owns once semantics.
3142+
if (opts == null || typeof opts === 'boolean' || opts.once !== true) {
3143+
return opts;
3144+
}
3145+
return {
3146+
capture: opts.capture,
3147+
passive: opts.passive,
3148+
signal: opts.signal,
3149+
};
3150+
}
31053151
function normalizeListenerOptions(
31063152
opts: ?EventListenerOptionsOrUseCapture,
31073153
): string {
@@ -3166,16 +3212,24 @@ FragmentInstance.prototype.dispatchEvent = function (
31663212
: document.createTextNode('');
31673213
if (eventListeners) {
31683214
for (let i = 0; i < eventListeners.length; i++) {
3169-
const {type, listener, optionsOrUseCapture} = eventListeners[i];
3170-
temp.addEventListener(type, listener, optionsOrUseCapture);
3215+
const {type, attachedListener, optionsOrUseCapture} = eventListeners[i];
3216+
temp.addEventListener(
3217+
type,
3218+
attachedListener,
3219+
getAttachOptions(optionsOrUseCapture),
3220+
);
31713221
}
31723222
}
31733223
parentHostInstance.appendChild(temp);
31743224
const cancelable = temp.dispatchEvent(event);
31753225
if (eventListeners) {
31763226
for (let i = 0; i < eventListeners.length; i++) {
3177-
const {type, listener, optionsOrUseCapture} = eventListeners[i];
3178-
temp.removeEventListener(type, listener, optionsOrUseCapture);
3227+
const {type, attachedListener, optionsOrUseCapture} = eventListeners[i];
3228+
temp.removeEventListener(
3229+
type,
3230+
attachedListener,
3231+
getAttachOptions(optionsOrUseCapture),
3232+
);
31793233
}
31803234
}
31813235
parentHostInstance.removeChild(temp);
@@ -3730,8 +3784,12 @@ export function commitNewChildToFragmentInstance(
37303784
const eventListeners = fragmentInstance._eventListeners;
37313785
if (eventListeners !== null) {
37323786
for (let i = 0; i < eventListeners.length; i++) {
3733-
const {type, listener, optionsOrUseCapture} = eventListeners[i];
3734-
childInstance.addEventListener(type, listener, optionsOrUseCapture);
3787+
const {type, attachedListener, optionsOrUseCapture} = eventListeners[i];
3788+
childInstance.addEventListener(
3789+
type,
3790+
attachedListener,
3791+
getAttachOptions(optionsOrUseCapture),
3792+
);
37353793
}
37363794
}
37373795
// Observers and fragment handles only apply to element children.
@@ -3756,8 +3814,12 @@ export function deleteChildFromFragmentInstance(
37563814
const eventListeners = fragmentInstance._eventListeners;
37573815
if (eventListeners !== null) {
37583816
for (let i = 0; i < eventListeners.length; i++) {
3759-
const {type, listener, optionsOrUseCapture} = eventListeners[i];
3760-
childInstance.removeEventListener(type, listener, optionsOrUseCapture);
3817+
const {type, attachedListener, optionsOrUseCapture} = eventListeners[i];
3818+
childInstance.removeEventListener(
3819+
type,
3820+
attachedListener,
3821+
getAttachOptions(optionsOrUseCapture),
3822+
);
37613823
}
37623824
}
37633825
if (childInstance.nodeType === TEXT_NODE) {

‎packages/react-dom/src/__tests__/ReactDOMFragmentRefs-test.js‎

Lines changed: 94 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -809,6 +809,100 @@ describe('FragmentRefs', () => {
809809
expect(hasClicked).toBe(true);
810810
});
811811

812+
// @gate enableFragmentRefs
813+
it('fires a once listener only once across existing children', async () => {
814+
const fragmentRef = React.createRef();
815+
const childARef = React.createRef();
816+
const childBRef = React.createRef();
817+
const root = ReactDOMClient.createRoot(container);
818+
819+
await act(() => {
820+
root.render(
821+
<div>
822+
<Fragment ref={fragmentRef}>
823+
<div ref={childARef} id="a">
824+
A
825+
</div>
826+
<div ref={childBRef} id="b">
827+
B
828+
</div>
829+
</Fragment>
830+
</div>,
831+
);
832+
});
833+
834+
const logs = [];
835+
fragmentRef.current.addEventListener(
836+
'click',
837+
() => {
838+
logs.push('once');
839+
},
840+
{once: true},
841+
);
842+
843+
childARef.current.click();
844+
expect(logs).toEqual(['once']);
845+
846+
logs.length = 0;
847+
childBRef.current.click();
848+
expect(logs).toEqual([]);
849+
});
850+
851+
// @gate enableFragmentRefs
852+
it('does not re-arm a once listener when a new child is inserted', async () => {
853+
const fragmentRef = React.createRef();
854+
const childARef = React.createRef();
855+
const childBRef = React.createRef();
856+
const root = ReactDOMClient.createRoot(container);
857+
let showChildB;
858+
859+
function Component() {
860+
const [shouldShowChildB, setShouldShowChildB] = React.useState(false);
861+
showChildB = () => {
862+
setShouldShowChildB(true);
863+
};
864+
865+
return (
866+
<div>
867+
<Fragment ref={fragmentRef}>
868+
<div ref={childARef} id="a">
869+
A
870+
</div>
871+
{shouldShowChildB && (
872+
<div ref={childBRef} id="b">
873+
B
874+
</div>
875+
)}
876+
</Fragment>
877+
</div>
878+
);
879+
}
880+
881+
await act(() => {
882+
root.render(<Component />);
883+
});
884+
885+
const logs = [];
886+
fragmentRef.current.addEventListener(
887+
'click',
888+
() => {
889+
logs.push('once');
890+
},
891+
{once: true},
892+
);
893+
894+
childARef.current.click();
895+
expect(logs).toEqual(['once']);
896+
897+
await act(() => {
898+
showChildB();
899+
});
900+
901+
logs.length = 0;
902+
childBRef.current.click();
903+
expect(logs).toEqual([]);
904+
});
905+
812906
// @gate enableFragmentRefs && enableFragmentRefsTextNodes
813907
it('adds an event listener to a newly added text child', async () => {
814908
const fragmentRef = React.createRef();

‎packages/react-reconciler/src/ReactFiberCommitWork.js‎

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3303,7 +3303,6 @@ function reappearLayoutEffects(
33033303
}
33043304
case HostHoistable:
33053305
case HostComponent: {
3306-
// TODO: Enable HostText for RN
33073306
if (
33083307
enableFragmentRefs &&
33093308
(finishedWork.tag === HostComponent ||

0 commit comments

Comments
 (0)