Skip to content

Commit

Permalink
Convert ReactMount to createRoot (#28075)
Browse files Browse the repository at this point in the history 
To convert this file, I started replacing all the calls in line, and
quickly realized that we already have most of these tests covered in
other files. So I found the test that we didn't already have for
`create/hydrateRoot` and added them, then renamed `ReactMount` to
`ReactLegacyMount`.
  • Loading branch information
@rickhanlonii
rickhanlonii authored Jan 25, 2024
1 parent 11c9fd0 commit 5420c4e
Show file tree
Hide file tree
Showing 3 changed files with 263 additions and 140 deletions.
37 changes: 37 additions & 0 deletions packages/react-dom/src/__tests__/ReactDOMHydrationDiff-test.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -113,6 +113,43 @@ describe('ReactDOMServerHydration', () => {
}
});

// @gate __DEV__
it('warns when escaping on a checksum mismatch', () => {
function Mismatch({isClient}) {
if (isClient) {
return (
<div>This markup contains an nbsp entity: &nbsp; client text</div>
);
}
return (
<div>This markup contains an nbsp entity: &nbsp; server text</div>
);
}

/* eslint-disable no-irregular-whitespace */
if (gate(flags => flags.enableClientRenderFallbackOnTextMismatch)) {
expect(testMismatch(Mismatch)).toMatchInlineSnapshot(`
[
"Warning: Text content did not match. Server: "This markup contains an nbsp entity:   server text" Client: "This markup contains an nbsp entity:   client text"
in div (at **)
in Mismatch (at **)",
"Warning: An error occurred during hydration. The server HTML was replaced with client content in <div>.",
"Caught [Text content does not match server-rendered HTML.]",
"Caught [There was an error while hydrating. Because the error happened outside of a Suspense boundary, the entire root will switch to client rendering.]",
]
`);
} else {
expect(testMismatch(Mismatch)).toMatchInlineSnapshot(`
[
"Warning: Text content did not match. Server: "This markup contains an nbsp entity:   server text" Client: "This markup contains an nbsp entity:   client text"
in div (at **)
in Mismatch (at **)",
]
`);
}
/* eslint-enable no-irregular-whitespace */
});

