Skip to content

Commit 9c2e2b8

Browse files
unstubbableunstubbable
authored
[Flight] Don't drop debug info if there's only a readable debug channel (facebook#34304)
When the Flight Client is waiting for pending debug chunks, it drops the debug info if there is no writable side of the debug channel defined. However, it should instead check if there's no readable side defined. Fixing this is not only important for browser clients that don't want or need a return channel, but it's also crucial for server-side rendering, because the Node and Edge clients only accept a readable side of the debug channel. So they can't even define a noop writable side as a workaround.
1 parent 4123f6b commit 9c2e2b8

18 files changed

+763
-105
lines changed

‎packages/react-client/src/ReactFlightClient.js‎

Lines changed: 29 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -341,6 +341,11 @@ export type FindSourceMapURLCallback = (
341341

342342
export type DebugChannelCallback = (message: string) => void;
343343

344+
export type DebugChannel = {
345+
hasReadable: boolean,
346+
callback: DebugChannelCallback | null,
347+
};
348+
344349
type Response = {
345350
_bundlerConfig: ServerConsumerModuleMap,
346351
_serverReferenceConfig: null | ServerManifest,
@@ -362,7 +367,7 @@ type Response = {
362367
_debugRootStack?: null | Error, // DEV-only
363368
_debugRootTask?: null | ConsoleTask, // DEV-only
364369
_debugFindSourceMapURL?: void | FindSourceMapURLCallback, // DEV-only
365-
_debugChannel?: void | DebugChannelCallback, // DEV-only
370+
_debugChannel?: void | DebugChannel, // DEV-only
366371
_blockedConsole?: null | SomeChunk<ConsoleEntry>, // DEV-only
367372
_replayConsole: boolean, // DEV-only
368373
_rootEnvironmentName: string, // DEV-only, the requested environment name.
@@ -404,16 +409,16 @@ function getWeakResponse(response: Response): WeakResponse {
404409
}
405410
}
406411

407-
function cleanupDebugChannel(debugChannel: DebugChannelCallback): void {
408-
// When a Response gets GC:ed because nobody is referring to any of the objects that lazily
409-
// loads from the Response anymore, then we can close the debug channel.
410-
debugChannel('');
412+
function closeDebugChannel(debugChannel: DebugChannel): void {
413+
if (debugChannel.callback) {
414+
debugChannel.callback('');
415+
}
411416
}
412417

413418
// If FinalizationRegistry doesn't exist, we cannot use the debugChannel.
414419
const debugChannelRegistry =
415420
__DEV__ && typeof FinalizationRegistry === 'function'
416-
? new FinalizationRegistry(cleanupDebugChannel)
421+
? new FinalizationRegistry(closeDebugChannel)
417422
: null;
418423

419424
function readChunk<T>(chunk: SomeChunk<T>): T {
@@ -1007,7 +1012,7 @@ export function reportGlobalError(
10071012
if (debugChannel !== undefined) {
10081013
// If we don't have any more ways of reading data, we don't have to send any
10091014
// more neither. So we close the writable side.
1010-
debugChannel('');
1015+
closeDebugChannel(debugChannel);
10111016
response._debugChannel = undefined;
10121017
}
10131018
}
@@ -1494,8 +1499,8 @@ function waitForReference<T>(
14941499
): T {
14951500
if (
14961501
__DEV__ &&
1497-
// TODO: This should check for the existence of the "readable" side, not the "writable".
1498-
response._debugChannel === undefined
1502+
(response._debugChannel === undefined ||
1503+
!response._debugChannel.hasReadable)
14991504
) {
15001505
if (
15011506
referencedChunk.status === PENDING &&
@@ -2262,15 +2267,16 @@ function parseModelString(
22622267
case 'Y': {
22632268
if (__DEV__) {
22642269
if (value.length > 2) {
2265-
const debugChannel = response._debugChannel;
2266-
if (debugChannel) {
2270+
const debugChannelCallback =
2271+
response._debugChannel && response._debugChannel.callback;
2272+
if (debugChannelCallback) {
22672273
if (value[2] === '@') {
22682274
// This is a deferred Promise.
22692275
const ref = value.slice(3); // We assume this doesn't have a path just id.
22702276
const id = parseInt(ref, 16);
22712277
if (!response._chunks.has(id)) {
22722278
// We haven't seen this id before. Query the server to start sending it.
2273-
debugChannel('P:' + ref);
2279+
debugChannelCallback('P:' + ref);
22742280
}
22752281
// Start waiting. This now creates a pending chunk if it doesn't already exist.
22762282
// This is the actual Promise we're waiting for.
@@ -2280,7 +2286,7 @@ function parseModelString(
22802286
const id = parseInt(ref, 16);
22812287
if (!response._chunks.has(id)) {
22822288
// We haven't seen this id before. Query the server to start sending it.
2283-
debugChannel('Q:' + ref);
2289+
debugChannelCallback('Q:' + ref);
22842290
}
22852291
// Start waiting. This now creates a pending chunk if it doesn't already exist.
22862292
const chunk = getChunk(response, id);
@@ -2358,7 +2364,7 @@ function ResponseInstance(
23582364
findSourceMapURL: void | FindSourceMapURLCallback, // DEV-only
23592365
replayConsole: boolean, // DEV-only
23602366
environmentName: void | string, // DEV-only
2361-
debugChannel: void | DebugChannelCallback, // DEV-only
2367+
debugChannel: void | DebugChannel, // DEV-only
23622368
) {
23632369
const chunks: Map<number, SomeChunk<any>> = new Map();
23642370
this._bundlerConfig = bundlerConfig;
@@ -2420,10 +2426,14 @@ function ResponseInstance(
24202426
this._rootEnvironmentName = rootEnv;
24212427
if (debugChannel) {
24222428
if (debugChannelRegistry === null) {
2423-
// We can't safely clean things up later, so we immediately close the debug channel.
2424-
debugChannel('');
2429+
// We can't safely clean things up later, so we immediately close the
2430+
// debug channel.
2431+
closeDebugChannel(debugChannel);
24252432
this._debugChannel = undefined;
24262433
} else {
2434+
// When a Response gets GC:ed because nobody is referring to any of the
2435+
// objects that lazily load from the Response anymore, then we can close
2436+
// the debug channel.
24272437
debugChannelRegistry.register(this, debugChannel);
24282438
}
24292439
}
@@ -2451,7 +2461,7 @@ export function createResponse(
24512461
findSourceMapURL: void | FindSourceMapURLCallback, // DEV-only
24522462
replayConsole: boolean, // DEV-only
24532463
environmentName: void | string, // DEV-only
2454-
debugChannel: void | DebugChannelCallback, // DEV-only
2464+
debugChannel: void | DebugChannel, // DEV-only
24552465
): WeakResponse {
24562466
return getWeakResponse(
24572467
// $FlowFixMe[invalid-constructor]: the shapes are exact here but Flow doesn't like constructors
@@ -3545,8 +3555,8 @@ function resolveDebugModel(
35453555
if (
35463556
__DEV__ &&
35473557
((debugChunk: any): SomeChunk<any>).status === BLOCKED &&
3548-
// TODO: This should check for the existence of the "readable" side, not the "writable".
3549-
response._debugChannel === undefined
3558+
(response._debugChannel === undefined ||
3559+
!response._debugChannel.hasReadable)
35503560
) {
35513561
if (json[0] === '"' && json[1] === '$') {
35523562
const path = json.slice(2, json.length - 1).split(':');

‎packages/react-server-dom-esm/src/client/ReactFlightDOMClientBrowser.js‎

Lines changed: 17 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -10,9 +10,10 @@
1010
import type {Thenable} from 'shared/ReactTypes.js';
1111

1212
import type {
13-
Response as FlightResponse,
14-
FindSourceMapURLCallback,
13+
DebugChannel,
1514
DebugChannelCallback,
15+
FindSourceMapURLCallback,
16+
Response as FlightResponse,
1617
} from 'react-client/src/ReactFlightClient';
1718

1819
import type {ReactServerValue} from 'react-client/src/ReactFlightReplyClient';
@@ -72,6 +73,19 @@ function createDebugCallbackFromWritableStream(
7273
}
7374

7475
function createResponseFromOptions(options: void | Options) {
76+
const debugChannel: void | DebugChannel =
77+
__DEV__ && options && options.debugChannel !== undefined
78+
? {
79+
hasReadable: options.debugChannel.readable !== undefined,
80+
callback:
81+
options.debugChannel.writable !== undefined
82+
? createDebugCallbackFromWritableStream(
83+
options.debugChannel.writable,
84+
)
85+
: null,
86+
}
87+
: undefined;
88+
7589
return createResponse(
7690
options && options.moduleBaseURL ? options.moduleBaseURL : '',
7791
null,
@@ -89,12 +103,7 @@ function createResponseFromOptions(options: void | Options) {
89103
__DEV__ && options && options.environmentName
90104
? options.environmentName
91105
: undefined,
92-
__DEV__ &&
93-
options &&
94-
options.debugChannel !== undefined &&
95-
options.debugChannel.writable !== undefined
96-
? createDebugCallbackFromWritableStream(options.debugChannel.writable)
97-
: undefined,
106+
debugChannel,
98107
);
99108
}
100109

‎packages/react-server-dom-esm/src/client/ReactFlightDOMClientNode.js‎

Lines changed: 11 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -10,8 +10,9 @@
1010
import type {Thenable, ReactCustomFormAction} from 'shared/ReactTypes.js';
1111

1212
import type {
13-
Response,
13+
DebugChannel,
1414
FindSourceMapURLCallback,
15+
Response,
1516
} from 'react-client/src/ReactFlightClient';
1617

1718
import type {Readable} from 'stream';
@@ -88,6 +89,14 @@ function createFromNodeStream<T>(
8889
moduleBaseURL: string,
8990
options?: Options,
9091
): Thenable<T> {
92+
const debugChannel: void | DebugChannel =
93+
__DEV__ && options && options.debugChannel !== undefined
94+
? {
95+
hasReadable: options.debugChannel.readable !== undefined,
96+
callback: null,
97+
}
98+
: undefined;
99+
91100
const response: Response = createResponse(
92101
moduleRootPath,
93102
null,
@@ -103,6 +112,7 @@ function createFromNodeStream<T>(
103112
__DEV__ && options && options.environmentName
104113
? options.environmentName
105114
: undefined,
115+
debugChannel,
106116
);
107117

108118
if (__DEV__ && options && options.debugChannel) {

‎packages/react-server-dom-parcel/src/client/ReactFlightDOMClientBrowser.js‎

Lines changed: 37 additions & 45 deletions
Original file line numberDiff line numberDiff line change
@@ -9,8 +9,9 @@
99

1010
import type {Thenable} from 'shared/ReactTypes.js';
1111
import type {
12-
Response as FlightResponse,
12+
DebugChannel,
1313
DebugChannelCallback,
14+
Response as FlightResponse,
1415
} from 'react-client/src/ReactFlightClient';
1516
import type {ReactServerValue} from 'react-client/src/ReactFlightReplyClient';
1617
import type {ServerReferenceId} from '../client/ReactFlightClientConfigBundlerParcel';
@@ -99,6 +100,39 @@ function createDebugCallbackFromWritableStream(
99100
};
100101
}
101102

103+
function createResponseFromOptions(options: void | Options) {
104+
const debugChannel: void | DebugChannel =
105+
__DEV__ && options && options.debugChannel !== undefined
106+
? {
107+
hasReadable: options.debugChannel.readable !== undefined,
108+
callback:
109+
options.debugChannel.writable !== undefined
110+
? createDebugCallbackFromWritableStream(
111+
options.debugChannel.writable,
112+
)
113+
: null,
114+
}
115+
: undefined;
116+
117+
return createResponse(
118+
null, // bundlerConfig
119+
null, // serverReferenceConfig
120+
null, // moduleLoading
121+
callCurrentServerCallback,
122+
undefined, // encodeFormAction
123+
undefined, // nonce
124+
options && options.temporaryReferences
125+
? options.temporaryReferences
126+
: undefined,
127+
__DEV__ ? findSourceMapURL : undefined,
128+
__DEV__ ? (options ? options.replayConsoleLogs !== false : true) : false, // defaults to true
129+
__DEV__ && options && options.environmentName
130+
? options.environmentName
131+
: undefined,
132+
debugChannel,
133+
);
134+
}
135+
102136
function startReadingFromUniversalStream(
103137
response: FlightResponse,
104138
stream: ReadableStream,
@@ -176,28 +210,7 @@ export function createFromReadableStream<T>(
176210
stream: ReadableStream,
177211
options?: Options,
178212
): Thenable<T> {
179-
const response: FlightResponse = createResponse(
180-
null, // bundlerConfig
181-
null, // serverReferenceConfig
182-
null, // moduleLoading
183-
callCurrentServerCallback,
184-
undefined, // encodeFormAction
185-
undefined, // nonce
186-
options && options.temporaryReferences
187-
? options.temporaryReferences
188-
: undefined,
189-
__DEV__ ? findSourceMapURL : undefined,
190-
__DEV__ ? (options ? options.replayConsoleLogs !== false : true) : false, // defaults to true
191-
__DEV__ && options && options.environmentName
192-
? options.environmentName
193-
: undefined,
194-
__DEV__ &&
195-
options &&
196-
options.debugChannel !== undefined &&
197-
options.debugChannel.writable !== undefined
198-
? createDebugCallbackFromWritableStream(options.debugChannel.writable)
199-
: undefined,
200-
);
213+
const response: FlightResponse = createResponseFromOptions(options);
201214
if (
202215
__DEV__ &&
203216
options &&
@@ -226,28 +239,7 @@ export function createFromFetch<T>(
226239
promiseForResponse: Promise<Response>,
227240
options?: Options,
228241
): Thenable<T> {
229-
const response: FlightResponse = createResponse(
230-
null, // bundlerConfig
231-
null, // serverReferenceConfig
232-
null, // moduleLoading
233-
callCurrentServerCallback,
234-
undefined, // encodeFormAction
235-
undefined, // nonce
236-
options && options.temporaryReferences
237-
? options.temporaryReferences
238-
: undefined,
239-
__DEV__ ? findSourceMapURL : undefined,
240-
__DEV__ ? (options ? options.replayConsoleLogs !== false : true) : false, // defaults to true
241-
__DEV__ && options && options.environmentName
242-
? options.environmentName
243-
: undefined,
244-
__DEV__ &&
245-
options &&
246-
options.debugChannel !== undefined &&
247-
options.debugChannel.writable !== undefined
248-
? createDebugCallbackFromWritableStream(options.debugChannel.writable)
249-
: undefined,
250-
);
242+
const response: FlightResponse = createResponseFromOptions(options);
251243
promiseForResponse.then(
252244
function (r) {
253245
if (

‎packages/react-server-dom-parcel/src/client/ReactFlightDOMClientEdge.js‎

Lines changed: 13 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,10 @@
99

1010
import type {Thenable, ReactCustomFormAction} from 'shared/ReactTypes.js';
1111

12-
import type {Response as FlightResponse} from 'react-client/src/ReactFlightClient';
12+
import type {
13+
DebugChannel,
14+
Response as FlightResponse,
15+
} from 'react-client/src/ReactFlightClient';
1316
import type {ReactServerValue} from 'react-client/src/ReactFlightReplyClient';
1417

1518
import {
@@ -81,6 +84,14 @@ export type Options = {
8184
};
8285

8386
function createResponseFromOptions(options?: Options) {
87+
const debugChannel: void | DebugChannel =
88+
__DEV__ && options && options.debugChannel !== undefined
89+
? {
90+
hasReadable: options.debugChannel.readable !== undefined,
91+
callback: null,
92+
}
93+
: undefined;
94+
8495
return createResponse(
8596
null, // bundlerConfig
8697
null, // serverReferenceConfig
@@ -96,6 +107,7 @@ function createResponseFromOptions(options?: Options) {
96107
__DEV__ && options && options.environmentName
97108
? options.environmentName
98109
: undefined,
110+
debugChannel,
99111
);
100112
}
101113

‎packages/react-server-dom-parcel/src/client/ReactFlightDOMClientNode.js‎

Lines changed: 10 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,7 @@
88
*/
99

1010
import type {Thenable, ReactCustomFormAction} from 'shared/ReactTypes.js';
11-
import type {Response} from 'react-client/src/ReactFlightClient';
11+
import type {DebugChannel, Response} from 'react-client/src/ReactFlightClient';
1212
import type {Readable} from 'stream';
1313

1414
import {
@@ -82,6 +82,14 @@ export function createFromNodeStream<T>(
8282
stream: Readable,
8383
options?: Options,
8484
): Thenable<T> {
85+
const debugChannel: void | DebugChannel =
86+
__DEV__ && options && options.debugChannel !== undefined
87+
? {
88+
hasReadable: options.debugChannel.readable !== undefined,
89+
callback: null,
90+
}
91+
: undefined;
92+
8593
const response: Response = createResponse(
8694
null, // bundlerConfig
8795
null, // serverReferenceConfig
@@ -95,6 +103,7 @@ export function createFromNodeStream<T>(
95103
__DEV__ && options && options.environmentName
96104
? options.environmentName
97105
: undefined,
106+
debugChannel,
98107
);
99108

100109
if (__DEV__ && options && options.debugChannel) {

0 commit comments

Comments
 (0)