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 committed Mar 13, 2021
1 parent f227e7f commit 11d2dd9
Show file tree
Hide file tree
Showing 16 changed files with 190 additions and 617 deletions.
39 changes: 0 additions & 39 deletions packages/react-dom/src/__tests__/ReactCompositeComponent-test.js
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
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
Loading

0 comments on commit 11d2dd9

Please sign in to comment.