Fix preserved URL not cleared when navigating to root or restarting#2422
Conversation
PR dyad-sh#2336 introduced previewCurrentUrlAtom to preserve routes across HMR, but it wasn't cleared when: 1. Navigating back to root (/) via pushState/replaceState 2. Restarting the app This caused the wrong route to load after restart/HMR if the user had previously visited a sub-route and then navigated back to root. Changes: - Clear preserved URL in pushState/replaceState handlers when pathname is "/" - Clear preserved URL in restartApp before restarting - Add E2E test for restart after navigating back to root Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
|
@BugBot run |
Summary of ChangesHello @wwwillchen, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request resolves a regression that caused incorrect URL restoration after an application restart, particularly when navigating back to the root path. The changes ensure that the application's internal state for preserved URLs is correctly cleared during root navigation and before restarts, providing a more predictable user experience. Highlights
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
There was a problem hiding this comment.
Code Review
This pull request effectively fixes a regression where the preserved URL was not cleared upon navigating to the root path or restarting the application. The changes in PreviewIframe.tsx and useRunApp.ts correctly handle clearing the stale URL. The addition of a new end-to-end test in restart.spec.ts is excellent as it verifies the fix and prevents future regressions. I have one suggestion to improve maintainability by addressing code duplication. Overall, this is a solid contribution.
| } else if (newUrlObj.origin === appUrlObj.origin) { | ||
| // Clear preserved URL when navigating back to root | ||
| setPreservedUrls((prev) => { | ||
| const next = { ...prev }; | ||
| delete next[selectedAppId]; | ||
| return next; | ||
| }); | ||
| } |
There was a problem hiding this comment.
This logic for handling preserved URLs is also present in the pushState handler (lines 631-657). Duplicating this code block makes it harder to maintain, as future changes would need to be made in two places. Consider extracting this logic into a helper function within the useEffect hook to avoid repetition and ensure consistency between pushState and replaceState handling.
Greptile OverviewGreptile SummaryFixed regression from PR #2336 where navigating back to root ( Changes:
The fix ensures that whenever a user navigates to the root path, the preserved URL is explicitly deleted from state, preventing incorrect route restoration during subsequent HMR or restart operations. Confidence Score: 5/5
Important Files Changed
Sequence DiagramsequenceDiagram
participant User
participant PreviewIframe
participant Router as React Router in iframe
participant PreservedUrls as previewCurrentUrlAtom
participant RestartApp as useRunApp.restartApp()
Note over User,RestartApp: Scenario: Navigate to /about, then back to /, then restart
User->>PreviewIframe: Navigate to /about
Router->>PreviewIframe: postMessage: pushState with /about
PreviewIframe->>PreviewIframe: Check: pathname !== "/" && pathname !== ""
PreviewIframe->>PreservedUrls: Store /about for appId
User->>PreviewIframe: Navigate back to /
Router->>PreviewIframe: postMessage: pushState with /
PreviewIframe->>PreviewIframe: Check: pathname === "/" or pathname === ""
PreviewIframe->>PreservedUrls: Delete entry for appId (FIX)
User->>PreviewIframe: Click Restart button
PreviewIframe->>RestartApp: Call restartApp()
RestartApp->>PreservedUrls: Delete entry for appId (FIX)
RestartApp->>RestartApp: Stop and restart app
Note over PreviewIframe,PreservedUrls: On component remount after restart
PreviewIframe->>PreservedUrls: Check for preserved URL
PreservedUrls-->>PreviewIframe: null (no preserved URL)
PreviewIframe->>PreviewIframe: Load root URL /
|
|
@BugBot run |
Summary
previewCurrentUrlAtomwasn't cleared when navigating back to root (/)pushState/replaceStatehandlers when pathname is "/" or emptyrestartAppbefore restarting to prevent stale route restorationFixes the issue where HMR/restart would load the wrong URL after navigating back to root from a sub-route.
Test plan
restart after navigating back to root should stay on rootpassesrefresh preserves current routestill passes🤖 Generated with Claude Code
Note
Medium Risk
Touches preview navigation state and route preservation behavior, which could affect iframe routing/back-forward history across HMR and restarts. Changes are localized and covered by a new Playwright E2E regression test.
Overview
Fixes a regression where the preview could restore a stale sub-route after returning to
/and then triggering HMR/reload.PreviewIframenow clearspreviewCurrentUrlAtomwhenpushState/replaceStatenavigates back to the app’s root path, anduseRunApp.restartAppclears the preserved URL before restarting to avoid stale route restoration. Adds a Playwright E2E test (hmr_path.spec.ts) covering the root-navigation + HMR scenario, and allows disabling the Claude stop hook viaDISABLE_DYAD_STOP_HOOK.Written by Cursor Bugbot for commit e5aaa06. This will update automatically on new commits. Configure here.
Summary by cubic
Fixes a regression where the preview restored a stale sub-route after returning to “/” or restarting. We now clear the preserved URL so the app stays on the expected root route.
Bug Fixes
New Features
Written for commit e5aaa06. Summary will update on new commits.