// @gate __DEV__
it('warns when client and server render different html', () => {
function Mismatch({isClient}) {
Expand Down
233 changes: 93 additions & 140 deletions packages/react-dom/src/__tests__/ReactDOMRoot-test.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -175,27 +175,6 @@ describe('ReactDOMRoot', () => {
);
});

it('clears existing children with legacy API', async () => {
container.innerHTML = '<div>a</div><div>b</div>';
ReactDOM.render(
<div>
<span>c</span>
<span>d</span>
</div>,
container,
);
expect(container.textContent).toEqual('cd');
ReactDOM.render(
<div>
<span>d</span>
<span>c</span>
</div>,
container,
);
await waitForAll([]);
expect(container.textContent).toEqual('dc');
});

it('clears existing children', async () => {
container.innerHTML = '<div>a</div><div>b</div>';
const root = ReactDOMClient.createRoot(container);
Expand Down Expand Up @@ -223,122 +202,6 @@ describe('ReactDOMRoot', () => {
}).toThrow('createRoot(...): Target container is not a DOM element.');
});

it('warns when rendering with legacy API into createRoot() container', async () => {
const root = ReactDOMClient.createRoot(container);
root.render(<div>Hi</div>);
await waitForAll([]);
expect(container.textContent).toEqual('Hi');
expect(() => {
ReactDOM.render(<div>Bye</div>, container);
}).toErrorDev(
[
// We care about this warning:
'You are calling ReactDOM.render() on a container that was previously ' +
'passed to ReactDOMClient.createRoot(). This is not supported. ' +
'Did you mean to call root.render(element)?',
// This is more of a symptom but restructuring the code to avoid it isn't worth it:
'Replacing React-rendered children with a new root component.',
],
{withoutStack: true},
);
await waitForAll([]);
// This works now but we could disallow it:
expect(container.textContent).toEqual('Bye');
});

it('warns when hydrating with legacy API into createRoot() container', async () => {
const root = ReactDOMClient.createRoot(container);
root.render(<div>Hi</div>);
await waitForAll([]);
expect(container.textContent).toEqual('Hi');
expect(() => {
ReactDOM.hydrate(<div>Hi</div>, container);
}).toErrorDev(
[
// We care about this warning:
'You are calling ReactDOM.hydrate() on a container that was previously ' +
'passed to ReactDOMClient.createRoot(). This is not supported. ' +
'Did you mean to call hydrateRoot(container, element)?',
// This is more of a symptom but restructuring the code to avoid it isn't worth it:
'Replacing React-rendered children with a new root component.',
],
{withoutStack: true},
);
});

it('callback passed to legacy hydrate() API', () => {
container.innerHTML = '<div>Hi</div>';
ReactDOM.hydrate(<div>Hi</div>, container, () => {
Scheduler.log('callback');
});
expect(container.textContent).toEqual('Hi');
assertLog(['callback']);
});

it('warns when unmounting with legacy API (no previous content)', async () => {
const root = ReactDOMClient.createRoot(container);
root.render(<div>Hi</div>);
await waitForAll([]);
expect(container.textContent).toEqual('Hi');
let unmounted = false;
expect(() => {
unmounted = ReactDOM.unmountComponentAtNode(container);
}).toErrorDev(
[
// We care about this warning:
'You are calling ReactDOM.unmountComponentAtNode() on a container that was previously ' +
'passed to ReactDOMClient.createRoot(). This is not supported. Did you mean to call root.unmount()?',
// This is more of a symptom but restructuring the code to avoid it isn't worth it:
"The node you're attempting to unmount was rendered by React and is not a top-level container.",
],
{withoutStack: true},
);
expect(unmounted).toBe(false);
await waitForAll([]);
expect(container.textContent).toEqual('Hi');
root.unmount();
await waitForAll([]);
expect(container.textContent).toEqual('');
});

it('warns when unmounting with legacy API (has previous content)', async () => {
// Currently createRoot().render() doesn't clear this.
container.appendChild(document.createElement('div'));
// The rest is the same as test above.
const root = ReactDOMClient.createRoot(container);
root.render(<div>Hi</div>);
await waitForAll([]);
expect(container.textContent).toEqual('Hi');
let unmounted = false;
expect(() => {
unmounted = ReactDOM.unmountComponentAtNode(container);
}).toErrorDev(
[
'Did you mean to call root.unmount()?',
// This is more of a symptom but restructuring the code to avoid it isn't worth it:
"The node you're attempting to unmount was rendered by React and is not a top-level container.",
],
{withoutStack: true},
);
expect(unmounted).toBe(false);
await waitForAll([]);
expect(container.textContent).toEqual('Hi');
root.unmount();
await waitForAll([]);
expect(container.textContent).toEqual('');
});

it('warns when passing legacy container to createRoot()', () => {
ReactDOM.render(<div>Hi</div>, container);
expect(() => {
ReactDOMClient.createRoot(container);
}).toErrorDev(
'You are calling ReactDOMClient.createRoot() on a container that was previously ' +
'passed to ReactDOM.render(). This is not supported.',
{withoutStack: true},
);
});

it('warns when creating two roots managing the same container', () => {
ReactDOMClient.createRoot(container);
expect(() => {
Expand Down Expand Up @@ -399,6 +262,80 @@ describe('ReactDOMRoot', () => {
}
});

it('should render different components in same root', async () => {
document.body.appendChild(container);
const root = ReactDOMClient.createRoot(container);

await act(() => {
root.render(<div />);
});
expect(container.firstChild.nodeName).toBe('DIV');

await act(() => {
root.render(<span />);
});
expect(container.firstChild.nodeName).toBe('SPAN');
});

it('should not warn if mounting into non-empty node', async () => {
container.innerHTML = '<div></div>';
const root = ReactDOMClient.createRoot(container);
await act(() => {
root.render(<div />);
});

expect(true).toBe(true);
});

it('should reuse markup if rendering to the same target twice', async () => {
const root = ReactDOMClient.createRoot(container);
await act(() => {
root.render(<div />);
});
const firstElm = container.firstChild;
await act(() => {
root.render(<div />);
});

expect(firstElm).toBe(container.firstChild);
});

it('should unmount and remount if the key changes', async () => {
function Component({text}) {
useEffect(() => {
Scheduler.log('Mount');

return () => {
Scheduler.log('Unmount');
};
}, []);

return <span>{text}</span>;
}

const root = ReactDOMClient.createRoot(container);

await act(() => {
root.render(<Component text="orange" key="A" />);
});
expect(container.firstChild.innerHTML).toBe('orange');
assertLog(['Mount']);

// If we change the key, the component is unmounted and remounted
await act(() => {
root.render(<Component text="green" key="B" />);
});
expect(container.firstChild.innerHTML).toBe('green');
assertLog(['Unmount', 'Mount']);

// But if we don't change the key, the component instance is reused
await act(() => {
root.render(<Component text="blue" key="B" />);
});
expect(container.firstChild.innerHTML).toBe('blue');
assertLog([]);
});

it('throws if unmounting a root that has had its contents removed', async () => {
const root = ReactDOMClient.createRoot(container);
await act(() => {
Expand Down Expand Up @@ -514,9 +451,6 @@ describe('ReactDOMRoot', () => {
expect(() => ReactDOMClient.hydrateRoot(commentNode)).toThrow(
'hydrateRoot(...): Target container is not a DOM element.',
);

// Still works in the legacy API
ReactDOM.render(<div />, commentNode);
});

it('warn if no children passed to hydrateRoot', async () => {
Expand All @@ -539,4 +473,23 @@ describe('ReactDOMRoot', () => {
},
);
});

it('warns when given a function', () => {
function Component() {
return <div />;
}

const root = ReactDOMClient.createRoot(document.createElement('div'));

expect(() => {
ReactDOM.flushSync(() => {
root.render(Component);
});
}).toErrorDev(
'Functions are not valid as a React child. ' +
'This may happen if you return a Component instead of <Component /> from render. ' +
'Or maybe you meant to call this function rather than return it.',
{withoutStack: true},
);
});
});
Loading

0 comments on commit 5420c4e

Please sign in to comment.