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.

Sign up for GitHub

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

Jump to bottom

Convert useFocusWithin to createRoot #28128

Merged
merged 3 commits into from
Jan 29, 2024
Merged

Convert useFocusWithin to createRoot #28128

Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
0a97b1c
Convert useFocusWithin to createRoot
Jan 27, 2024
eceadd9
Fix infinite suspense
Jan 27, 2024
4b3f5a9
Assert after flush
Jan 28, 2024
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
73 changes: 47 additions & 26 deletions ...interactions/events/src/dom/create-event-handle/__tests__/useFocusWithin-test.internal.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,6 @@ import {createEventTarget, setPointerEvent} from 'dom-event-testing-library';

let React;
let ReactFeatureFlags;
let ReactDOM;
let ReactDOMClient;
let useFocusWithin;
let act;
Expand All @@ -25,7 +24,6 @@ function initializeModules(hasPointerEvents) {
ReactFeatureFlags.enableScopeAPI = true;
ReactFeatureFlags.enableCreateEventHandleAPI = true;
React = require('react');
ReactDOM = require('react-dom');
ReactDOMClient = require('react-dom/client');
act = require('internal-test-utils').act;

Expand All @@ -42,17 +40,22 @@ const table = [[forcePointerEvents], [!forcePointerEvents]];
describe.each(table)(`useFocus`, hasPointerEvents => {
let container;
let container2;
let root;

beforeEach(() => {
initializeModules(hasPointerEvents);
container = document.createElement('div');
document.body.appendChild(container);
container2 = document.createElement('div');
document.body.appendChild(container2);
root = ReactDOMClient.createRoot(container);
});

afterEach(() => {
ReactDOM.render(null, container);
afterEach(async () => {
await act(() => {
root.render(null);
});

document.body.removeChild(container);
document.body.removeChild(container2);
container = null;
Expand All @@ -75,7 +78,7 @@ describe.each(table)(`useFocus`, hasPointerEvents => {
return <div ref={focusWithinRef} />;
};
await act(() => {
ReactDOM.render(<Component />, container);
root.render(<Component />);
});
};

Expand Down Expand Up @@ -111,7 +114,7 @@ describe.each(table)(`useFocus`, hasPointerEvents => {
innerRef = React.createRef();
innerRef2 = React.createRef();
await act(() => {
ReactDOM.render(<Component show={true} />, container);
root.render(<Component show={true} />);
});
};

Expand Down Expand Up @@ -188,7 +191,7 @@ describe.each(table)(`useFocus`, hasPointerEvents => {
innerRef = React.createRef();
innerRef2 = React.createRef();
await act(() => {
ReactDOM.render(<Component show={true} />, container);
root.render(<Component show={true} />);
});
};

Expand Down Expand Up @@ -314,7 +317,7 @@ describe.each(table)(`useFocus`, hasPointerEvents => {
);
};
await act(() => {
ReactDOM.render(<Component />, container);
root.render(<Component />);
});

const target = createEventTarget(inputRef.current);
Expand Down Expand Up @@ -354,7 +357,7 @@ describe.each(table)(`useFocus`, hasPointerEvents => {
};

await act(() => {
ReactDOM.render(<Component show={true} />, container);
root.render(<Component show={true} />);
});

const inner = innerRef.current;
Expand All @@ -363,7 +366,10 @@ describe.each(table)(`useFocus`, hasPointerEvents => {
target.focus();
expect(onBeforeBlurWithin).toHaveBeenCalledTimes(0);
expect(onAfterBlurWithin).toHaveBeenCalledTimes(0);
ReactDOM.render(<Component show={false} />, container);
await act(() => {
root.render(<Component show={false} />);
});

expect(onBeforeBlurWithin).toHaveBeenCalledTimes(1);
expect(onAfterBlurWithin).toHaveBeenCalledTimes(1);
expect(onAfterBlurWithin).toHaveBeenCalledWith(
Expand Down Expand Up @@ -391,7 +397,7 @@ describe.each(table)(`useFocus`, hasPointerEvents => {
};

await act(() => {
ReactDOM.render(<Component show={true} />, container);
root.render(<Component show={true} />);
});

const inner = innerRef.current;
Expand All @@ -400,7 +406,11 @@ describe.each(table)(`useFocus`, hasPointerEvents => {
target.focus();
expect(onBeforeBlurWithin).toHaveBeenCalledTimes(0);
expect(onAfterBlurWithin).toHaveBeenCalledTimes(0);
ReactDOM.render(<Component show={false} />, container);

await act(() => {
root.render(<Component show={false} />);
});

expect(onBeforeBlurWithin).toHaveBeenCalledTimes(1);
expect(onAfterBlurWithin).toHaveBeenCalledTimes(1);
expect(onAfterBlurWithin).toHaveBeenCalledWith(
Expand Down Expand Up @@ -433,13 +443,16 @@ describe.each(table)(`useFocus`, hasPointerEvents => {
};

await act(() => {
ReactDOM.render(<Component show={true} />, container);
root.render(<Component show={true} />);
});

inputRef.current.focus();
expect(onBeforeBlurWithin).toHaveBeenCalledTimes(0);
expect(onAfterBlurWithin).toHaveBeenCalledTimes(0);
ReactDOM.render(<Component show={false} />, container);
await act(() => {
root.render(<Component show={false} />);
});

expect(onBeforeBlurWithin).toHaveBeenCalledTimes(1);
expect(onAfterBlurWithin).toHaveBeenCalledTimes(1);
});
Expand Down Expand Up @@ -469,15 +482,15 @@ describe.each(table)(`useFocus`, hasPointerEvents => {
};

await act(() => {
ReactDOM.render(<Component show={true} />, container);
root.render(<Component show={true} />);
});

const inner = innerRef.current;
const target = createEventTarget(inner);
target.keydown({key: 'Tab'});
target.focus();
await act(() => {
ReactDOM.render(<Component show={false} />, container);
root.render(<Component show={false} />);
});
expect(targetNodes).toEqual([targetNode]);
});
Expand Down Expand Up @@ -512,10 +525,10 @@ describe.each(table)(`useFocus`, hasPointerEvents => {
);
};

const root = ReactDOMClient.createRoot(container2);
const root2 = ReactDOMClient.createRoot(container2);

await act(() => {
root.render(<Component />);
root2.render(<Component />);
});
expect(container2.innerHTML).toBe('<div><input></div>');

Expand All @@ -528,14 +541,18 @@ describe.each(table)(`useFocus`, hasPointerEvents => {

suspend = true;
await act(() => {
root.render(<Component />);
root2.render(<Component />);
});
expect(container2.innerHTML).toBe(
'<div><input style="display: none;">Loading...</div>',
);
expect(onBeforeBlurWithin).toHaveBeenCalledTimes(1);
expect(onAfterBlurWithin).toHaveBeenCalledTimes(1);
resolve();
await act(() => {
suspend = false;
resolve();
});
Comment on lines +551 to +554
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Matches other testing patterns e.g.

react/packages/react-dom/src/__tests__/ReactTestUtilsAct-test.js

Lines 773 to 777 in a8875ea

// resolve the promise
await act(async () => {
resolved = true;
resolve();
});

Without also ensuring we don't throw the promise, we end up in an infinite loop and ultimately crash with RangeError: Potential infinite loop: exceeded 1500 iterations.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Makes sense, the infinite loop error is because of the root.render(null); in the after each. If we flush here, it would be good to assert on the output:

Suggested change
});
});
expect(container2.innerHTML).toBe('<div><input style=""></div>');

expect(container2.innerHTML).toBe('<div><input style=""></div>');
});

// @gate www
Expand Down Expand Up @@ -569,37 +586,41 @@ describe.each(table)(`useFocus`, hasPointerEvents => {
);
};

const root = ReactDOMClient.createRoot(container2);
const root2 = ReactDOMClient.createRoot(container2);

await act(() => {
root.render(<Component />);
root2.render(<Component />);
});

expect(onBeforeBlurWithin).toHaveBeenCalledTimes(0);
expect(onAfterBlurWithin).toHaveBeenCalledTimes(0);

suspend = true;
await act(() => {
root.render(<Component />);
root2.render(<Component />);
});
expect(onBeforeBlurWithin).toHaveBeenCalledTimes(0);
expect(onAfterBlurWithin).toHaveBeenCalledTimes(0);

await act(() => {
root.render(<Component />);
root2.render(<Component />);
});
expect(onBeforeBlurWithin).toHaveBeenCalledTimes(0);
expect(onAfterBlurWithin).toHaveBeenCalledTimes(0);

buttonRef.current.focus();
suspend = false;
await act(() => {
root.render(<Component />);
root2.render(<Component />);
});
expect(onBeforeBlurWithin).toHaveBeenCalledTimes(1);
expect(onAfterBlurWithin).toHaveBeenCalledTimes(1);

resolve();
await act(() => {
suspend = false;
resolve();
});
expect(container2.innerHTML).toBe('<div><input style=""></div>');
});
});
});