From 6a60b5d7a49f633e89a4e46db7d13f510efc7101 Mon Sep 17 00:00:00 2001 From: Dan Abramov Date: Thu, 5 Oct 2017 13:24:18 +0100 Subject: [PATCH 1/4] Add a failing test for text nodes within components --- .../ReactDOMServerIntegration-test.js | 24 +++++++++++++++++++ 1 file changed, 24 insertions(+) diff --git a/src/renderers/dom/shared/__tests__/ReactDOMServerIntegration-test.js b/src/renderers/dom/shared/__tests__/ReactDOMServerIntegration-test.js index 642e99a0e06db..51da0f2041358 100644 --- a/src/renderers/dom/shared/__tests__/ReactDOMServerIntegration-test.js +++ b/src/renderers/dom/shared/__tests__/ReactDOMServerIntegration-test.js @@ -1117,6 +1117,30 @@ describe('ReactDOMServerIntegration', () => { expectTextNode(e.childNodes[1], 'bar'); } }); + + itRenders( + 'a component returning text node between two text nodes', + async render => { + const B = () => 'b'; + const e = await render(
{'a'}{'c'}
); + if ( + render === serverRender || + render === clientRenderOnServerString || + render === streamRender + ) { + // In the server render output there's a comment between them. + expect(e.childNodes.length).toBe(5); + expectTextNode(e.childNodes[0], 'a'); + expectTextNode(e.childNodes[2], 'b'); + expectTextNode(e.childNodes[4], 'c'); + } else { + expect(e.childNodes.length).toBe(3); + expectTextNode(e.childNodes[0], 'a'); + expectTextNode(e.childNodes[1], 'b'); + expectTextNode(e.childNodes[2], 'c'); + } + }, + ); }); describe('number children', function() { From fd4d0a7f25c94547a4e4ad1eb9dd7d3cbeac4aa0 Mon Sep 17 00:00:00 2001 From: Dan Abramov Date: Thu, 5 Oct 2017 13:25:05 +0100 Subject: [PATCH 2/4] Reset the text node flag only when emitting tags This ensures we don't reset it when we exit components that return strings. --- src/renderers/shared/server/ReactPartialRenderer.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/renderers/shared/server/ReactPartialRenderer.js b/src/renderers/shared/server/ReactPartialRenderer.js index 5d128fe84da87..9a241a6a6a3c0 100644 --- a/src/renderers/shared/server/ReactPartialRenderer.js +++ b/src/renderers/shared/server/ReactPartialRenderer.js @@ -476,7 +476,6 @@ class ReactDOMServerRenderer { var frame = this.stack[this.stack.length - 1]; if (frame.childIndex >= frame.children.length) { out += frame.footer; - this.previousWasTextNode = false; this.stack.pop(); if (frame.tag === 'select') { this.currentSelectValue = null; @@ -827,6 +826,7 @@ class ReactDOMServerRenderer { frame.debugElementStack = []; } this.stack.push(frame); + this.previousWasTextNode = false; return out; } } From 0f5910bfea59e457e354f2e65ed4bc8b8ab9846f Mon Sep 17 00:00:00 2001 From: Dan Abramov Date: Thu, 5 Oct 2017 13:59:14 +0100 Subject: [PATCH 3/4] Add a failing test for a more complex tree --- .../ReactDOMServerIntegration-test.js | 49 +++++++++++++++++++ 1 file changed, 49 insertions(+) diff --git a/src/renderers/dom/shared/__tests__/ReactDOMServerIntegration-test.js b/src/renderers/dom/shared/__tests__/ReactDOMServerIntegration-test.js index 51da0f2041358..549d39858ce1d 100644 --- a/src/renderers/dom/shared/__tests__/ReactDOMServerIntegration-test.js +++ b/src/renderers/dom/shared/__tests__/ReactDOMServerIntegration-test.js @@ -1141,6 +1141,55 @@ describe('ReactDOMServerIntegration', () => { } }, ); + + itRenders('a tree with sibling host and text nodes', async render => { + class X extends React.Component { + render() { + return [null, [], false]; + } + } + + function Y() { + return [, ['c']]; + } + + function Z() { + return null; + } + + const e = await render( +
+ {[['a'], 'b']} +
+ + d +
+ e +
, + ); + if ( + render === serverRender || + render === clientRenderOnServerString || + render === streamRender + ) { + // In the server render output there's comments between text nodes. + expect(e.childNodes.length).toBe(5); + expectTextNode(e.childNodes[0], 'a'); + expectTextNode(e.childNodes[2], 'b'); + expect(e.childNodes[3].childNodes.length).toBe(3); + expectTextNode(e.childNodes[3].childNodes[0], 'c'); + expectTextNode(e.childNodes[3].childNodes[2], 'd'); + expectTextNode(e.childNodes[4], 'e'); + } else { + expect(e.childNodes.length).toBe(4); + expectTextNode(e.childNodes[0], 'a'); + expectTextNode(e.childNodes[1], 'b'); + expect(e.childNodes[2].childNodes.length).toBe(2); + expectTextNode(e.childNodes[2].childNodes[0], 'c'); + expectTextNode(e.childNodes[2].childNodes[1], 'd'); + expectTextNode(e.childNodes[3], 'e'); + } + }); }); describe('number children', function() { From cd2df6ce9e2fe9e4b2bc2a0352f7c32db0c14a3e Mon Sep 17 00:00:00 2001 From: Dan Abramov Date: Thu, 5 Oct 2017 13:59:50 +0100 Subject: [PATCH 4/4] Also reset text flag when emitting a footer This fixes the {'a'}{'b'} case and prevents an unnecessary comment before 'b'. --- src/renderers/shared/server/ReactPartialRenderer.js | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/src/renderers/shared/server/ReactPartialRenderer.js b/src/renderers/shared/server/ReactPartialRenderer.js index 9a241a6a6a3c0..facf1c2bc88b9 100644 --- a/src/renderers/shared/server/ReactPartialRenderer.js +++ b/src/renderers/shared/server/ReactPartialRenderer.js @@ -475,7 +475,11 @@ class ReactDOMServerRenderer { } var frame = this.stack[this.stack.length - 1]; if (frame.childIndex >= frame.children.length) { - out += frame.footer; + var footer = frame.footer; + out += footer; + if (footer !== '') { + this.previousWasTextNode = false; + } this.stack.pop(); if (frame.tag === 'select') { this.currentSelectValue = null;