From 88fcd4732794d9580acfdd968a281fff6cfee614 Mon Sep 17 00:00:00 2001 From: Sebastian Markbage Date: Mon, 14 Jun 2021 18:28:00 -0400 Subject: [PATCH 1/2] Clean up partial renderer entry points I made a mistake by leaving server.browser.stable in which is the partial renderer for the browser build of stable. That should use the legacy fizz one. Since the only usage of the partial renderer now is at FB and we don't use it with Node, I removed the Node build of partial renderer too. --- .../react-dom/server.browser.classic.fb.js | 2 +- packages/react-dom/server.browser.stable.js | 16 ------ packages/react-dom/server.node.classic.fb.js | 17 ------ .../src/server/ReactDOMNodeStreamRenderer.js | 55 ------------------- ...tDOMServerLegacyPartialRendererBrowser.js} | 0 .../src/server/ReactDOMServerNode.js | 22 -------- scripts/shared/inlinedHostConfigs.js | 2 - 7 files changed, 1 insertion(+), 113 deletions(-) delete mode 100644 packages/react-dom/server.browser.stable.js delete mode 100644 packages/react-dom/server.node.classic.fb.js delete mode 100644 packages/react-dom/src/server/ReactDOMNodeStreamRenderer.js rename packages/react-dom/src/server/{ReactDOMServerBrowser.js => ReactDOMServerLegacyPartialRendererBrowser.js} (100%) delete mode 100644 packages/react-dom/src/server/ReactDOMServerNode.js diff --git a/packages/react-dom/server.browser.classic.fb.js b/packages/react-dom/server.browser.classic.fb.js index c57063649a9f0..39d975d20e84b 100644 --- a/packages/react-dom/server.browser.classic.fb.js +++ b/packages/react-dom/server.browser.classic.fb.js @@ -13,4 +13,4 @@ export { renderToNodeStream, renderToStaticNodeStream, version, -} from './src/server/ReactDOMServerBrowser'; +} from './src/server/ReactDOMServerLegacyPartialRendererBrowser'; diff --git a/packages/react-dom/server.browser.stable.js b/packages/react-dom/server.browser.stable.js deleted file mode 100644 index c57063649a9f0..0000000000000 --- a/packages/react-dom/server.browser.stable.js +++ /dev/null @@ -1,16 +0,0 @@ -/** - * Copyright (c) Facebook, Inc. and its affiliates. - * - * This source code is licensed under the MIT license found in the - * LICENSE file in the root directory of this source tree. - * - * @flow - */ - -export { - renderToString, - renderToStaticMarkup, - renderToNodeStream, - renderToStaticNodeStream, - version, -} from './src/server/ReactDOMServerBrowser'; diff --git a/packages/react-dom/server.node.classic.fb.js b/packages/react-dom/server.node.classic.fb.js deleted file mode 100644 index e610a8818f961..0000000000000 --- a/packages/react-dom/server.node.classic.fb.js +++ /dev/null @@ -1,17 +0,0 @@ -/** - * Copyright (c) Facebook, Inc. and its affiliates. - * - * This source code is licensed under the MIT license found in the - * LICENSE file in the root directory of this source tree. - * - * @flow - */ - -// For some reason Flow doesn't like export * in this file. I don't know why. -export { - renderToString, - renderToStaticMarkup, - renderToNodeStream, - renderToStaticNodeStream, - version, -} from './src/server/ReactDOMServerNode'; diff --git a/packages/react-dom/src/server/ReactDOMNodeStreamRenderer.js b/packages/react-dom/src/server/ReactDOMNodeStreamRenderer.js deleted file mode 100644 index 6b032556db4d3..0000000000000 --- a/packages/react-dom/src/server/ReactDOMNodeStreamRenderer.js +++ /dev/null @@ -1,55 +0,0 @@ -/** - * Copyright (c) Facebook, Inc. and its affiliates. - * - * This source code is licensed under the MIT license found in the - * LICENSE file in the root directory of this source tree. - */ -import type {ServerOptions} from './ReactPartialRenderer'; - -import {Readable} from 'stream'; - -import ReactPartialRenderer from './ReactPartialRenderer'; - -// This is a Readable Node.js stream which wraps the ReactDOMPartialRenderer. -class ReactMarkupReadableStream extends Readable { - constructor(element, makeStaticMarkup, options) { - // Calls the stream.Readable(options) constructor. Consider exposing built-in - // features like highWaterMark in the future. - super({}); - this.partialRenderer = new ReactPartialRenderer( - element, - makeStaticMarkup, - options, - ); - } - - _destroy(err, callback) { - this.partialRenderer.destroy(); - callback(err); - } - - _read(size) { - try { - this.push(this.partialRenderer.read(size)); - } catch (err) { - this.destroy(err); - } - } -} -/** - * Render a ReactElement to its initial HTML. This should only be used on the - * server. - * See https://reactjs.org/docs/react-dom-server.html#rendertonodestream - */ -export function renderToNodeStream(element, options?: ServerOptions) { - return new ReactMarkupReadableStream(element, false, options); -} - -/** - * Similar to renderToNodeStream, except this doesn't create extra DOM attributes - * such as data-react-id that React uses internally. - * See https://reactjs.org/docs/react-dom-server.html#rendertostaticnodestream - */ -export function renderToStaticNodeStream(element, options?: ServerOptions) { - return new ReactMarkupReadableStream(element, true, options); -} diff --git a/packages/react-dom/src/server/ReactDOMServerBrowser.js b/packages/react-dom/src/server/ReactDOMServerLegacyPartialRendererBrowser.js similarity index 100% rename from packages/react-dom/src/server/ReactDOMServerBrowser.js rename to packages/react-dom/src/server/ReactDOMServerLegacyPartialRendererBrowser.js diff --git a/packages/react-dom/src/server/ReactDOMServerNode.js b/packages/react-dom/src/server/ReactDOMServerNode.js deleted file mode 100644 index 1347434b6900c..0000000000000 --- a/packages/react-dom/src/server/ReactDOMServerNode.js +++ /dev/null @@ -1,22 +0,0 @@ -/** - * Copyright (c) Facebook, Inc. and its affiliates. - * - * This source code is licensed under the MIT license found in the - * LICENSE file in the root directory of this source tree. - */ - -import ReactVersion from 'shared/ReactVersion'; - -import {renderToString, renderToStaticMarkup} from './ReactDOMStringRenderer'; -import { - renderToNodeStream, - renderToStaticNodeStream, -} from './ReactDOMNodeStreamRenderer'; - -export { - renderToString, - renderToStaticMarkup, - renderToNodeStream, - renderToStaticNodeStream, - ReactVersion as version, -}; diff --git a/scripts/shared/inlinedHostConfigs.js b/scripts/shared/inlinedHostConfigs.js index 146bc4d64ffe2..1ec1a250fcae6 100644 --- a/scripts/shared/inlinedHostConfigs.js +++ b/scripts/shared/inlinedHostConfigs.js @@ -20,7 +20,6 @@ module.exports = [ 'react-dom', 'react-dom/unstable-fizz', 'react-dom/unstable-fizz.node', - 'react-dom/server.node.stable', 'react-dom/src/server/ReactDOMFizzServerNode.js', // react-dom/unstable-fizz.node 'react-server-dom-webpack', 'react-server-dom-webpack/writer', @@ -45,7 +44,6 @@ module.exports = [ 'react-dom', 'react-dom/testing', 'react-dom/unstable-fizz.browser', - 'react-dom/server.browser.stable', 'react-dom/src/server/ReactDOMFizzServerBrowser.js', // react-dom/unstable-fizz.browser 'react-server-dom-webpack', 'react-server-dom-webpack/writer.browser.server', From e4e590939c8c442c457874d604eb02f1e9782802 Mon Sep 17 00:00:00 2001 From: Sebastian Markbage Date: Mon, 14 Jun 2021 18:35:45 -0400 Subject: [PATCH 2/2] Remove GC test No code is running this path anymore. Ideally this should be ported to a Fizz form. --- ...eactDOMServerIntegrationNewContext-test.js | 71 ------------------- 1 file changed, 71 deletions(-) diff --git a/packages/react-dom/src/__tests__/ReactDOMServerIntegrationNewContext-test.js b/packages/react-dom/src/__tests__/ReactDOMServerIntegrationNewContext-test.js index 601a42e725240..85c214067531b 100644 --- a/packages/react-dom/src/__tests__/ReactDOMServerIntegrationNewContext-test.js +++ b/packages/react-dom/src/__tests__/ReactDOMServerIntegrationNewContext-test.js @@ -486,77 +486,6 @@ describe('ReactDOMServerIntegration', () => { } }); - // Regression test for https://github.com/facebook/react/issues/14705 - // @gate !experimental && www - it('does not pollute later renders when stream destroyed', () => { - const LoggedInUser = React.createContext('default'); - - const AppWithUser = user => ( - -
- {whoAmI => whoAmI} -
- - ); - - const stream = ReactDOMServer.renderToNodeStream( - AppWithUser('Amy'), - ).setEncoding('utf8'); - - // This is an implementation detail because we test a memory leak - const {threadID} = stream.partialRenderer; - - // Read enough to render Provider but not enough for it to be exited - stream._read(10); - expect(LoggedInUser[threadID]).toBe('Amy'); - - stream.destroy(); - - const AppWithUserNoProvider = () => ( - {whoAmI => whoAmI} - ); - - const stream2 = ReactDOMServer.renderToNodeStream( - AppWithUserNoProvider(), - ).setEncoding('utf8'); - - // Sanity check to ensure 2nd render has same threadID as 1st render, - // otherwise this test is not testing what it's meant to - expect(stream2.partialRenderer.threadID).toBe(threadID); - - const markup = stream2.read(Infinity); - - expect(markup).toBe('default'); - }); - - // Regression test for https://github.com/facebook/react/issues/14705 - // @gate !experimental && www - it('frees context value reference when stream destroyed', () => { - const LoggedInUser = React.createContext('default'); - - const AppWithUser = user => ( - -
- {whoAmI => whoAmI} -
- - ); - - const stream = ReactDOMServer.renderToNodeStream( - AppWithUser('Amy'), - ).setEncoding('utf8'); - - // This is an implementation detail because we test a memory leak - const {threadID} = stream.partialRenderer; - - // Read enough to render Provider but not enough for it to be exited - stream._read(10); - expect(LoggedInUser[threadID]).toBe('Amy'); - - stream.destroy(); - expect(LoggedInUser[threadID]).toBe('default'); - }); - it('does not pollute sync renders after an error', () => { const LoggedInUser = React.createContext('default'); const Crash = () => {