Skip to content

Commit

Permalink
Remove data-reactroot from server rendering and hydration heuristic
Browse files Browse the repository at this point in the history 
This was used to implicitly hydrate if you call ReactDOM.render.

We've had a warning to explicitly use ReactDOM.hydrate(...) instead of
ReactDOM.render(...). We can now remove this from the generated markup.
(And avoid adding it to Fizz.)

This is a little strange to do now since we're trying hard to make the
root API work the same.

But if we kept it, we'd need to keep it in the generated output which adds
unnecessary bytes. It also risks people relying on it, in the Fizz world
where as this is an opportunity to create that clean state.

We could possibly only keep it in the old server rendering APIs but then
that creates an implicit dependency between which server API and which
client API that you use. Currently you can really mix and match either way.
  • Loading branch information
@sebmarkbage
sebmarkbage committed May 13, 2021
1 parent 46491dc commit f8c0827
Show file tree
Hide file tree
Showing 16 changed files with 31 additions and 456 deletions.
39 changes: 0 additions & 39 deletions packages/react-dom/src/__tests__/ReactCompositeComponent-test.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,6 @@ let ChildUpdates;
let MorphingComponent;
let React;
let ReactDOM;
let ReactDOMServer;
let ReactCurrentOwner;
let ReactTestUtils;
let PropTypes;
Expand Down Expand Up @@ -65,7 +64,6 @@ describe('ReactCompositeComponent', () => {
jest.resetModules();
React = require('react');
ReactDOM = require('react-dom');
ReactDOMServer = require('react-dom/server');
ReactCurrentOwner = require('react')
.__SECRET_INTERNALS_DO_NOT_USE_OR_YOU_WILL_BE_FIRED.ReactCurrentOwner;
ReactTestUtils = require('react-dom/test-utils');
Expand Down Expand Up @@ -170,43 +168,6 @@ describe('ReactCompositeComponent', () => {
expect(el.tagName).toBe('A');
});

it('should not thrash a server rendered layout with client side one', () => {
class Child extends React.Component {
render() {
return null;
}
}

class Parent extends React.Component {
render() {
return (
<div>
<Child />
</div>
);
}
}

const markup = ReactDOMServer.renderToString(<Parent />);

// Old API based on heuristic
let container = document.createElement('div');
container.innerHTML = markup;
expect(() =>
ReactDOM.render(<Parent />, container),
).toWarnDev(
'render(): Calling ReactDOM.render() to hydrate server-rendered markup ' +
'will stop working in React v18. Replace the ReactDOM.render() call ' +
'with ReactDOM.hydrate() if you want React to attach to the server HTML.',
{withoutStack: true},
);

// New explicit API
container = document.createElement('div');
container.innerHTML = markup;
ReactDOM.hydrate(<Parent />, container);
});

it('should react to state changes from callbacks', () => {
const container = document.createElement('div');
document.body.appendChild(container);
Expand Down
4 changes: 1 addition & 3 deletions packages/react-dom/src/__tests__/ReactLegacyContextDisabled-test.internal.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -139,9 +139,7 @@ describe('ReactLegacyContextDisabled', () => {
'LegacyFnConsumer uses the legacy contextTypes API which is no longer supported. ' +
'Use React.createContext() with React.useContext() instead.',
]);
expect(text).toBe(
'<span data-reactroot="">{}<!-- -->undefined<!-- -->undefined</span>',
);
expect(text).toBe('<span>{}<!-- -->undefined<!-- -->undefined</span>');
expect(lifecycleContextLog).toEqual([{}, {}, {}]);
});

Expand Down
219 changes: 0 additions & 219 deletions packages/react-dom/src/__tests__/ReactRenderDocument-test.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -33,225 +33,6 @@ describe('rendering React components at document', () => {
ReactDOMServer = require('react-dom/server');
});

describe('with old implicit hydration API', () => {
function expectDeprecationWarningWithFiber(callback) {
expect(
callback,
).toWarnDev(
'render(): Calling ReactDOM.render() to hydrate server-rendered markup ' +
'will stop working in React v18. Replace the ReactDOM.render() call ' +
'with ReactDOM.hydrate() if you want React to attach to the server HTML.',
{withoutStack: true},
);
}

it('should be able to adopt server markup', () => {
class Root extends React.Component {
render() {
return (
<html>
<head>
<title>Hello World</title>
</head>
<body>{'Hello ' + this.props.hello}</body>
</html>
);
}
}

const markup = ReactDOMServer.renderToString(<Root hello="world" />);
const testDocument = getTestDocument(markup);
const body = testDocument.body;

expectDeprecationWarningWithFiber(() =>
ReactDOM.render(<Root hello="world" />, testDocument),
);
expect(testDocument.body.innerHTML).toBe('Hello world');

ReactDOM.render(<Root hello="moon" />, testDocument);
expect(testDocument.body.innerHTML).toBe('Hello moon');

expect(body === testDocument.body).toBe(true);
});

it('should not be able to unmount component from document node', () => {
class Root extends React.Component {
render() {
return (
<html>
<head>
<title>Hello World</title>
</head>
<body>Hello world</body>
</html>
);
}
}

const markup = ReactDOMServer.renderToString(<Root />);
const testDocument = getTestDocument(markup);
expectDeprecationWarningWithFiber(() =>
ReactDOM.render(<Root />, testDocument),
);
expect(testDocument.body.innerHTML).toBe('Hello world');

// In Fiber this actually works. It might not be a good idea though.
ReactDOM.unmountComponentAtNode(testDocument);
expect(testDocument.firstChild).toBe(null);
});

it('should not be able to switch root constructors', () => {
class Component extends React.Component {
render() {
return (
<html>
<head>
<title>Hello World</title>
</head>
<body>Hello world</body>
</html>
);
}
}

class Component2 extends React.Component {
render() {
return (
<html>
<head>
<title>Hello World</title>
</head>
<body>Goodbye world</body>
</html>
);
}
}

const markup = ReactDOMServer.renderToString(<Component />);
const testDocument = getTestDocument(markup);

expectDeprecationWarningWithFiber(() =>
ReactDOM.render(<Component />, testDocument),
);
expect(testDocument.body.innerHTML).toBe('Hello world');

// This works but is probably a bad idea.
ReactDOM.render(<Component2 />, testDocument);

expect(testDocument.body.innerHTML).toBe('Goodbye world');
});

it('should be able to mount into document', () => {
class Component extends React.Component {
render() {
return (
<html>
<head>
<title>Hello World</title>
</head>
<body>{this.props.text}</body>
</html>
);
}
}

const markup = ReactDOMServer.renderToString(
<Component text="Hello world" />,
);
const testDocument = getTestDocument(markup);

expectDeprecationWarningWithFiber(() =>
ReactDOM.render(<Component text="Hello world" />, testDocument),
);

expect(testDocument.body.innerHTML).toBe('Hello world');
});

it('renders over an existing text child without throwing', () => {
const container = document.createElement('div');
container.textContent = 'potato';
ReactDOM.render(<div>parsnip</div>, container);
expect(container.textContent).toBe('parsnip');
// We don't expect a warning about new hydration API here because
// we aren't sure if the user meant to hydrate or replace a stub node.
// We would see a warning if the container had React-rendered HTML in it.
});

it('should give helpful errors on state desync', () => {
class Component extends React.Component {
render() {
return (
<html>
<head>
<title>Hello World</title>
</head>
<body>{this.props.text}</body>
</html>
);
}
}

const markup = ReactDOMServer.renderToString(
<Component text="Goodbye world" />,
);
const testDocument = getTestDocument(markup);

expect(() => {
expect(() =>
ReactDOM.render(<Component text="Hello world" />, testDocument),
).toWarnDev(
'render(): Calling ReactDOM.render() to hydrate server-rendered markup ' +
'will stop working in React v18. Replace the ReactDOM.render() call ' +
'with ReactDOM.hydrate() if you want React to attach to the server HTML.',
{withoutStack: true},
);
}).toErrorDev('Warning: Text content did not match.');
});

it('should throw on full document render w/ no markup', () => {
const testDocument = getTestDocument();

class Component extends React.Component {
render() {
return (
<html>
<head>
<title>Hello World</title>
</head>
<body>{this.props.text}</body>
</html>
);
}
}

ReactDOM.render(<Component text="Hello world" />, testDocument);
expect(testDocument.body.innerHTML).toBe('Hello world');
// We don't expect a warning about new hydration API here because
// we aren't sure if the user meant to hydrate or replace the document.
// We would see a warning if the document had React-rendered HTML in it.
});

it('supports findDOMNode on full-page components', () => {
const tree = (
<html>
<head>
<title>Hello World</title>
</head>
<body>Hello world</body>
</html>
);

const markup = ReactDOMServer.renderToString(tree);
const testDocument = getTestDocument(markup);
let component;
expectDeprecationWarningWithFiber(() => {
component = ReactDOM.render(tree, testDocument);
});
expect(testDocument.body.innerHTML).toBe('Hello world');
expect(ReactDOM.findDOMNode(component).tagName).toBe('HTML');
});
});

describe('with new explicit hydration API', () => {
it('should be able to adopt server markup', () => {
class Root extends React.Component {
Expand Down
22 changes: 5 additions & 17 deletions packages/react-dom/src/__tests__/ReactServerRendering-test.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -31,14 +31,12 @@ describe('ReactDOMServer', () => {
describe('renderToString', () => {
it('should generate simple markup', () => {
const response = ReactDOMServer.renderToString(<span>hello world</span>);
expect(response).toMatch(
new RegExp('<span data-reactroot=""' + '>hello world</span>'),
);
expect(response).toMatch(new RegExp('<span' + '>hello world</span>'));
});

it('should generate simple markup for self-closing tags', () => {
const response = ReactDOMServer.renderToString(<img />);
expect(response).toMatch(new RegExp('<img data-reactroot=""' + '/>'));
expect(response).toMatch(new RegExp('<img' + '/>'));
});

it('should generate comment markup for component returns null', () => {
Expand Down Expand Up @@ -74,10 +72,7 @@ describe('ReactDOMServer', () => {
const response = ReactDOMServer.renderToString(<Parent />);
expect(response).toMatch(
new RegExp(
'<div ' +
'data-reactroot' +
'=""' +
'>' +
'<div>' +
'<span' +
'>' +
'My name is <!-- -->child' +
Expand Down Expand Up @@ -136,12 +131,7 @@ describe('ReactDOMServer', () => {

expect(response).toMatch(
new RegExp(
'<span ' +
'data-reactroot' +
'=""' +
'>' +
'Component name: <!-- -->TestComponent' +
'</span>',
'<span>' + 'Component name: <!-- -->TestComponent' + '</span>',
),
);
expect(lifecycle).toEqual([
Expand Down Expand Up @@ -580,9 +570,7 @@ describe('ReactDOMServer', () => {
it('should generate simple markup', () => {
const SuccessfulElement = React.createElement(() => <img />);
const response = ReactDOMServer.renderToNodeStream(SuccessfulElement);
expect(response.read().toString()).toMatch(
new RegExp('<img data-reactroot=""' + '/>'),
);
expect(response.read().toString()).toMatch(new RegExp('<img' + '/>'));
});

it('should handle errors correctly', () => {
Expand Down
Loading

0 comments on commit f8c0827

Please sign in to comment.