@samitouri / QOS-React / commits / 1454f9c10c

Fix invariant parity for SSR (#10333)

Dan Abramov committed Jul 31, 2017 at 21:13 UTC 1454f9c10c4f487bd87227aad0162d0c888b20e3

6 files changed +172 -72

src/renderers/dom/ReactDOMNodeStreamRenderer.js

+2 -2

@@ -45,7 +45,7 @@ function renderToStream(element) {

45 if (disableNewFiberFeatures) {

46 invariant(

47 React.isValidElement(element),

48 - 'renderToStream(): You must pass a valid ReactElement.',

48 + 'renderToStream(): Invalid component element.',

49 );

50 }

51 return new ReactMarkupReadableStream(element, false);

@@ -61,7 +61,7 @@ function renderToStaticStream(element) {

61 if (disableNewFiberFeatures) {

62 invariant(

63 React.isValidElement(element),

64 - 'renderToStaticStream(): You must pass a valid ReactElement.',

64 + 'renderToStaticStream(): Invalid component element.',

65 );

66 }

67 return new ReactMarkupReadableStream(element, true);

src/renderers/dom/ReactDOMStringRenderer.js

+2 -2

@@ -26,7 +26,7 @@ function renderToString(element) {

26 if (disableNewFiberFeatures) {

27 invariant(

28 React.isValidElement(element),

29 - 'renderToString(): You must pass a valid ReactElement.',

29 + 'renderToString(): Invalid component element.',

30 );

31 }

32 var renderer = new ReactPartialRenderer(element, false);

@@ -44,7 +44,7 @@ function renderToStaticMarkup(element) {

44 if (disableNewFiberFeatures) {

45 invariant(

46 React.isValidElement(element),

47 - 'renderToStaticMarkup(): You must pass a valid ReactElement.',

47 + 'renderToStaticMarkup(): Invalid component element.',

48 );

49 }

50 var renderer = new ReactPartialRenderer(element, true);

src/renderers/dom/shared/__tests__/ReactDOMServerIntegration-test.js

+150 -64

@@ -23,6 +23,9 @@ let ReactTestUtils;

24 const stream = require('stream');

26 +const TEXT_NODE_TYPE = 3;

27 +const COMMENT_NODE_TYPE = 8;

28 +

29 // Helper functions for rendering tests

30 // ====================================

@@ -138,15 +141,36 @@ const clientCleanRender = (element, errorCount = 0) => {

141 const clientRenderOnServerString = async (element, errorCount = 0) => {

142 const markup = await renderIntoString(element, errorCount);

143 resetModules();

144 +

145 var domElement = document.createElement('div');

146 domElement.innerHTML = markup;

143 - const serverElement = domElement.firstChild;

144 - const clientElement = await renderIntoDom(element, domElement, errorCount);

145 - // assert that the DOM element hasn't been replaced.

146 - // Note that we cannot use expect(serverElement).toBe(clientElement) because

147 - // of jest bug #1772

148 - expect(serverElement === clientElement).toBe(true);

149 - return clientElement;

147 + let serverNode = domElement.firstChild;

148 +

149 + const firstClientNode = await renderIntoDom(element, domElement, errorCount);

150 + let clientNode = firstClientNode;

151 +

152 + // Make sure all top level nodes match up

153 + while (serverNode || clientNode) {

154 + expect(serverNode != null).toBe(true);

155 + expect(clientNode != null).toBe(true);

156 + expect(clientNode.nodeType).toBe(serverNode.nodeType);

157 + if (clientNode.nodeType === TEXT_NODE_TYPE) {

158 + // Text nodes are stateless so we can just compare values.

159 + // This works around a current issue where hydration replaces top-level

160 + // text node, but otherwise works.

161 + // TODO: we can remove this branch if we add explicit hydration opt-in.

162 + // https://github.com/facebook/react/issues/10189

163 + expect(serverNode.nodeValue).toBe(clientNode.nodeValue);

164 + } else {

165 + // Assert that the DOM element hasn't been replaced.

166 + // Note that we cannot use expect(serverNode).toBe(clientNode) because

167 + // of jest bug #1772.

168 + expect(serverNode === clientNode).toBe(true);

169 + }

170 + serverNode = serverNode.nextSibling;

171 + clientNode = clientNode.nextSibling;

172 + }

173 + return firstClientNode;

174 };

175

176 function BadMarkupExpected() {}

@@ -227,21 +251,28 @@ function itClientRenders(desc, testFn) {

251 });

252 }

253

230 -function itThrows(desc, testFn) {

254 +function itThrows(desc, testFn, partialMessage) {

255 it(`throws ${desc}`, () => {

256 return testFn().then(

257 () => expect(false).toBe('The promise resolved and should not have.'),

234 - () => {},

258 + err => {

259 + expect(err).toBeInstanceOf(Error);

260 + expect(err.message).toContain(partialMessage);

261 + },

262 );

263 });

264 }

265

239 -function itThrowsWhenRendering(desc, testFn) {

240 - itThrows(`when rendering ${desc} with server string render`, () =>

241 - testFn(serverRender),

266 +function itThrowsWhenRendering(desc, testFn, partialMessage) {

267 + itThrows(

268 + `when rendering ${desc} with server string render`,

269 + () => testFn(serverRender),

270 + partialMessage,

271 );

243 - itThrows(`when rendering ${desc} with clean client render`, () =>

244 - testFn(clientCleanRender),

272 + itThrows(

273 + `when rendering ${desc} with clean client render`,

274 + () => testFn(clientCleanRender),

275 + partialMessage,

276 );

277

278 // we subtract one from the warning count here because the throw means that it won't

@@ -252,6 +283,7 @@ function itThrowsWhenRendering(desc, testFn) {

283 testFn((element, warningCount = 0) =>

284 clientRenderOnBadMarkup(element, warningCount - 1),

285 ),

286 + partialMessage,

287 );

288 }

289

@@ -298,6 +330,7 @@ function resetModules() {

330 // TODO: can we express this test with only public API?

331 ExecutionEnvironment = require('ExecutionEnvironment');

332

333 + require('ReactFeatureFlags').disableNewFiberFeatures = false;

334 PropTypes = require('prop-types');

335 React = require('react');

336 ReactDOM = require('react-dom');

@@ -311,7 +344,7 @@ function resetModules() {

344 if (typeof onAfterResetModules === 'function') {

345 onAfterResetModules();

346 }

314 -

347 + require('ReactFeatureFlags').disableNewFiberFeatures = false;

348 ReactDOMServer = require('react-dom/server');

349

350 // Finally, reset modules one more time before importing the stream renderer.

@@ -319,7 +352,7 @@ function resetModules() {

352 if (typeof onAfterResetModules === 'function') {

353 onAfterResetModules();

354 }

322 -

355 + require('ReactFeatureFlags').disableNewFiberFeatures = false;

356 ReactDOMNodeStream = require('react-dom/node-stream');

357 }

358

@@ -331,19 +364,6 @@ describe('ReactDOMServerIntegration', () => {

364 });

365

366 describe('basic rendering', function() {

334 - beforeEach(() => {

335 - onAfterResetModules = () => {

336 - const ReactFeatureFlags = require('ReactFeatureFlags');

337 - ReactFeatureFlags.disableNewFiberFeatures = false;

338 - };

339 -

340 - resetModules();

341 - });

342 -

343 - afterEach(() => {

344 - onAfterResetModules = null;

345 - });

346 -

367 itRenders('a blank div', async render => {

368 const e = await render(<div />);

369 expect(e.tagName).toBe('DIV');

@@ -361,7 +381,25 @@ describe('ReactDOMServerIntegration', () => {

381 });

382

383 if (ReactDOMFeatureFlags.useFiber) {

364 - itRenders('a array type children as a child', async render => {

384 + itRenders('a string', async render => {

385 + let e = await render('Hello');

386 + expect(e.nodeType).toBe(3);

387 + expect(e.nodeValue).toMatch('Hello');

388 + });

389 +

390 + itRenders('a number', async render => {

391 + let e = await render(42);

392 + expect(e.nodeType).toBe(3);

393 + expect(e.nodeValue).toMatch('42');

394 + });

395 +

396 + itRenders('an array with one child', async render => {

397 + let e = await render([<div key={1}>text1</div>]);

398 + let parent = e.parentNode;

399 + expect(parent.childNodes[0].tagName).toBe('DIV');

400 + });

401 +

402 + itRenders('an array with several children', async render => {

403 let Header = props => {

404 return <p>header</p>;

405 };

@@ -382,13 +420,7 @@ describe('ReactDOMServerIntegration', () => {

420 expect(parent.childNodes[4].tagName).toBe('H3');

421 });

422

385 - itRenders('a single array element children as a child', async render => {

386 - let e = await render([<div key={1}>text1</div>]);

387 - let parent = e.parentNode;

388 - expect(parent.childNodes[0].tagName).toBe('DIV');

389 - });

390 -

391 - itRenders('a nested array children as a child', async render => {

423 + itRenders('a nested array', async render => {

424 let e = await render([

425 [<div key={1}>text1</div>],

426 <span key={1}>text2</span>,

@@ -400,7 +432,7 @@ describe('ReactDOMServerIntegration', () => {

432 expect(parent.childNodes[2].tagName).toBe('P');

433 });

434

403 - itRenders('an iterable as a child', async render => {

435 + itRenders('an iterable', async render => {

436 const threeDivIterable = {

437 '@@iterator': function() {

438 var i = 0;

@@ -801,10 +833,6 @@ describe('ReactDOMServerIntegration', () => {

833 });

834

835 describe('elements and children', function() {

804 - // helper functions.

805 - const TEXT_NODE_TYPE = 3;

806 - const COMMENT_NODE_TYPE = 8;

807 -

836 function expectNode(node, type, value) {

837 expect(node).not.toBe(null);

838 expect(node.nodeType).toBe(type);

@@ -1401,26 +1429,81 @@ describe('ReactDOMServerIntegration', () => {

1429 });

1430

1431 describe('components that throw errors', function() {

1404 - itThrowsWhenRendering('a string component', async render => {

1405 - const StringComponent = () => 'foo';

1406 - await render(<StringComponent />, 1);

1407 - });

1432 + itThrowsWhenRendering(

1433 + 'a function returning undefined',

1434 + async render => {

1435 + const UndefinedComponent = () => undefined;

1436 + await render(<UndefinedComponent />, 1);

1437 + },

1438 + ReactDOMFeatureFlags.useFiber

1439 + ? 'UndefinedComponent(...): Nothing was returned from render. ' +

1440 + 'This usually means a return statement is missing. Or, to ' +

1441 + 'render nothing, return null.'

1442 + : 'A valid React element (or null) must be returned. ' +

1443 + 'You may have returned undefined, an array or some other invalid object.',

1444 + );

1445

1409 - itThrowsWhenRendering('an undefined component', async render => {

1410 - const UndefinedComponent = () => undefined;

1411 - await render(<UndefinedComponent />, 1);

1412 - });

1446 + itThrowsWhenRendering(

1447 + 'a class returning undefined',

1448 + async render => {

1449 + class UndefinedComponent extends React.Component {

1450 + render() {

1451 + return undefined;

1452 + }

1453 + }

1454 + await render(<UndefinedComponent />, 1);

1455 + },

1456 + ReactDOMFeatureFlags.useFiber

1457 + ? 'UndefinedComponent(...): Nothing was returned from render. ' +

1458 + 'This usually means a return statement is missing. Or, to ' +

1459 + 'render nothing, return null.'

1460 + : 'A valid React element (or null) must be returned. ' +

1461 + 'You may have returned undefined, an array or some other invalid object.',

1462 + );

1463

1414 - itThrowsWhenRendering('a number component', async render => {

1415 - const NumberComponent = () => 54;

1416 - await render(<NumberComponent />, 1);

1417 - });

1464 + itThrowsWhenRendering(

1465 + 'a function returning an object',

1466 + async render => {

1467 + const ObjectComponent = () => ({x: 123});

1468 + await render(<ObjectComponent />, 1);

1469 + },

1470 + ReactDOMFeatureFlags.useFiber

1471 + ? 'Objects are not valid as a React child (found: object with keys ' +

1472 + '{x}). If you meant to render a collection of children, use ' +

1473 + 'an array instead.'

1474 + : 'A valid React element (or null) must be returned. ' +

1475 + 'You may have returned undefined, an array or some other invalid object.',

1476 + );

1477

1419 - itThrowsWhenRendering('null', render => render(null));

1420 - itThrowsWhenRendering('false', render => render(false));

1421 - itThrowsWhenRendering('undefined', render => render(undefined));

1422 - itThrowsWhenRendering('number', render => render(30));

1423 - itThrowsWhenRendering('string', render => render('foo'));

1478 + itThrowsWhenRendering(

1479 + 'a class returning an object',

1480 + async render => {

1481 + class ObjectComponent extends React.Component {

1482 + render() {

1483 + return {x: 123};

1484 + }

1485 + }

1486 + await render(<ObjectComponent />, 1);

1487 + },

1488 + ReactDOMFeatureFlags.useFiber

1489 + ? 'Objects are not valid as a React child (found: object with keys ' +

1490 + '{x}). If you meant to render a collection of children, use ' +

1491 + 'an array instead.'

1492 + : 'A valid React element (or null) must be returned. ' +

1493 + 'You may have returned undefined, an array or some other invalid object.',

1494 + );

1495 +

1496 + itThrowsWhenRendering(

1497 + 'top-level object',

1498 + async render => {

1499 + await render({x: 123});

1500 + },

1501 + ReactDOMFeatureFlags.useFiber

1502 + ? 'Objects are not valid as a React child (found: object with keys ' +

1503 + '{x}). If you meant to render a collection of children, use ' +

1504 + 'an array instead.'

1505 + : 'Invalid component element.',

1506 + );

1507 });

1508 });

1509

@@ -2165,7 +2248,7 @@ describe('ReactDOMServerIntegration', () => {

2248 itThrowsWhenRendering(

2249 'if getChildContext exists without childContextTypes',

2250 render => {

2168 - class Component extends React.Component {

2251 + class MyComponent extends React.Component {

2252 render() {

2253 return <div />;

2254 }

@@ -2173,14 +2256,16 @@ describe('ReactDOMServerIntegration', () => {

2256 return {foo: 'bar'};

2257 }

2258 }

2176 - return render(<Component />);

2259 + return render(<MyComponent />);

2260 },

2261 + 'MyComponent.getChildContext(): childContextTypes must be defined ' +

2262 + 'in order to use getChildContext().',

2263 );

2264

2265 itThrowsWhenRendering(

2266 'if getChildContext returns a value not in childContextTypes',

2267 render => {

2183 - class Component extends React.Component {

2268 + class MyComponent extends React.Component {

2269 render() {

2270 return <div />;

2271 }

@@ -2188,9 +2273,10 @@ describe('ReactDOMServerIntegration', () => {

2273 return {value1: 'foo', value2: 'bar'};

2274 }

2275 }

2191 - Component.childContextTypes = {value1: PropTypes.string};

2192 - return render(<Component />);

2276 + MyComponent.childContextTypes = {value1: PropTypes.string};

2277 + return render(<MyComponent />);

2278 },

2279 + 'MyComponent.getChildContext(): key "value2" is not defined in childContextTypes.',

2280 );

2281 });

2282

src/renderers/dom/shared/__tests__/ReactServerRendering-test.js

+2 -2

@@ -327,7 +327,7 @@ describe('ReactDOMServer', () => {

327 ).toThrowError(

328 ReactDOMFeatureFlags.useFiber

329 ? 'Objects are not valid as a React child (found: object with keys {x})'

330 - : 'renderToString(): You must pass a valid ReactElement.',

330 + : 'renderToString(): Invalid component element.',

331 );

332 });

333 });

@@ -443,7 +443,7 @@ describe('ReactDOMServer', () => {

443 ).toThrowError(

444 ReactDOMFeatureFlags.useFiber

445 ? 'Objects are not valid as a React child (found: object with keys {x})'

446 - : 'renderToStaticMarkup(): You must pass a valid ReactElement.',

446 + : 'renderToStaticMarkup(): Invalid component element.',

447 );

448 });

449

src/renderers/dom/stack/server/ReactServerRendering.js

+2 -2

@@ -79,7 +79,7 @@ function renderToStringImpl(element, makeStaticMarkup) {

79 function renderToString(element) {

80 invariant(

81 React.isValidElement(element),

82 - 'renderToString(): You must pass a valid ReactElement.',

82 + 'renderToString(): Invalid component element.',

83 );

84 return renderToStringImpl(element, false);

85 }

@@ -92,7 +92,7 @@ function renderToString(element) {

92 function renderToStaticMarkup(element) {

93 invariant(

94 React.isValidElement(element),

95 - 'renderToStaticMarkup(): You must pass a valid ReactElement.',

95 + 'renderToStaticMarkup(): Invalid component element.',

96 );

97 return renderToStringImpl(element, true);

98 }

src/renderers/shared/server/ReactPartialRenderer.js

+14

@@ -309,6 +309,18 @@ function createOpenTagMarkup(

309 return ret;

310 }

311

312 +function validateRenderResult(child, type) {

313 + if (child === undefined) {

314 + invariant(

315 + false,

316 + '%s(...): Nothing was returned from render. This usually means a ' +

317 + 'return statement is missing. Or, to render nothing, ' +

318 + 'return null.',

319 + getComponentName(type) || 'Component',

320 + );

321 + }

322 +}

323 +

324 function resolve(child, context) {

325 while (React.isValidElement(child)) {

326 if (__DEV__) {

@@ -351,6 +363,7 @@ function resolve(child, context) {

363 inst = Component(child.props, publicContext, updater);

364 if (inst == null || inst.render == null) {

365 child = inst;

366 + validateRenderResult(child, Component);

367 continue;

368 }

369 }

@@ -405,6 +418,7 @@ function resolve(child, context) {

418 child = null;

419 }

420 }

421 + validateRenderResult(child, Component);

422

423 var childContext;

424 if (typeof inst.getChildContext === 'function') {