Fix invariant parity for SSR #10333
Fix invariant parity for SSR #10333
Conversation
| @@ -45,7 +45,7 @@ function renderToStream(element) { | |||
| if (disableNewFiberFeatures) { | |||
| invariant( | |||
| React.isValidElement(element), | |||
| 'renderToStream(): You must pass a valid ReactElement.', | |||
| 'renderToStream(): Invalid component element.', | |||
There was a problem hiding this comment.
Changed these for consistency with ReactDOM.render message. I'm not sure if it mattered in all cases (e.g. these ones are behind a flag that's now false in integration suite) but I changed them all for consistency.
| let clientNode = firstClientNode; | ||
|
|
||
| // Make sure all top level nodes match up | ||
| while (serverNode || clientNode) { |
There was a problem hiding this comment.
We used to only check the first one.
| // text node, but otherwise works. | ||
| // TODO: we can remove this branch if we add explicit hydration opt-in. | ||
| // https://github.com/facebook/react/issues/10189 | ||
| expect(serverNode.nodeValue).toBe(clientNode.nodeValue); |
There was a problem hiding this comment.
Special case because shouldReuseContent() bails out. Happy to change this to use hydration API per conversation with @sebmarkbage, but I'd like to do this as followup.
| @@ -227,21 +251,28 @@ function itClientRenders(desc, testFn) { | |||
| }); | |||
| } | |||
|
|
|||
| function itThrows(desc, testFn) { | |||
| function itThrows(desc, testFn, partialMessage) { | |||
There was a problem hiding this comment.
Adding this to every error. Helped uncover cases where we crashed instead of throwing something useful.
| @@ -298,6 +330,7 @@ function resetModules() { | |||
| // TODO: can we express this test with only public API? | |||
| ExecutionEnvironment = require('ExecutionEnvironment'); | |||
|
|
|||
| require('ReactFeatureFlags').disableNewFiberFeatures = false; | |||
There was a problem hiding this comment.
New features are now enabled for all fixtures in this file. We don't ship "no new features" SSR builds anymore.
| @@ -361,7 +381,25 @@ describe('ReactDOMServerIntegration', () => { | |||
| }); | |||
|
|
|||
| if (ReactDOMFeatureFlags.useFiber) { | |||
| itRenders('a array type children as a child', async render => { | |||
| itRenders('a string', async render => { | |||
There was a problem hiding this comment.
Added tests for top level string and number, and renamed array tests.
| @@ -1401,26 +1429,81 @@ describe('ReactDOMServerIntegration', () => { | |||
| }); | |||
|
|
|||
| describe('components that throw errors', function() { | |||
| itThrowsWhenRendering('a string component', async render => { | |||
There was a problem hiding this comment.
This is now valid.
|
|
||
| itThrowsWhenRendering('a number component', async render => { |
There was a problem hiding this comment.
This is now valid too.
There's a few different things here that was a bit difficult to untangle.
The goals were:
I'll add more inline comments for specific changes.