Skip to content

Commit 33be060

Browse files
motiz88facebook-github-bot
authored andcommitted
Always reload the frontend when launching, even in an existing window (#53407)
Summary: Pull Request resolved: #53407 Changelog: [Internal] ## Context Upon receiving a launch command, the RNDT shell either: 1. Creates a new window and navigates to the requested frontend URL. 2. Brings an existing window to the foreground *with no further navigation*. In the happy path, (2) is a pretty nice experience: it preserves all prior UI state in the frontend and leaves the user with an instantly responsive debugger - this can be quite a bit faster than (1) because of the overhead of loading and parsing source maps for example. However, this breaks down if the frontend is not in a usable state to begin with. This is, sadly, a frequent-enough occurrence that we must account for it: the CDP connection may have been lost, the frontend app itself might have failed to load the last time, etc. Preserving everything that's nice about (2) while also making it fully reliable - incrementally bringing the frontend to the state specified by a new URL - would require delicate engineering across the shell and frontend codebases, which is an amount of complexity I would like to sidestep for now. NOTE: The more complex solution **is 100% worth implementing in the long term,** as it has tangible benefits for the user, and matches Chrome best. ## This diff Here we take a much cheaper approach than the one described above: the shell will *always* initiate navigation to the new frontend URL, regardless of whether it does so in a new window or a previously opened one. This will consistently bring the user to a state where the frontend is open and working (although it will reset any ephemeral UI state in the process, and typically take a noticeable amount of time to load). Even with this simplified approach, the standalone shell still offers a better experience than launching in a browser (if only because it is zero-install and avoids the "dead tab spam" problem). Reviewed By: huntie Differential Revision: D80711185 fbshipit-source-id: 8f376ccf1717c48a1742c798da3171ac6d2f8af0
1 parent a843119 commit 33be060

1 file changed

Lines changed: 21 additions & 24 deletions

File tree

packages/debugger-shell/src/electron/MainInstanceEntryPoint.js

Lines changed: 21 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -36,49 +36,45 @@ function handleLaunchArgs(argv: string[]) {
3636
});
3737

3838
// Find an existing window for this app and launch configuration.
39-
const existingWindow = BrowserWindow.getAllWindows().find(window => {
39+
let frontendWindow = BrowserWindow.getAllWindows().find(window => {
4040
const metadata = windowMetadata.get(window);
4141
if (!metadata) {
4242
return false;
4343
}
4444
return metadata.windowKey === windowKey;
4545
});
4646

47-
if (existingWindow) {
47+
if (frontendWindow) {
4848
// If the window is already visible, flash it.
49-
if (existingWindow.isVisible()) {
50-
existingWindow.flashFrame(true);
49+
if (frontendWindow.isVisible()) {
50+
frontendWindow.flashFrame(true);
5151
setTimeout(() => {
52-
existingWindow.flashFrame(false);
52+
frontendWindow.flashFrame(false);
5353
}, 1000);
5454
}
55-
if (process.platform === 'darwin') {
56-
app.focus({
57-
steal: true,
58-
});
59-
}
60-
existingWindow.focus();
61-
return;
55+
} else {
56+
// Create the browser window.
57+
frontendWindow = new BrowserWindow({
58+
width: 1200,
59+
height: 600,
60+
webPreferences: {
61+
partition: 'persist:react-native-devtools',
62+
preload: require.resolve('./preload.js'),
63+
},
64+
// Icon for Linux
65+
icon: path.join(__dirname, 'resources', 'icon.png'),
66+
});
6267
}
6368

64-
// Create the browser window.
65-
const frontendWindow = new BrowserWindow({
66-
width: 1200,
67-
height: 600,
68-
webPreferences: {
69-
partition: 'persist:react-native-devtools',
70-
preload: require.resolve('./preload.js'),
71-
},
72-
// Icon for Linux
73-
icon: path.join(__dirname, 'resources', 'icon.png'),
74-
});
75-
7669
// Open links in the default browser instead of in new Electron windows.
7770
frontendWindow.webContents.setWindowOpenHandler(({url}) => {
7871
shell.openExternal(url);
7972
return {action: 'deny'};
8073
});
8174

75+
// TODO: If the window contains a live, working frontend instance with a valid connection to the backend,
76+
// we should avoid this reload and instead send the frontend a message to handle the launch arguments
77+
// dynamically (e.g. update the launch ID for telemetry purposes, handle deeplinking to a specific CDT panel, etc).
8278
frontendWindow.loadURL(frontendUrl);
8379

8480
windowMetadata.set(frontendWindow, {
@@ -90,6 +86,7 @@ function handleLaunchArgs(argv: string[]) {
9086
steal: true,
9187
});
9288
}
89+
frontendWindow.focus();
9390
}
9491

9592
app.whenReady().then(() => {

0 commit comments

Comments
 (0)