From 8bf2ed6187ae5de23ae9f12d717cc620be056fff Mon Sep 17 00:00:00 2001 From: Lee Byron Date: Wed, 17 Oct 2018 11:24:38 -0700 Subject: [PATCH] BUG: ReactPartialRenderer / New Context polutes mutable global state The new context API stores the provided values on the shared context instance. When used in a synchronous context, this is not an issue. However when used in an concurrent context this can cause a "push provider" from one react render to have an effect on an unrelated concurrent react render. I've encountered this bug in production when using renderToNodeStream, which asks ReactPartialRenderer for bytes up to a high water mark before yielding. If two Node Streams are created and read from in parallel, the state of one can polute the other. I wrote a failing test to illustrate the conditions under which this happens. I'm also concerned that the experimental concurrent/async React rendering on the client could suffer from the same issue. --- ...eactDOMServerIntegrationNewContext-test.js | 33 +++++++++++++++++++ 1 file changed, 33 insertions(+) diff --git a/packages/react-dom/src/__tests__/ReactDOMServerIntegrationNewContext-test.js b/packages/react-dom/src/__tests__/ReactDOMServerIntegrationNewContext-test.js index af9a6095e0a4..334ff0e2cbd2 100644 --- a/packages/react-dom/src/__tests__/ReactDOMServerIntegrationNewContext-test.js +++ b/packages/react-dom/src/__tests__/ReactDOMServerIntegrationNewContext-test.js @@ -249,5 +249,38 @@ describe('ReactDOMServerIntegration', () => { expect(e.querySelector('#language2').textContent).toBe('sanskrit'); expect(e.querySelector('#language3').textContent).toBe('french'); }); + + it('does not pollute parallel node streams', () => { + const LoggedInUser = React.createContext(); + + const AppWithUser = user => ( + +
+ {whoAmI => whoAmI} +
+
+ {whoAmI => whoAmI} +
+
+ ); + + const streamAmy = ReactDOMServer.renderToNodeStream( + AppWithUser('Amy'), + ).setEncoding('utf8'); + const streamBob = ReactDOMServer.renderToNodeStream( + AppWithUser('Bob'), + ).setEncoding('utf8'); + + // 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 + // highWaterMark boundary of 2^14 chars. + streamAmy._read(20); + streamBob._read(20); + streamAmy._read(20); + streamBob._read(20); + + expect(streamAmy.read()).toBe('
Amy
'); + expect(streamBob.read()).toBe('
Bob
'); + }); }); });