Skip to content

Commit d407004

Browse files
committed
fix(html): load the page in full when swapping it in fails
When anything in the swap threw (`parsePage` reading router data from a build that writes it differently, a component throwing as it unmounts), the handler's promise rejected. The Navigation API has no fallback for that, it only fires `navigateerror`: the URL had already moved to the next page while the document kept showing the previous one, or half of the next. Any error now takes the path a page from another deployment already took: it is logged, and the page reloads, which is a full load of the URL the navigation moved to. That cannot loop: `shouldIntercept` never intercepts a reload, and a page loaded in full does not navigate by itself. `shouldIntercept` is exported, with unit tests that pin this. Assisted-by: Claude Opus 5.5 <noreply@anthropic.com>
1 parent 33881f2 commit d407004

4 files changed

Lines changed: 101 additions & 11 deletions

File tree

‎e2e/client-side-navigation.spec.js‎

Lines changed: 32 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -148,4 +148,36 @@ test.describe('Client-side navigation', () => {
148148
await expect(page).toHaveURL(/\/assert\.json$/);
149149
expect(await page.evaluate(() => window.__document)).toBeUndefined();
150150
});
151+
152+
test('loads the page in full when it cannot swap it in', async ({ page }) => {
153+
// The router's copy is the page as another build writes it, with router
154+
// data this one cannot read; the full load gets the real page
155+
await page.route(/\/all(\.html)?$/, async route => {
156+
if (route.request().resourceType() !== 'fetch') {
157+
return route.fallback();
158+
}
159+
160+
const response = await route.fetch();
161+
const body = (await response.text()).replace(
162+
/(<script[^>]*data-router[^>]*>)[^<]*/,
163+
'$1{"assets":{}}'
164+
);
165+
166+
return route.fulfill({ response, body });
167+
});
168+
169+
let loads = 0;
170+
page.on('load', () => loads++);
171+
172+
await page.evaluate(() => {
173+
navigation.navigate('all.html');
174+
});
175+
176+
await expect(page).toHaveTitle(/^All \|/);
177+
expect(await page.evaluate(() => window.__document)).toBeUndefined();
178+
179+
// Reloads are never intercepted, so the full load is the only one
180+
await page.waitForTimeout(500);
181+
expect(loads).toBe(1);
182+
});
151183
});

‎packages/react/src/html/README.md‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -532,7 +532,7 @@ Everything else is a regular navigation: links outside the site (including
532532
other versions of the docs), files that are not pages (such as the JSON and
533533
Markdown renderings), and every navigation in a browser without the Navigation
534534
API. So is a page that loads scripts or stylesheets the current one did not,
535-
such as one from a newer deployment.
535+
such as one from a newer deployment, or one that fails to swap in.
536536
537537
When customizing the site, keep in mind that:
538538

‎packages/react/src/html/ui/__tests__/router.test.mjs‎

Lines changed: 51 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
11
import assert from 'node:assert/strict';
22
import { describe, it } from 'node:test';
33

4-
import { isPage, withoutFragment } from '../router.mjs';
4+
import { isPage, shouldIntercept, withoutFragment } from '../router.mjs';
55

66
const root = 'https://nodejs.org/docs/latest/api/';
77

@@ -52,3 +52,53 @@ describe('withoutFragment', () => {
5252
assert.strictEqual(withoutFragment(`${root}fs.html`), `${root}fs.html`);
5353
});
5454
});
55+
56+
describe('shouldIntercept', () => {
57+
const url = new URL('fs.html', root);
58+
59+
/**
60+
* The `navigate` event following a link to another page fires.
61+
*
62+
* @param {object} [overrides]
63+
*/
64+
const event = overrides => ({
65+
canIntercept: true,
66+
hashChange: false,
67+
downloadRequest: null,
68+
formData: null,
69+
navigationType: 'push',
70+
...overrides,
71+
});
72+
73+
it('intercepts following a link to another page', () => {
74+
assert.ok(shouldIntercept(event(), url, root));
75+
});
76+
77+
it('intercepts going back and forward between pages', () => {
78+
assert.ok(
79+
shouldIntercept(event({ navigationType: 'traverse' }), url, root)
80+
);
81+
});
82+
83+
it('never intercepts a reload, which is always a full load', () => {
84+
assert.ok(!shouldIntercept(event({ navigationType: 'reload' }), url, root));
85+
});
86+
87+
it('leaves fragments, downloads, forms and other sites to the browser', () => {
88+
for (const overrides of [
89+
{ hashChange: true },
90+
{ downloadRequest: 'fs.html' },
91+
{ formData: new FormData() },
92+
{ canIntercept: false },
93+
]) {
94+
assert.ok(
95+
!shouldIntercept(event(overrides), url, root),
96+
Object.keys(overrides)[0]
97+
);
98+
}
99+
});
100+
101+
it('leaves files that are not pages to the browser', () => {
102+
assert.ok(!shouldIntercept(event(), new URL('fs.json', root), root));
103+
});
104+
});

‎packages/react/src/html/ui/router.mjs‎

Lines changed: 17 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -56,13 +56,14 @@ export const shouldFollowLink = link =>
5656
(!link.target || link.target === '_self');
5757

5858
/**
59-
* Whether a navigation event should be intercepted by the router.
59+
* Whether a navigation event should be intercepted by the router. Reloads
60+
* never are, so the router can always fall back to one for a full load.
6061
*
6162
* @param {NavigateEvent} event
6263
* @param {URL} url
6364
* @param {string} root
6465
*/
65-
const shouldIntercept = (event, url, root) =>
66+
export const shouldIntercept = (event, url, root) =>
6667
event.canIntercept &&
6768
!event.hashChange &&
6869
event.downloadRequest === null &&
@@ -201,17 +202,24 @@ export const startRouter = ({ unmount, islands }) => {
201202
return;
202203
}
203204

204-
const doc = page && parsePage(page, assets);
205+
try {
206+
const doc = page && parsePage(page, assets);
205207

206-
if (!doc) {
207-
// The navigation has already moved to the page's URL, so reloading
208-
// is a full load of that page
209-
location.reload();
208+
if (doc) {
209+
transition(() =>
210+
showPage(doc, () => event.scroll(), unmount, islands)
211+
);
210212

211-
return;
213+
return;
214+
}
215+
} catch (error) {
216+
// The swap may have stopped halfway, which only a full load undoes
217+
console.error('[router] failed to show', url.href, error);
212218
}
213219

214-
transition(() => showPage(doc, () => event.scroll(), unmount, islands));
220+
// The navigation has already moved to the page's URL, so reloading
221+
// is a full load of that page (see `shouldIntercept`)
222+
location.reload();
215223
},
216224
});
217225
});

0 commit comments

Comments
 (0)