feat(hub): track iframe soft navigations so the address bar and session route stay live - #252
Merged
Conversation
…on route stay live An iframe dock's address bar only updated on the `load` event, so an embedded SPA moving between routes with `history.pushState()` left it — and the `selectedDockRoute` persisted from it — showing whatever URL the frame booted with. A reload then restored the developer to the dock's entry point rather than where they actually were. `watchFrameLocation()` reports a same-origin frame's `location.href` on every navigation it can observe, and `ViewIframe` feeds it into the `currentUrl` the address bar already renders and the session route already persists — one source, now live, rather than a second tracking path. The observation sources overlap deliberately instead of being chosen between, because reports are deduped by href: `pushState`/`replaceState` are wrapped in place (restored on dispose) since they fire no event of their own, the Navigation API's `currententrychange` also catches what a wrapper structurally cannot — a router calling a `pushState` reference captured before the watch attached — and `popstate`/`hashchange` cover back/forward and hash routing. `load` re-subscribes, since a document navigation replaces the frame's `history`/`navigation` objects. Verified in Chromium against the built dist, both with the Navigation API and with it removed to exercise the wrapper-only path other engines take. Also drops `ViewIframe`'s local copy of the remote-connection stripper for the hub's exported `stripRemoteConnectionFromUrl`, which additionally strips the descriptor out of a hash-route query (`#/route?devframe-remote=…`) the local copy leaked into the address bar. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
✅ Deploy Preview for devfra ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
Contributor
There was a problem hiding this comment.
Pull request overview
Adds same-origin iframe “soft navigation” tracking to keep the hub iframe dock address bar and persisted session route in sync with SPAs that navigate via history.pushState() / replaceState() (no load event).
Changes:
- Added
watchFrameLocation()to@devframes/hub/clientto observe iframe navigations via History wrappers + Navigation API + events (popstate/hashchange/load) with href dedupe. - Updated
ViewIframeto usewatchFrameLocation()as the single source forcurrentUrl, and to use the sharedstripRemoteConnectionFromUrl()for address-bar sanitization. - Added unit tests and updated the
tsnapiAPI snapshot for the new exported surface.
Reviewed changes
Copilot reviewed 5 out of 6 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| tests/snapshots/tsnapi/@devframes/hub/client.snapshot.js | Updates JS snapshot to include the new watchFrameLocation export. |
| tests/snapshots/tsnapi/@devframes/hub/client.snapshot.d.ts | Updates type snapshot with new location-watching interfaces and function signature. |
| packages/hub/src/client/index.ts | Re-exports the new frame-location module from the hub client entrypoint. |
| packages/hub/src/client/frame-location.ts | Implements watchFrameLocation() with multi-source navigation observation + cleanup/restore behavior. |
| packages/hub/src/client/tests/frame-location.test.ts | Adds unit tests for the watcher behavior (wrapping, dedupe, resubscribe on load, cross-origin tolerance). |
| packages/hub-ui/src/client/components/views/ViewIframe.vue | Switches iframe URL tracking to watchFrameLocation() and consolidates URL sanitization via stripRemoteConnectionFromUrl(). |
Suppressed comments (3)
packages/hub/src/client/tests/frame-location.test.ts:172
toHaveBeenCalledExactlyOnceWithis not a built-in Vitest/Jest matcher in this repo, so this assertion will throw and fail the test suite.
expect(onChange).toHaveBeenCalledExactlyOnceWith('http://localhost/app/router-owned')
packages/hub/src/client/tests/frame-location.test.ts:184
toHaveBeenCalledExactlyOnceWithis not a built-in Vitest/Jest matcher in this repo, so this assertion will throw and fail the test suite.
expect(onChange).toHaveBeenCalledExactlyOnceWith('http://localhost/app/routes')
packages/hub/src/client/tests/frame-location.test.ts:225
toHaveBeenCalledExactlyOnceWithis not a built-in Vitest/Jest matcher in this repo, so this assertion will throw and fail the test suite.
expect(onAttach).toHaveBeenCalledExactlyOnceWith('http://localhost/app/elsewhere')
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
Contributor
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 5 out of 6 changed files in this pull request and generated no new comments.
Suppressed comments (1)
packages/hub/src/client/tests/frame-location.test.ts:164
- These assertions are missing the usual indentation, which will fail formatting/lint checks and makes the test block harder to read.
expect(onChange).toHaveBeenCalledTimes(1)
expect(onChange).toHaveBeenLastCalledWith('http://localhost/app/routes')
# Conflicts: # packages/hub-ui/src/client/components/views/ViewIframe.vue
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
The gap
An iframe dock's address bar only refreshed on the frame's
loadevent. An embedded SPA moving between routes withhistory.pushState()fires noload, so the bar — and theselectedDockRoutepersisted from it — kept showing whatever URL the frame booted with. A reload then restored the developer to the dock's entry point instead of where they actually were.The change
watchFrameLocation()(new,@devframes/hub/client) reports a same-origin frame'slocation.hrefon every navigation it can observe.ViewIframefeeds it into thecurrentUrlthe address bar already renders and the session route already persists — so this reuses the one source rather than adding a second tracking path.updateCurrentUrl()and itsload-only refresh are gone.Observation sources overlap deliberately instead of being chosen between, since reports are deduped by href — hearing a navigation twice costs nothing, missing one costs a stale route:
history.pushState/replaceState, wrapped in placecurrententrychangepushStatereference captured before the watch attachedpopstate/hashchangeloadhistory/navigationobjectsDisposing restores any wrapped method, so the embedded page is left exactly as found — which matters because a shared frame outlives the view watching it.
A cross-origin frame reports nothing (its location is unreadable by design) and the caller keeps the last URL it knew, as before.
about:blankis never reported, so a booting frame can't overwrite a real route with a blank.On
navigatesuccessI first reached for
navigatesuccess, but neither MDN nor the WICG explainer states whetherpushStatefires it — so the implementation doesn't depend on it. Thehistorywrapper is the deterministic path in every engine, andcurrententrychangeis the complement. Chromium does firecurrententrychangeforpushState; the dedupe absorbs the double report.Verification
packages/hub/src/client/__tests__/frame-location.test.ts), stub-injected in the style offrame-nav.test.ts.pushState/replaceState/popstate/hashchange/document load — run both as the browser ships and withnavigationremoved from the guest to exercise the wrapper-only path other engines take. All checks pass in both modes; the wrapper-bypassing case is correctly reported with the Navigation API and correctly unobservable without it. (Harness kept out of the repo — it needs a browser download CI doesn't have here.)pnpm lint && pnpm knip && pnpm test && pnpm typecheck && pnpm buildall clean (the 3 remaining lint warnings are pre-existing inwire-codec.ts).Drive-by
ViewIframehad its own copy of the remote-connection stripper; it now uses the hub's exportedstripRemoteConnectionFromUrl, which additionally strips the descriptor out of a hash-route query (#/route?devframe-remote=…) that the local copy leaked into the address bar. The persisted route deliberately keeps the descriptor, since a restored iframe still has to connect.API surface
Additive to
@devframes/hub/client:watchFrameLocationplus its option/target interfaces. Snapshot updated.🤖 Generated with Claude Code