Skip to content

Commit 351279d

Browse files
committed
stream-node: fire shell callback only after successful shell write
1 parent 41e804d commit 351279d

2 files changed

Lines changed: 75 additions & 20 deletions

File tree

src/stream-node.js

Lines changed: 51 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -26,6 +26,41 @@ export function renderToPipeableStream(vnode, options, context) {
2626
const controller = new AbortController();
2727
const stream = new PassThrough();
2828
let waitingForDrain = null;
29+
let shellReadyCalled = false;
30+
let allReadyCalled = false;
31+
let errored = false;
32+
let shellReadyScheduled = false;
33+
34+
function callOnShellReady() {
35+
if (shellReadyCalled || errored) return;
36+
shellReadyCalled = true;
37+
options.onShellReady && options.onShellReady();
38+
}
39+
40+
function callOnAllReady() {
41+
if (allReadyCalled || errored) return;
42+
allReadyCalled = true;
43+
options.onAllReady && options.onAllReady();
44+
}
45+
46+
function callOnError(error) {
47+
if (errored) return;
48+
errored = true;
49+
if (options.onError) {
50+
options.onError(error);
51+
} else {
52+
throw error;
53+
}
54+
}
55+
56+
function scheduleOnShellReady() {
57+
if (shellReadyCalled || shellReadyScheduled || errored) return;
58+
shellReadyScheduled = true;
59+
Promise.resolve().then(() => {
60+
shellReadyScheduled = false;
61+
callOnShellReady();
62+
});
63+
}
2964

3065
/**
3166
* @returns {Promise<void>}
@@ -59,33 +94,29 @@ export function renderToPipeableStream(vnode, options, context) {
5994
return waitingForDrain;
6095
}
6196

62-
renderToChunks(vnode, {
63-
context,
64-
abortSignal: controller.signal,
65-
async onWrite(s) {
66-
if (stream.destroyed || stream.writableEnded) return;
67-
if (!stream.write(encoder.encode(s))) {
68-
await waitForDrain();
69-
}
70-
}
71-
})
97+
Promise.resolve()
98+
.then(() =>
99+
renderToChunks(vnode, {
100+
context,
101+
abortSignal: controller.signal,
102+
async onWrite(s) {
103+
scheduleOnShellReady();
104+
if (stream.destroyed || stream.writableEnded) return;
105+
if (!stream.write(encoder.encode(s))) {
106+
await waitForDrain();
107+
}
108+
}
109+
})
110+
)
72111
.then(() => {
73-
options.onAllReady && options.onAllReady();
112+
callOnAllReady();
74113
stream.end();
75114
})
76115
.catch((error) => {
77116
stream.destroy();
78-
if (options.onError) {
79-
options.onError(error);
80-
} else {
81-
throw error;
82-
}
117+
callOnError(error);
83118
});
84119

85-
Promise.resolve().then(() => {
86-
options.onShellReady && options.onShellReady();
87-
});
88-
89120
return {
90121
/**
91122
* @param {unknown} [reason]

test/compat/stream-node.test.jsx

Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -88,4 +88,28 @@ describe('renderToPipeableStream', () => {
8888

8989
expect(error).to.be.undefined;
9090
});
91+
92+
it('should not call onShellReady if shell rendering throws', async () => {
93+
let shellReady = false;
94+
let caught;
95+
96+
function Thrower() {
97+
throw new Error('shell failed');
98+
}
99+
100+
renderToPipeableStream(<Thrower />, {
101+
onShellReady: () => {
102+
shellReady = true;
103+
},
104+
onError: (error) => {
105+
caught = error;
106+
}
107+
});
108+
109+
await new Promise((resolve) => setTimeout(resolve, 0));
110+
111+
expect(shellReady).toBe(false);
112+
expect(caught).toBeInstanceOf(Error);
113+
expect(caught.message).toBe('shell failed');
114+
});
91115
});

0 commit comments

Comments
 (0)