fix transposed escape functions (#25534)
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.
Josh Story committed
Oct 22, 2022 at 09:56 UTC
934177598e583672ac12fbc63288848c2f2930bc
7 files changed
+31
-11
packages/react-dom-bindings/src/server/ReactDOMLegacyServerStreamConfig.js
+2
-2
@@ -12,8 +12,8 @@ export interface Destination {
12
destroy(error: Error): mixed;
13
}
14
15
-export type PrecomputedChunk = string;
16
-export type Chunk = string;
15
+export opaque type PrecomputedChunk = string;
16
+export opaque type Chunk = string;
17
18
export function scheduleWork(callback: () => void) {
19
callback();
packages/react-dom-bindings/src/server/ReactDOMServerFormatConfig.js
+3
-3
@@ -2385,7 +2385,7 @@ export function writeInitialResources(
2385
} else {
2386
target.push(
2387
precedencePlaceholderStart,
2388
- escapeTextForBrowser(stringToChunk(precedence)),
2388
+ stringToChunk(escapeTextForBrowser(precedence)),
2389
precedencePlaceholderEnd,
2390
);
2391
}
@@ -2417,7 +2417,7 @@ export function writeInitialResources(
2417
case 'title': {
2418
pushStartTitleImpl(target, r.props, responseState);
2419
if (typeof r.props.children === 'string') {
2420
- target.push(escapeTextForBrowser(stringToChunk(r.props.children)));
2420
+ target.push(stringToChunk(escapeTextForBrowser(r.props.children)));
2421
}
2422
pushEndInstance(target, target, 'title', r.props);
2423
break;
@@ -2518,7 +2518,7 @@ export function writeImmediateResources(
2518
case 'title': {
2519
pushStartTitleImpl(target, r.props, responseState);
2520
if (typeof r.props.children === 'string') {
2521
- target.push(escapeTextForBrowser(stringToChunk(r.props.children)));
2521
+ target.push(stringToChunk(escapeTextForBrowser(r.props.children)));
2522
}
2523
pushEndInstance(target, target, 'title', r.props);
2524
break;
packages/react-dom-bindings/src/server/escapeTextForBrowser.js
+4
-2
@@ -28,6 +28,8 @@
28
* CLAIM, DAMAGES OR OTHER LIABILITY, WHETHER IN AN ACTION OF CONTRACT,
29
* TORT OR OTHERWISE, ARISING FROM, OUT OF OR IN CONNECTION WITH THE
30
* SOFTWARE OR THE USE OR OTHER DEALINGS IN THE SOFTWARE.
31
+ *
32
+ * @flow
33
*/
34
35
// code copied and modified from escape-html
@@ -103,12 +105,12 @@ function escapeHtml(string) {
105
* @param {*} text Text value to escape.
106
* @return {string} An escaped string.
107
*/
106
-function escapeTextForBrowser(text) {
108
+function escapeTextForBrowser(text: string | number | boolean): string {
109
if (typeof text === 'boolean' || typeof text === 'number') {
110
// this shortcircuit helps perf for types that we know will never have
111
// special characters, especially given that this function is used often
112
// for numeric dom ids.
111
- return '' + text;
113
+ return '' + (text: any);
114
}
115
return escapeHtml(text);
116
}
packages/react-dom/src/__tests__/ReactDOMFizzServerBrowser-test.js
+18
@@ -484,4 +484,22 @@ describe('ReactDOMFizzServerBrowser', () => {
484
485
expect(errors).toEqual(['uh oh', 'uh oh']);
486
});
487
+
488
+ // https://github.com/facebook/react/pull/25534/files - fix transposed escape functions
489
+ // @gate enableFloat
490
+ it('should encode title properly', async () => {
491
+ const stream = await ReactDOMFizzServer.renderToReadableStream(
492
+ <html>
493
+ <head>
494
+ <title>foo</title>
495
+ </head>
496
+ <body>bar</body>
497
+ </html>,
498
+ );
499
+
500
+ const result = await readResult(stream);
501
+ expect(result).toEqual(
502
+ '<!DOCTYPE html><html><head><title>foo</title></title></head><body>bar</body></html>',
503
+ );
504
+ });
505
});
packages/react-server-dom-relay/src/ReactServerStreamConfigFB.js
+2
-2
@@ -14,8 +14,8 @@ export type Destination = {
14
error: mixed,
15
};
16
17
-export type PrecomputedChunk = string;
18
-export type Chunk = string;
17
+export opaque type PrecomputedChunk = string;
18
+export opaque type Chunk = string;
19
20
export function scheduleWork(callback: () => void) {
21
// We don't schedule work in this model, and instead expect performWork to always be called repeatedly.
packages/react-server/src/ReactServerStreamConfigBrowser.js
+1
-1
@@ -10,7 +10,7 @@
10
export type Destination = ReadableStreamController;
11
12
export type PrecomputedChunk = Uint8Array;
13
-export type Chunk = Uint8Array;
13
+export opaque type Chunk = Uint8Array;
14
15
export function scheduleWork(callback: () => void) {
16
callback();
packages/react-server/src/ReactServerStreamConfigNode.js
+1
-1
@@ -17,7 +17,7 @@ interface MightBeFlushable {
17
export type Destination = Writable & MightBeFlushable;
18
19
export type PrecomputedChunk = Uint8Array;
20
-export type Chunk = string;
20
+export opaque type Chunk = string;
21
22
export function scheduleWork(callback: () => void) {
23
setImmediate(callback);