fix transposed escape functions (#25534)

gnoff · web-flow · commit 934177598e58 · 2022-10-22T09:56:40.000-07:00
escapeTextForBrowser accepts any type so flow did not identify that we
were escaping a Chunk rather than a string. It's tricky because we
sometimes want to be able to escape non strings.

I've also updated the types for `Chunk` and `escapeTextForBrowser`
so that we should be able to catch this statically in the future.

The reason this did not show up in tests is almost all of our tests of
float (the areas affected by transpositions) are tested using the Node
runtime where a chunk type is a string. It may be wise to run these
tests in every runtime in the future or at least make sure there is
broad representation of resources in each specific runtime test suite.
diff --git a/packages/react-dom-bindings/src/server/ReactDOMLegacyServerStreamConfig.js b/packages/react-dom-bindings/src/server/ReactDOMLegacyServerStreamConfig.js
@@ -12,8 +12,8 @@ export interface Destination {
   destroy(error: Error): mixed;
 }
 
-export type PrecomputedChunk = string;
-export type Chunk = string;
+export opaque type PrecomputedChunk = string;
+export opaque type Chunk = string;
 
 export function scheduleWork(callback: () => void) {
   callback();
diff --git a/packages/react-dom-bindings/src/server/ReactDOMServerFormatConfig.js b/packages/react-dom-bindings/src/server/ReactDOMServerFormatConfig.js
@@ -2385,7 +2385,7 @@ export function writeInitialResources(
     } else {
       target.push(
         precedencePlaceholderStart,
-        escapeTextForBrowser(stringToChunk(precedence)),
+        stringToChunk(escapeTextForBrowser(precedence)),
         precedencePlaceholderEnd,
       );
     }
@@ -2417,7 +2417,7 @@ export function writeInitialResources(
       case 'title': {
         pushStartTitleImpl(target, r.props, responseState);
         if (typeof r.props.children === 'string') {
-          target.push(escapeTextForBrowser(stringToChunk(r.props.children)));
+          target.push(stringToChunk(escapeTextForBrowser(r.props.children)));
         }
         pushEndInstance(target, target, 'title', r.props);
         break;
@@ -2518,7 +2518,7 @@ export function writeImmediateResources(
       case 'title': {
         pushStartTitleImpl(target, r.props, responseState);
         if (typeof r.props.children === 'string') {
-          target.push(escapeTextForBrowser(stringToChunk(r.props.children)));
+          target.push(stringToChunk(escapeTextForBrowser(r.props.children)));
         }
         pushEndInstance(target, target, 'title', r.props);
         break;
diff --git a/packages/react-dom-bindings/src/server/escapeTextForBrowser.js b/packages/react-dom-bindings/src/server/escapeTextForBrowser.js
@@ -28,6 +28,8 @@
  * CLAIM, DAMAGES OR OTHER LIABILITY, WHETHER IN AN ACTION OF CONTRACT,
  * TORT OR OTHERWISE, ARISING FROM, OUT OF OR IN CONNECTION WITH THE
  * SOFTWARE OR THE USE OR OTHER DEALINGS IN THE SOFTWARE.
+ *
+ * @flow
  */
 
 // code copied and modified from escape-html
@@ -103,12 +105,12 @@ function escapeHtml(string) {
  * @param {*} text Text value to escape.
  * @return {string} An escaped string.
  */
-function escapeTextForBrowser(text) {
+function escapeTextForBrowser(text: string | number | boolean): string {
   if (typeof text === 'boolean' || typeof text === 'number') {
     // this shortcircuit helps perf for types that we know will never have
     // special characters, especially given that this function is used often
     // for numeric dom ids.
-    return '' + text;
+    return '' + (text: any);
   }
   return escapeHtml(text);
 }
diff --git a/packages/react-dom/src/__tests__/ReactDOMFizzServerBrowser-test.js b/packages/react-dom/src/__tests__/ReactDOMFizzServerBrowser-test.js
@@ -484,4 +484,22 @@ describe('ReactDOMFizzServerBrowser', () => {
 
     expect(errors).toEqual(['uh oh', 'uh oh']);
   });
+
+  // https://github.com/facebook/react/pull/25534/files - fix transposed escape functions
+  // @gate enableFloat
+  it('should encode title properly', async () => {
+    const stream = await ReactDOMFizzServer.renderToReadableStream(
+      <html>
+        <head>
+          <title>foo</title>
+        </head>
+        <body>bar</body>
+      </html>,
+    );
+
+    const result = await readResult(stream);
+    expect(result).toEqual(
+      '<!DOCTYPE html><html><head><title>foo</title></title></head><body>bar</body></html>',
+    );
+  });
 });
diff --git a/packages/react-server-dom-relay/src/ReactServerStreamConfigFB.js b/packages/react-server-dom-relay/src/ReactServerStreamConfigFB.js
@@ -14,8 +14,8 @@ export type Destination = {
   error: mixed,
 };
 
-export type PrecomputedChunk = string;
-export type Chunk = string;
+export opaque type PrecomputedChunk = string;
+export opaque type Chunk = string;
 
 export function scheduleWork(callback: () => void) {
   // We don't schedule work in this model, and instead expect performWork to always be called repeatedly.
diff --git a/packages/react-server/src/ReactServerStreamConfigBrowser.js b/packages/react-server/src/ReactServerStreamConfigBrowser.js
@@ -10,7 +10,7 @@
 export type Destination = ReadableStreamController;
 
 export type PrecomputedChunk = Uint8Array;
-export type Chunk = Uint8Array;
+export opaque type Chunk = Uint8Array;
 
 export function scheduleWork(callback: () => void) {
   callback();
diff --git a/packages/react-server/src/ReactServerStreamConfigNode.js b/packages/react-server/src/ReactServerStreamConfigNode.js
@@ -17,7 +17,7 @@ interface MightBeFlushable {
 export type Destination = Writable & MightBeFlushable;
 
 export type PrecomputedChunk = Uint8Array;
-export type Chunk = string;
+export opaque type Chunk = string;
 
 export function scheduleWork(callback: () => void) {
   setImmediate(callback);
