Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Deprecate renderToNodeStream (and fix textarea bug) #23359

Merged
merged 4 commits into from
Feb 25, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -101,7 +101,7 @@ describe('ReactDOMServerIntegration', () => {
) {
// For plain server markup result we have comments between.
// If we're able to hydrate, they remain.
expect(e.childNodes.length).toBe(5);
expect(e.childNodes.length).toBe(render === streamRender ? 6 : 5);
expectTextNode(e.childNodes[0], ' ');
expectTextNode(e.childNodes[2], ' ');
expectTextNode(e.childNodes[4], ' ');
Expand All @@ -119,8 +119,8 @@ describe('ReactDOMServerIntegration', () => {
Text<span>More Text</span>
</div>,
);
expect(e.childNodes.length).toBe(2);
const spanNode = e.childNodes[1];
expect(e.childNodes.length).toBe(render === streamRender ? 3 : 2);
const spanNode = e.childNodes[render === streamRender ? 2 : 1];
expectTextNode(e.childNodes[0], 'Text');
expect(spanNode.tagName).toBe('SPAN');
expect(spanNode.childNodes.length).toBe(1);
Expand All @@ -147,19 +147,19 @@ describe('ReactDOMServerIntegration', () => {
itRenders('a custom element with text', async render => {
const e = await render(<custom-element>Text</custom-element>);
expect(e.tagName).toBe('CUSTOM-ELEMENT');
expect(e.childNodes.length).toBe(1);
expect(e.childNodes.length).toBe(render === streamRender ? 2 : 1);
expectNode(e.firstChild, TEXT_NODE_TYPE, 'Text');
});

itRenders('a leading blank child with a text sibling', async render => {
const e = await render(<div>{''}foo</div>);
expect(e.childNodes.length).toBe(1);
expect(e.childNodes.length).toBe(render === streamRender ? 2 : 1);
expectTextNode(e.childNodes[0], 'foo');
});

itRenders('a trailing blank child with a text sibling', async render => {
const e = await render(<div>foo{''}</div>);
expect(e.childNodes.length).toBe(1);
expect(e.childNodes.length).toBe(render === streamRender ? 2 : 1);
expectTextNode(e.childNodes[0], 'foo');
});

Expand All @@ -176,7 +176,7 @@ describe('ReactDOMServerIntegration', () => {
render === streamRender
) {
// In the server render output there's a comment between them.
expect(e.childNodes.length).toBe(3);
expect(e.childNodes.length).toBe(render === streamRender ? 4 : 3);
expectTextNode(e.childNodes[0], 'foo');
expectTextNode(e.childNodes[2], 'bar');
} else {
Expand All @@ -203,7 +203,7 @@ describe('ReactDOMServerIntegration', () => {
render === streamRender
) {
// In the server render output there's a comment between them.
expect(e.childNodes.length).toBe(5);
expect(e.childNodes.length).toBe(render === streamRender ? 6 : 5);
expectTextNode(e.childNodes[0], 'a');
expectTextNode(e.childNodes[2], 'b');
expectTextNode(e.childNodes[4], 'c');
Expand Down Expand Up @@ -240,11 +240,7 @@ describe('ReactDOMServerIntegration', () => {
e
</div>,
);
if (
render === serverRender ||
render === clientRenderOnServerString ||
render === streamRender
) {
if (render === serverRender || render === clientRenderOnServerString) {
// In the server render output there's comments between text nodes.
expect(e.childNodes.length).toBe(5);
expectTextNode(e.childNodes[0], 'a');
Expand All @@ -253,6 +249,15 @@ describe('ReactDOMServerIntegration', () => {
expectTextNode(e.childNodes[3].childNodes[0], 'c');
expectTextNode(e.childNodes[3].childNodes[2], 'd');
expectTextNode(e.childNodes[4], 'e');
} else if (render === streamRender) {
// In the server render output there's comments after each text node.
expect(e.childNodes.length).toBe(7);
expectTextNode(e.childNodes[0], 'a');
expectTextNode(e.childNodes[2], 'b');
expect(e.childNodes[4].childNodes.length).toBe(4);
expectTextNode(e.childNodes[4].childNodes[0], 'c');
expectTextNode(e.childNodes[4].childNodes[2], 'd');
expectTextNode(e.childNodes[5], 'e');
} else {
expect(e.childNodes.length).toBe(4);
expectTextNode(e.childNodes[0], 'a');
Expand Down Expand Up @@ -291,7 +296,7 @@ describe('ReactDOMServerIntegration', () => {
render === streamRender
) {
// In the server markup there's a comment between.
expect(e.childNodes.length).toBe(3);
expect(e.childNodes.length).toBe(render === streamRender ? 4 : 3);
expectTextNode(e.childNodes[0], 'foo');
expectTextNode(e.childNodes[2], '40');
} else {
Expand Down Expand Up @@ -330,13 +335,13 @@ describe('ReactDOMServerIntegration', () => {

itRenders('null children as blank', async render => {
const e = await render(<div>{null}foo</div>);
expect(e.childNodes.length).toBe(1);
expect(e.childNodes.length).toBe(render === streamRender ? 2 : 1);
expectTextNode(e.childNodes[0], 'foo');
});

itRenders('false children as blank', async render => {
const e = await render(<div>{false}foo</div>);
expect(e.childNodes.length).toBe(1);
expect(e.childNodes.length).toBe(render === streamRender ? 2 : 1);
expectTextNode(e.childNodes[0], 'foo');
});

Expand All @@ -348,7 +353,7 @@ describe('ReactDOMServerIntegration', () => {
{false}
</div>,
);
expect(e.childNodes.length).toBe(1);
expect(e.childNodes.length).toBe(render === streamRender ? 2 : 1);
expectTextNode(e.childNodes[0], 'foo');
});

Expand Down Expand Up @@ -735,10 +740,10 @@ describe('ReactDOMServerIntegration', () => {
</div>,
);
expect(e.id).toBe('parent');
expect(e.childNodes.length).toBe(3);
expect(e.childNodes.length).toBe(render === streamRender ? 4 : 3);
const child1 = e.childNodes[0];
const textNode = e.childNodes[1];
const child2 = e.childNodes[2];
const child2 = e.childNodes[render === streamRender ? 3 : 2];
expect(child1.id).toBe('child1');
expect(child1.childNodes.length).toBe(0);
expectTextNode(textNode, ' ');
Expand All @@ -752,10 +757,10 @@ describe('ReactDOMServerIntegration', () => {
async render => {
// prettier-ignore
const e = await render(<div id="parent"> <div id="child" /> </div>); // eslint-disable-line no-multi-spaces
expect(e.childNodes.length).toBe(3);
expect(e.childNodes.length).toBe(render === streamRender ? 5 : 3);
const textNode1 = e.childNodes[0];
const child = e.childNodes[1];
const textNode2 = e.childNodes[2];
const child = e.childNodes[render === streamRender ? 2 : 1];
const textNode2 = e.childNodes[render === streamRender ? 3 : 2];
expect(e.id).toBe('parent');
expectTextNode(textNode1, ' ');
expect(child.id).toBe('child');
Expand All @@ -778,7 +783,9 @@ describe('ReactDOMServerIntegration', () => {
) {
// For plain server markup result we have comments between.
// If we're able to hydrate, they remain.
expect(parent.childNodes.length).toBe(5);
expect(parent.childNodes.length).toBe(
render === streamRender ? 6 : 5,
);
expectTextNode(parent.childNodes[0], 'a');
expectTextNode(parent.childNodes[2], 'b');
expectTextNode(parent.childNodes[4], 'c');
Expand Down Expand Up @@ -810,7 +817,7 @@ describe('ReactDOMServerIntegration', () => {
render === clientRenderOnServerString ||
render === streamRender
) {
expect(e.childNodes.length).toBe(3);
expect(e.childNodes.length).toBe(render === streamRender ? 4 : 3);
expectTextNode(e.childNodes[0], '<span>Text1&quot;</span>');
expectTextNode(e.childNodes[2], '<span>Text2&quot;</span>');
} else {
Expand Down Expand Up @@ -861,7 +868,7 @@ describe('ReactDOMServerIntegration', () => {
);
if (render === serverRender || render === streamRender) {
// We have three nodes because there is a comment between them.
expect(e.childNodes.length).toBe(3);
expect(e.childNodes.length).toBe(render === streamRender ? 4 : 3);
// Everything becomes LF when parsed from server HTML.
// Null character is ignored.
expectNode(e.childNodes[0], TEXT_NODE_TYPE, 'foo\nbar');
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -409,12 +409,24 @@ describe('ReactDOMServerIntegration', () => {
</LoggedInUser.Provider>
);

const streamAmy = ReactDOMServer.renderToNodeStream(
AppWithUser('Amy'),
).setEncoding('utf8');
const streamBob = ReactDOMServer.renderToNodeStream(
AppWithUser('Bob'),
).setEncoding('utf8');
let streamAmy;
let streamBob;
expect(() => {
streamAmy = ReactDOMServer.renderToNodeStream(
AppWithUser('Amy'),
).setEncoding('utf8');
}).toErrorDev(
'renderToNodeStream is deprecated. Use renderToPipeableStream instead.',
{withoutStack: true},
);
expect(() => {
streamBob = ReactDOMServer.renderToNodeStream(
AppWithUser('Bob'),
).setEncoding('utf8');
}).toErrorDev(
'renderToNodeStream is deprecated. Use renderToPipeableStream instead.',
{withoutStack: true},
);

// Testing by filling the buffer using internal _read() with a small
// number of bytes to avoid a test case which needs to align to a
Expand Down Expand Up @@ -449,9 +461,14 @@ describe('ReactDOMServerIntegration', () => {
const streamCount = 34;

for (let i = 0; i < streamCount; i++) {
streams[i] = ReactDOMServer.renderToNodeStream(
NthRender(i % 2 === 0 ? 'Expected to be recreated' : i),
).setEncoding('utf8');
expect(() => {
streams[i] = ReactDOMServer.renderToNodeStream(
NthRender(i % 2 === 0 ? 'Expected to be recreated' : i),
).setEncoding('utf8');
}).toErrorDev(
'renderToNodeStream is deprecated. Use renderToPipeableStream instead.',
{withoutStack: true},
);
}

// Testing by filling the buffer using internal _read() with a small
Expand All @@ -468,9 +485,14 @@ describe('ReactDOMServerIntegration', () => {

// Recreate those same streams.
for (let i = 0; i < streamCount; i += 2) {
streams[i] = ReactDOMServer.renderToNodeStream(
NthRender(i),
).setEncoding('utf8');
expect(() => {
streams[i] = ReactDOMServer.renderToNodeStream(
NthRender(i),
).setEncoding('utf8');
}).toErrorDev(
'renderToNodeStream is deprecated. Use renderToPipeableStream instead.',
{withoutStack: true},
);
}

// Read a bit from all streams again.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -49,6 +49,12 @@ describe('ReactDOMServerIntegrationTextarea', () => {
expect(e.value).toBe('foo');
});

itRenders('a textarea with a value of undefined', async render => {
const e = await render(<textarea value={undefined} />);
expect(e.getAttribute('value')).toBe(null);
expect(e.value).toBe('');
});

itRenders('a textarea with a value and readOnly', async render => {
const e = await render(<textarea value="foo" readOnly={true} />);
// textarea DOM elements don't have a value **attribute**, the text is
Expand Down
16 changes: 14 additions & 2 deletions packages/react-dom/src/__tests__/ReactServerRendering-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -569,15 +569,27 @@ describe('ReactDOMServer', () => {
describe('renderToNodeStream', () => {
it('should generate simple markup', () => {
const SuccessfulElement = React.createElement(() => <img />);
const response = ReactDOMServer.renderToNodeStream(SuccessfulElement);
let response;
expect(() => {
response = ReactDOMServer.renderToNodeStream(SuccessfulElement);
}).toErrorDev(
'renderToNodeStream is deprecated. Use renderToPipeableStream instead.',
{withoutStack: true},
);
expect(response.read().toString()).toMatch(new RegExp('<img' + '/>'));
});

it('should handle errors correctly', () => {
const FailingElement = React.createElement(() => {
throw new Error('An Error');
});
const response = ReactDOMServer.renderToNodeStream(FailingElement);
let response;
expect(() => {
response = ReactDOMServer.renderToNodeStream(FailingElement);
}).toErrorDev(
'renderToNodeStream is deprecated. Use renderToPipeableStream instead.',
{withoutStack: true},
);
return new Promise(resolve => {
response.once('error', () => {
resolve();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -154,8 +154,11 @@ module.exports = function(initModules) {
() =>
new Promise((resolve, reject) => {
const writable = new DrainWritable();
const s = ReactDOMServer.renderToNodeStream(reactElement);
s.on('error', e => reject(e));
const s = ReactDOMServer.renderToPipeableStream(reactElement, {
onErrorShell(e) {
reject(e);
},
});
s.pipe(writable);
writable.on('finish', () => resolve(writable.buffer));
}),
Expand All @@ -168,7 +171,12 @@ module.exports = function(initModules) {
// Does not render on client or perform client-side revival.
async function streamRender(reactElement, errorCount = 0) {
const markup = await renderIntoStream(reactElement, errorCount);
return getContainerFromMarkup(reactElement, markup).firstChild;
let firstNode = getContainerFromMarkup(reactElement, markup).firstChild;
if (firstNode && firstNode.nodeType === Node.DOCUMENT_TYPE_NODE) {
// Skip document type nodes.
firstNode = firstNode.nextSibling;
}
return firstNode;
}

const clientCleanRender = (element, errorCount = 0) => {
Expand Down
5 changes: 5 additions & 0 deletions packages/react-dom/src/server/ReactDOMLegacyServerNode.js
Original file line number Diff line number Diff line change
Expand Up @@ -93,6 +93,11 @@ function renderToNodeStream(
children: ReactNodeList,
options?: ServerOptions,
): Readable {
if (__DEV__) {
console.error(
'renderToNodeStream is deprecated. Use renderToPipeableStream instead.',
);
}
return renderToNodeStreamImpl(children, options, false);
}

Expand Down
12 changes: 11 additions & 1 deletion packages/react-dom/src/server/ReactDOMServerFormatConfig.js
Original file line number Diff line number Diff line change
Expand Up @@ -1001,7 +1001,17 @@ function pushStartTextArea(
target.push(leadingNewline);
}

return value;
// ToString and push directly instead of recurse over children.
// We don't really support complex children in the value anyway.
// This also currently avoids a trailing comment node which breaks textarea.
if (value !== null) {
if (__DEV__) {
checkAttributeStringCoercion(value, 'value');
}
target.push(stringToChunk(encodeHTMLTextNode('' + value)));
}

return null;
}

function pushSelfClosing(
Expand Down