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

Convert ReactDOM-test to createRoot #28009

Merged
merged 1 commit into from
Jan 19, 2024
Merged

Convert ReactDOM-test to createRoot #28009

Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
76 changes: 52 additions & 24 deletions packages/react-dom/src/__tests__/ReactDOM-test.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,19 +11,25 @@

let React;
let ReactDOM;
let ReactDOMClient;
let ReactDOMServer;
let ReactTestUtils;

let act;

describe('ReactDOM', () => {
beforeEach(() => {
jest.resetModules();
React = require('react');
ReactDOM = require('react-dom');
ReactDOMClient = require('react-dom/client');
ReactDOMServer = require('react-dom/server');
ReactTestUtils = require('react-dom/test-utils');

act = require('internal-test-utils').act;
});

it('should bubble onSubmit', function () {
it('should bubble onSubmit', async () => {
const container = document.createElement('div');

let count = 0;
Expand All @@ -50,8 +56,11 @@ describe('ReactDOM', () => {
}

document.body.appendChild(container);
const root = ReactDOMClient.createRoot(container);
try {
ReactDOM.render(<Parent />, container);
await act(() => {
root.render(<Parent />);
});
Copy link
Member

Choose a reason for hiding this comment

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

Wonder if we should have a test where the click/focus/etc inside of act, to show what happens if you click before the render completes, since it's async in createRoot? Maybe we have a test for that somewhere.

Copy link
Member Author

Choose a reason for hiding this comment

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

Yeah, I don't have a good understanding of what tests exists. I'll keep this PR focused to just removing .render() calls.

buttonRef.click();
expect(count).toBe(1);
} finally {
Expand Down Expand Up @@ -228,7 +237,7 @@ describe('ReactDOM', () => {
);
});

it('preserves focus', () => {
it('preserves focus', async () => {
let input;
let input2;
class A extends React.Component {
Expand All @@ -255,8 +264,11 @@ describe('ReactDOM', () => {
const log = [];
const container = document.createElement('div');
document.body.appendChild(container);
const root = ReactDOMClient.createRoot(container);
try {
ReactDOM.render(<A showTwo={false} />, container);
await act(() => {
root.render(<A showTwo={false} />);
});
input.focus();

// When the second input is added, let's simulate losing focus, which is
Expand All @@ -277,7 +289,9 @@ describe('ReactDOM', () => {
});

expect(document.activeElement.id).toBe('one');
ReactDOM.render(<A showTwo={true} />, container);
await act(() => {
root.render(<A showTwo={true} />);
});
// input2 gets added, which causes input to get blurred. Then
// componentDidUpdate focuses input2 and that should make it down to here,
// not get overwritten by focus restoration.
Expand All @@ -288,7 +302,7 @@ describe('ReactDOM', () => {
}
});

it('calls focus() on autoFocus elements after they have been mounted to the DOM', () => {
it('calls focus() on autoFocus elements after they have been mounted to the DOM', async () => {
const originalFocus = HTMLElement.prototype.focus;

try {
Expand All @@ -305,14 +319,16 @@ describe('ReactDOM', () => {

const container = document.createElement('div');
document.body.appendChild(container);
ReactDOM.render(
<div>
<h1>Auto-focus Test</h1>
<input autoFocus={true} />
<p>The above input should be focused after mount.</p>
</div>,
container,
);
const root = ReactDOMClient.createRoot(container);
await act(() => {
root.render(
<div>
<h1>Auto-focus Test</h1>
<input autoFocus={true} />
<p>The above input should be focused after mount.</p>
</div>,
);
});

expect(inputFocusedAfterMount).toBe(true);
expect(focusedElement.tagName).toBe('INPUT');
Expand All @@ -321,7 +337,7 @@ describe('ReactDOM', () => {
}
});

it("shouldn't fire duplicate event handler while handling other nested dispatch", () => {
it("shouldn't fire duplicate event handler while handling other nested dispatch", async () => {
const actual = [];

class Wrapper extends React.Component {
Expand Down Expand Up @@ -352,8 +368,11 @@ describe('ReactDOM', () => {

const container = document.createElement('div');
document.body.appendChild(container);
const root = ReactDOMClient.createRoot(container);
try {
ReactDOM.render(<Wrapper />, container);
await act(() => {
root.render(<Wrapper />);
});

const expected = [
'1st node clicked',
Expand All @@ -366,7 +385,7 @@ describe('ReactDOM', () => {
}
});

it('should not crash with devtools installed', () => {
it('should not crash with devtools installed', async () => {
Copy link
Member

Choose a reason for hiding this comment

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

This test is weird because there's no assertions?

Copy link
Member Author

Choose a reason for hiding this comment

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

I guess it's just "Should not crash", ie. an implicit "assert(..).not.toThrow()".

try {
global.__REACT_DEVTOOLS_GLOBAL_HOOK__ = {
inject: function () {},
Expand All @@ -382,14 +401,17 @@ describe('ReactDOM', () => {
return <div />;
}
}
ReactDOM.render(<Component />, document.createElement('container'));
const root = ReactDOMClient.createRoot(document.createElement('div'));
await act(() => {
root.render(<Component />);
});
} finally {
delete global.__REACT_DEVTOOLS_GLOBAL_HOOK__;
}
});

it('should not crash calling findDOMNode inside a function component', () => {
const container = document.createElement('div');
it('should not crash calling findDOMNode inside a function component', async () => {
const root = ReactDOMClient.createRoot(document.createElement('div'));

class Component extends React.Component {
render() {
Expand All @@ -404,11 +426,13 @@ describe('ReactDOM', () => {
};

if (__DEV__) {
ReactDOM.render(<App />, container);
await act(() => {
root.render(<App />);
});
}
});

it('reports stacks with re-entrant renderToString() calls on the client', () => {
it('reports stacks with re-entrant renderToString() calls on the client', async () => {
function Child2(props) {
return <span ariaTypo3="no">{props.children}</span>;
}
Expand Down Expand Up @@ -441,8 +465,12 @@ describe('ReactDOM', () => {
);
}

const container = document.createElement('div');
expect(() => ReactDOM.render(<App />, container)).toErrorDev([
const root = ReactDOMClient.createRoot(document.createElement('div'));
await expect(async () => {
await act(() => {
root.render(<App />);
});
}).toErrorDev([
// ReactDOM(App > div > span)
'Invalid ARIA attribute `ariaTypo`. ARIA attributes follow the pattern aria-* and must be lowercase.\n' +
' in span (at **)\n' +
Expand Down