Skip to content

Fix preview navigation black screen when selecting routes from dropdown #2610

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
wwwillchen merged 3 commits into from
Feb 12, 2026
Merged

Fix preview navigation black screen when selecting routes from dropdown #2610

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
b89b4a6
Fix preview navigation black screen when selecting routes from dropdo…
wwwillchen Feb 10, 2026
d7cada2
fix
wwwillchen Feb 11, 2026
1bdd763
Merge branch 'main' into agent-2428-1770745558
wwwillchen Feb 12, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
25 changes: 25 additions & 0 deletions e2e-tests/refresh.spec.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -153,3 +153,28 @@ testSkipIfWindows(
).toBeVisible({ timeout: Timeout.MEDIUM });
},
);

testSkipIfWindows(
"spa navigation inside iframe does not change iframe src attribute",
async ({ po }) => {
await po.setUp({ autoApprove: true });
await po.sendPrompt("tc=multi-page");

await po.previewPanel.expectPreviewIframeIsVisible();

const iframe = po.previewPanel.getPreviewIframeElement();
await expect(
iframe.contentFrame().getByRole("heading", { name: "Home Page" }),
).toBeVisible({ timeout: Timeout.LONG });

const srcBeforeNavigation = await iframe.getAttribute("src");

await iframe.contentFrame().getByText("Go to About Page").click();
await expect(
iframe.contentFrame().getByRole("heading", { name: "About Page" }),
).toBeVisible({ timeout: Timeout.MEDIUM });

const srcAfterNavigation = await iframe.getAttribute("src");
expect(srcAfterNavigation).toBe(srcBeforeNavigation);
},
);
59 changes: 51 additions & 8 deletions src/components/preview_panel/PreviewIframe.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ import {
previewCurrentUrlAtom,
} from "@/atoms/appAtoms";
import { useAtomValue, useSetAtom, useAtom } from "jotai";
import { useEffect, useRef, useState } from "react";
import { useEffect, useMemo, useRef, useState } from "react";
import {
ArrowLeft,
ArrowRight,
Expand Down Expand Up @@ -936,10 +936,15 @@ export const PreviewIframe = ({ loading }: { loading: boolean }) => {
const baseUrl = new URL(appUrl).origin;
const newUrl = `${baseUrl}${path}`;
Comment thread
github-actions[bot] marked this conversation as resolved.
Comment on lines 936 to 937

@github-actions github-actions Bot Feb 12, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 MEDIUM — Unguarded new URL(appUrl) can throw

3/3 reviewers flagged this (2 MEDIUM, 1 LOW)

new URL(appUrl).origin on line 936 will throw a TypeError if appUrl is malformed. Every other navigation handler in this component (handleNavigateBack, handleNavigateForward, handleReload, pushState/replaceState handlers) wraps URL parsing in a try/catch. This is the only handler that doesn't, making it an inconsistency and a potential crash site during edge cases like app startup transitions.

Suggested change
const baseUrl = new URL(appUrl).origin;
const newUrl = `${baseUrl}${path}`;
try {
const baseUrl = new URL(appUrl).origin;
const newUrl = `${baseUrl}${path}`;

The closing } catch { console.error('Invalid appUrl during route navigation'); return; } should wrap the rest of the function body before the final } of the if block.


// Navigate to the URL
iframeRef.current.contentWindow.location.href = newUrl;

// iframeRef.current.src = newUrl;
// Use postMessage to navigate (same as back/forward) - this uses location.replace()
// which provides smooth navigation without the black screen flicker that location.href causes
iframeRef.current.contentWindow.postMessage(
{
type: "navigate",
payload: { url: newUrl },
},
"*",
Comment thread
github-actions[bot] marked this conversation as resolved.
);
Comment thread
github-actions[bot] marked this conversation as resolved.
Comment thread
github-actions[bot] marked this conversation as resolved.

// Update navigation history
const newHistory = [
Expand All @@ -950,9 +955,50 @@ export const PreviewIframe = ({ loading }: { loading: boolean }) => {
setCurrentHistoryPosition(newHistory.length - 1);
setCanGoBack(true);
setCanGoForward(false);

// Update iframe URL ref to match
currentIframeUrlRef.current = newUrl;

// Update preservedUrls to match navigation (for HMR remounts)
if (selectedAppId) {
// Clear preserved URL if navigating to root, otherwise update it
if (path === "/" || path === "") {
setPreservedUrls((prev) => {
const newUrls = { ...prev };
delete newUrls[selectedAppId];
return newUrls;
});
} else {
setPreservedUrls((prev) => ({
...prev,
[selectedAppId]: newUrl,
}));
}
}
Comment thread
github-actions[bot] marked this conversation as resolved.
}
};

// Freeze iframe src between remounts so in-iframe SPA navigation (pushState/replaceState)
// doesn't cause React to set a new src and trigger a second full navigation flicker.
const iframeSrc = useMemo(() => {
if (!appUrl) {
return undefined;
}

const currentUrl = currentIframeUrlRef.current;
if (!currentUrl) {
return appUrl;
}

try {
const currentOrigin = new URL(currentUrl).origin;
const appOrigin = new URL(appUrl).origin;
return currentOrigin === appOrigin ? currentUrl : appUrl;
} catch {
return appUrl;
}
}, [appUrl, reloadKey, selectedAppId]);
Comment on lines +983 to +1000

@devin-ai-integration devin-ai-integration Bot Feb 12, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 useMemo reads stale ref on app switch when origins match, producing wrong iframe src

The iframeSrc useMemo reads currentIframeUrlRef.current during render, but the ref is only reset to the new app's base URL in a useEffect (which runs after render). When selectedAppId changes, the useMemo recomputes with the old ref value. If the old and new apps share the same origin (e.g., both on localhost:3000), the origin check passes and the memo returns the old app's route URL instead of the new app's base URL. Subsequent renders don't fix this because the useMemo deps (appUrl, reloadKey, selectedAppId) don't change again.

Root Cause and Impact

Sequence when switching apps with the same origin:

  1. selectedAppId and appUrl change → useMemo recomputes.
  2. currentIframeUrlRef.current still holds the previous app's URL (e.g. http://localhost:3000/about).
  3. Origin check at line 994-996 sees matching origins → returns the stale URL.
  4. The useEffect at PreviewIframe.tsx:725-735 runs after render and sets currentIframeUrlRef.current = appUrl, but this doesn't trigger a useMemo recomputation because none of its deps changed.
  5. The iframe src is set to the old app's route URL for the lifetime of the component (until a reload or another appUrl change).

Impact: When two apps happen to share the same origin (e.g., one app stops and another starts on the same port), the iframe would load the wrong route from the previous app. In practice this is an uncommon edge case because Dyad typically assigns different ports to different apps.

Prompt for agents 
The useMemo for iframeSrc reads currentIframeUrlRef.current, but this ref is only updated in a useEffect that runs after render. When selectedAppId changes and the old/new apps share the same origin, the memo returns a stale URL. To fix this, either: (1) Reset currentIframeUrlRef.current synchronously during render (before the useMemo) when selectedAppId or appUrl changes, using a pattern like checking prevAppUrlRef inline. For example, add a synchronous check before the useMemo: if (appUrl && appUrl !== prevAppUrlRef.current) { currentIframeUrlRef.current = appUrl; prevAppUrlRef.current = appUrl; } Or (2) Add a separate state variable (e.g. iframeSrcVersion) that gets incremented in the useEffect alongside the ref reset, and include it in the useMemo dependency array to force recomputation.
Open in Devin Review

Was this helpful? React with 👍 or 👎 to provide feedback.

Comment on lines +983 to +1000

@github-actions github-actions Bot Feb 12, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 MEDIUM — useMemo reads a mutable ref but deps cannot track ref mutations

3/3 reviewers flagged this (3 MEDIUM)

This useMemo reads currentIframeUrlRef.current (line 988), but refs are not reactive — changes to the ref do NOT trigger re-computation of the memo. The dependency array [appUrl, reloadKey, selectedAppId] relies on the assumption that one of these values always changes whenever the ref needs to be re-read.

After navigateToRoute or back/forward handlers update the ref, none of these deps change, so the memo returns a stale value. This appears to be intentional (the comment says "freeze iframe src"), and it's the actual mechanism that prevents the black screen flicker. However, the coupling is fragile and undocumented — a future maintainer could easily break this by adding a code path that updates the ref expecting the memo to recompute, or by using iframeSrc where they actually need the current URL.

Suggestion: Add a comment directly on the dependency array documenting this intentional design:

Suggested change
const iframeSrc = useMemo(() => {
if (!appUrl) {
return undefined;
}
const currentUrl = currentIframeUrlRef.current;
if (!currentUrl) {
return appUrl;
}
try {
const currentOrigin = new URL(currentUrl).origin;
const appOrigin = new URL(appUrl).origin;
return currentOrigin === appOrigin ? currentUrl : appUrl;
} catch {
return appUrl;
}
}, [appUrl, reloadKey, selectedAppId]);
// Freeze iframe src between remounts so in-iframe SPA navigation (pushState/replaceState)
// doesn't cause React to set a new src and trigger a second full navigation flicker.
const iframeSrc = useMemo(() => {
if (!appUrl) {
return undefined;
}
const currentUrl = currentIframeUrlRef.current;
if (!currentUrl) {
return appUrl;
}
try {
const currentOrigin = new URL(currentUrl).origin;
const appOrigin = new URL(appUrl).origin;
return currentOrigin === appOrigin ? currentUrl : appUrl;
} catch {
return appUrl;
}
// NOTE: currentIframeUrlRef.current is intentionally read as a side-channel
// and is NOT in the dependency array. This memo only re-evaluates when appUrl,
// reloadKey, or selectedAppId change. Navigation handlers (navigateToRoute,
// handleNavigateBack, etc.) update the ref synchronously but do NOT trigger
// memo recomputation — this is what prevents the iframe src from being reset
// during SPA navigation, which would cause a visible black screen flicker.
// Do NOT use iframeSrc to read the current iframe URL; use currentIframeUrlRef.current instead.
}, [appUrl, reloadKey, selectedAppId]);


// Display loading state
if (loading) {
return (
Expand Down Expand Up @@ -984,9 +1030,6 @@ export const PreviewIframe = ({ loading }: { loading: boolean }) => {
restartApp();
};

// Convert null to undefined for iframe src prop compatibility
const iframeSrc = currentIframeUrlRef.current ?? appUrl ?? undefined;

return (
<div className="flex flex-col h-full">
{/* Browser-style header - hide when annotator is active */}
Expand Down
Loading