Skip to content

Commit 955f00a

Browse files
committed
Fix missed Metro ready race on web
1 parent 7eeab9a commit 955f00a

4 files changed

Lines changed: 97 additions & 16 deletions

File tree

packages/bundler-metro/src/__tests__/startup.test.ts

Lines changed: 58 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -142,6 +142,64 @@ describe('waitForMetroBackedAppReady', () => {
142142
expect(waitForReady).toHaveBeenCalledTimes(1);
143143
});
144144

145+
it('does not miss ready events emitted before bundle-request handling moves to the ready phase', async () => {
146+
const metroInstance = createMetroInstance();
147+
const readyListeners = new Set<() => void>();
148+
let readyAlreadyReported = false;
149+
150+
const emitReady = () => {
151+
readyAlreadyReported = true;
152+
for (const listener of readyListeners) {
153+
listener();
154+
}
155+
readyListeners.clear();
156+
};
157+
158+
const waitForReady = vi.fn(async (signal: AbortSignal) => {
159+
if (readyAlreadyReported) {
160+
return await waitForAbort(signal);
161+
}
162+
163+
return await new Promise<void>((resolve, reject) => {
164+
const onReady = () => {
165+
cleanup();
166+
resolve();
167+
};
168+
const onAbort = () => {
169+
cleanup();
170+
reject(signal.reason ?? createAbortError());
171+
};
172+
const cleanup = () => {
173+
readyListeners.delete(onReady);
174+
signal.removeEventListener('abort', onAbort);
175+
};
176+
177+
readyListeners.add(onReady);
178+
signal.addEventListener('abort', onAbort, { once: true });
179+
});
180+
});
181+
182+
const startAttempt = vi.fn(async () => {
183+
emitReady();
184+
emitBundleRequestObserved(metroInstance, 'app');
185+
});
186+
187+
await waitForMetroBackedAppReady({
188+
metro: metroInstance,
189+
platformId: 'web',
190+
bundleStartTimeout: 1_000,
191+
readyTimeout: 2_000,
192+
maxAppRestarts: 2,
193+
signal: new AbortController().signal,
194+
startAttempt,
195+
waitForReady,
196+
waitForCrash: async (signal) => await waitForAbort(signal),
197+
});
198+
199+
expect(startAttempt).toHaveBeenCalledTimes(1);
200+
expect(waitForReady).toHaveBeenCalledTimes(1);
201+
});
202+
145203
it('does not count Metro bundle build time against readyTimeout', async () => {
146204
vi.useFakeTimers();
147205

packages/bundler-metro/src/startup.ts

Lines changed: 25 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -109,16 +109,15 @@ const waitForReadyAfterBundleRequest = async (options: {
109109
events: MetroInstance['events'];
110110
readyTimeout: number;
111111
signal: AbortSignal;
112-
waitForReady: (signal: AbortSignal) => Promise<void>;
112+
readyPromise: Promise<void>;
113+
cancelReadyWait: () => void;
113114
}): Promise<void> => {
114-
const { events, readyTimeout, signal, waitForReady } = options;
115+
const { events, readyTimeout, signal, readyPromise, cancelReadyWait } = options;
115116

116117
return await new Promise<void>((resolve, reject) => {
117118
let bundlingInProgress = false;
118119
let settled = false;
119120
let timeoutId: ReturnType<typeof setTimeout> | null = null;
120-
const readyController = new AbortController();
121-
const readySignal = raceAbortSignals([signal, readyController.signal]);
122121

123122
const clearReadyTimer = () => {
124123
if (timeoutId) {
@@ -156,7 +155,7 @@ const waitForReadyAfterBundleRequest = async (options: {
156155
const startReadyTimer = () => {
157156
clearReadyTimer();
158157
timeoutId = setTimeout(() => {
159-
readyController.abort(new DOMException('The operation was aborted', 'AbortError'));
158+
cancelReadyWait();
160159
rejectOnce(new ReadyTimeoutError());
161160
}, readyTimeout);
162161
};
@@ -186,17 +185,20 @@ const waitForReadyAfterBundleRequest = async (options: {
186185
events.addListener(onMetroEvent);
187186
signal.addEventListener('abort', onAbort, { once: true });
188187

189-
void waitForReady(readySignal)
188+
void readyPromise
190189
.then(() => {
191190
resolveOnce();
192191
})
193192
.catch((error) => {
194193
if (
195-
readyController.signal.aborted &&
196-
!signal.aborted &&
197194
error instanceof DOMException &&
198195
error.name === 'AbortError'
199196
) {
197+
if (signal.aborted) {
198+
rejectOnce(
199+
signal.reason ?? new DOMException('The operation was aborted', 'AbortError')
200+
);
201+
}
200202
return;
201203
}
202204

@@ -246,6 +248,10 @@ export const waitForMetroBackedAppReady = async ({
246248
const attemptController = new AbortController();
247249
const attemptSignal = raceAbortSignals([signal, attemptController.signal]);
248250
const crashPromise = waitForCrash(attemptSignal);
251+
const readyController = new AbortController();
252+
const readyPromise = waitForReady(
253+
raceAbortSignals([attemptSignal, readyController.signal])
254+
);
249255

250256
try {
251257
const bundleRequestPromise = waitForBundleRequest({
@@ -264,17 +270,25 @@ export const waitForMetroBackedAppReady = async ({
264270
]);
265271
sawPrewarmRequest = bundleRequestResult.sawPrewarmRequest;
266272

267-
const readyPromise = waitForReadyAfterBundleRequest({
273+
const readyAfterBundleRequestPromise = waitForReadyAfterBundleRequest({
268274
events: metro.events,
269275
readyTimeout,
270276
signal: attemptSignal,
271-
waitForReady,
277+
readyPromise,
278+
cancelReadyWait: () => {
279+
readyController.abort(
280+
new DOMException('The operation was aborted', 'AbortError')
281+
);
282+
},
272283
});
273-
await Promise.race([readyPromise, crashPromise]);
284+
await Promise.race([readyAfterBundleRequestPromise, crashPromise]);
274285
attemptController.abort();
275286
onAttemptReset?.();
276287
return;
277288
} catch (error) {
289+
readyController.abort(
290+
new DOMException('The operation was aborted', 'AbortError')
291+
);
278292
attemptController.abort();
279293
onAttemptReset?.();
280294

packages/jest/src/__tests__/harness.test.ts

Lines changed: 13 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -27,11 +27,19 @@ const mocks = vi.hoisted(() => ({
2727
waitForMetroBackedAppReady: vi.fn(),
2828
}));
2929

30-
vi.mock('@react-native-harness/bundler-metro', () => ({
31-
getMetroInstance: mocks.getMetroInstance,
32-
isMetroCacheReusable: mocks.isMetroCacheReusable,
33-
waitForMetroBackedAppReady: mocks.waitForMetroBackedAppReady,
34-
}));
30+
vi.mock('@react-native-harness/bundler-metro', async () => {
31+
const actual =
32+
await vi.importActual<typeof import('@react-native-harness/bundler-metro')>(
33+
'@react-native-harness/bundler-metro'
34+
);
35+
36+
return {
37+
...actual,
38+
getMetroInstance: mocks.getMetroInstance,
39+
isMetroCacheReusable: mocks.isMetroCacheReusable,
40+
waitForMetroBackedAppReady: mocks.waitForMetroBackedAppReady,
41+
};
42+
});
3543

3644
vi.mock('@react-native-harness/bridge/server', () => ({
3745
getBridgeServer: mocks.getBridgeServer,

packages/jest/src/harness.ts

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,7 @@ import {
2020
isMetroCacheReusable,
2121
waitForMetroBackedAppReady,
2222
type MetroInstance,
23+
type ReportableEvent,
2324
} from '@react-native-harness/bundler-metro';
2425
import { createCrashArtifactWriter } from '@react-native-harness/tools';
2526
import {

0 commit comments

Comments
 (0)