Skip to content

Commit 89941d4

Browse files
sbesh91claude
andcommitted
revert: Undo traversal/canIntercept handling that broke tests
Reverts the isTraversal guard and conditional e.intercept() from router.js, and restores the original test handler behavior. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
1 parent 837867e commit 89941d4

File tree

2 files changed

+12
-12
lines changed

2 files changed

+12
-12
lines changed

src/router.js

Lines changed: 3 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -41,25 +41,21 @@ function handleNav(state, e) {
4141
// TODO: Double-check this can't fail to parse.
4242
// `.destination` is read-only, so I'm hoping it guarantees a valid URL.
4343
const url = new URL(e.destination.url);
44-
const isTraversal = e.navigationType === 'traverse';
4544

4645
if (
4746
e.hashChange ||
4847
e.downloadRequest !== null ||
4948
!isSameWindow(e) ||
50-
!isInScope(url) ||
51-
// For traversals (back/forward), canIntercept may be false (e.g. when the
52-
// destination entry was created via history.replaceState), but the URL
53-
// changes regardless — so we only gate on canIntercept for push/replace.
54-
(!isTraversal && !e.canIntercept)
49+
!isInScope(url)
5550
) {
5651
// This is set purely for our test suite so that we can check
5752
// if the event was ignored in another `navigate` handler.
5853
e['preact-iso-ignored'] = true;
5954
return state;
6055
}
6156

62-
if (e.canIntercept) e.intercept();
57+
//if (e.canIntercept)
58+
e.intercept();
6359
return url.href.replace(url.origin, '');
6460
}
6561

test/router.test.js

Lines changed: 9 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -570,10 +570,10 @@ describe('Router', () => {
570570

571571
let triedToNavigate = false;
572572
const handler = (e) => {
573+
e.intercept();
573574
if (e['preact-iso-ignored']) {
574575
triedToNavigate = true;
575576
}
576-
if (e.canIntercept) e.intercept();
577577
}
578578

579579
const App = () => {
@@ -610,12 +610,16 @@ describe('Router', () => {
610610
});
611611
}
612612

613+
// TODO: These tests are rather flakey, for some reason the additional handler
614+
// occasionally isn't running prior browser actually navigates away.
613615
for (const target of shouldNavigate) {
614616
it(`should allow default browser navigation for links with ${getName(target)}`, async () => {
615-
// WTR runs tests in an iframe, so _top/_parent target the WTR runner frame
616-
// (canIntercept=false). _blank/custom don't fire navigate events at all.
617-
if (target === '_top' || target === '_parent' || target === '_blank' || target === '_BLANK' || target === 'custom') return;
618-
617+
// Currently cross-window navigations, (e.g., `target="_blank"`), do not trigger a
618+
// `navigate` event, which makes this difficult to observe. Per the spec, however, this
619+
// might be a bug in Chrome's implementation:
620+
// https://github.com/WICG/navigation-api?tab=readme-ov-file#restrictions-on-firing-canceling-and-responding
621+
if (target === '_blank' || target === '_BLANK' || target === 'custom') return;
622+
619623
scratch.querySelector(`#${createId(target)}`).click();
620624
await sleep(1);
621625
expect(triedToNavigate).to.be.true;

0 commit comments

Comments
 (0)