Skip to content

Commit

Permalink
Fix event listener untracking
Browse files Browse the repository at this point in the history
  • Loading branch information
jackpope committed Mar 3, 2025
1 parent e08683f commit 3ae668f
Show file tree
Hide file tree
Showing 2 changed files with 84 additions and 4 deletions.
7 changes: 3 additions & 4 deletions packages/react-dom-bindings/src/client/ReactFiberConfigDOM.js
Original file line number Diff line number Diff line change
Expand Up @@ -1619,8 +1619,7 @@ FragmentInstancePseudoElement.prototype.removeEventListener = function (
listener: EventListener,
optionsOrUseCapture?: EventListenerOptionsOrUseCapture,
): void {
const listeners = this._eventListeners;
if (listeners === undefined || listeners.length === 0) {
if (this._eventListeners === undefined || this._eventListeners.length === 0) {
return;
}
traverseFragmentInstanceChildren(
Expand All @@ -1633,12 +1632,12 @@ FragmentInstancePseudoElement.prototype.removeEventListener = function (
optionsOrUseCapture,
),
);
const index = indexOfEventListener(listeners, {
const index = indexOfEventListener(this._eventListeners, {
type,
listener,
optionsOrUseCapture,
});
this._eventListeners = listeners.splice(index, 1);
this._eventListeners.splice(index, 1);
};
function removeEventListenerFromChild(
type: string,
Expand Down
81 changes: 81 additions & 0 deletions packages/react-dom/src/__tests__/ReactDOMFragmentRefs-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -412,6 +412,87 @@ describe('FragmentRefs', () => {
expect(logs).toEqual(['Host A', 'Host B']);
});

// @gate enableFragmentRefs
it('allows adding and cleaning up listeners in effects', async () => {
const root = ReactDOMClient.createRoot(container);

let logs = [];
function logClick(e) {
logs.push(e.currentTarget.id);
}

let rerender;
let removeEventListeners;

function Test() {
const fragmentRef = React.useRef(null);
const [_, setState] = React.useState(0);

Check failure on line 429 in packages/react-dom/src/__tests__/ReactDOMFragmentRefs-test.js

View workflow job for this annotation

GitHub Actions / Run eslint

'_' is assigned a value but never used
rerender = () => {
setState(p => p + 1);
};
removeEventListeners = () => {
fragmentRef.current.removeEventListener('click', logClick);
};
React.useEffect(() => {
fragmentRef.current.addEventListener('click', logClick);

return removeEventListeners;
});

return (
<Fragment ref={fragmentRef}>
<div id="child-a" />
</Fragment>
);
}

// The event listener was applied
await act(() => root.render(<Test />));
expect(logs).toEqual([]);
document.querySelector('#child-a').click();
expect(logs).toEqual(['child-a']);

// The event listener can be removed and re-added
logs = [];
await act(rerender);
document.querySelector('#child-a').click();
expect(logs).toEqual(['child-a']);

// The event listener
// Split into two tests
// 1. Removed event listeners are not added to new children
// 2. It can remove and reapply event listeners in effects
});

it('does not apply removed event listeners to new children', async () => {
const root = ReactDOMClient.createRoot(container);
const fragmentRef = React.createRef(null);
function Test() {
return (
<Fragment ref={fragmentRef}>
<div id="child-a" />
</Fragment>
);
}

let logs = [];
function logClick(e) {
logs.push(e.currentTarget.id);
}
await act(() => {
root.render(<Test />);
});
fragmentRef.current.addEventListener('click', logClick);
const childA = document.querySelector('#child-a');
childA.click();
expect(logs).toEqual(['child-a']);

logs = [];
fragmentRef.current.removeEventListener('click', logClick);
childA.click();
expect(logs).toEqual([]);
});

describe('with activity', () => {
// @gate enableFragmentRefs && enableActivity
it('does not apply event listeners to hidden trees', async () => {
Expand Down

0 comments on commit 3ae668f

Please sign in to comment.