Skip to content

Commit

Permalink
Fall back to full page reload if link href doesn't match anything (#3969
Browse files Browse the repository at this point in the history
)

* failing test for #3935

* fall back to full page navigation for unmatched routes - closes #3935

* changeset

* fix initial spa render of 404 errors (#3980)

Co-authored-by: mrkishi <mauriciokishi@gmail.com>

Co-authored-by: mrkishi <mauriciokishi@gmail.com>
  • Loading branch information
Rich-Harris and mrkishi authored Feb 17, 2022
1 parent dbcbc9a commit 11d4a4b
Show file tree
Hide file tree
Showing 7 changed files with 39 additions and 12 deletions.
5 changes: 5 additions & 0 deletions .changeset/four-news-turn.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
'@sveltejs/kit': patch
---

Fall back to full page reload if link href does not match route manifest
4 changes: 2 additions & 2 deletions packages/kit/src/runtime/app/navigation.js
Original file line number Diff line number Diff line change
Expand Up @@ -45,8 +45,8 @@ async function invalidate_(resource) {
/**
* @type {import('$app/navigation').prefetch}
*/
function prefetch_(href) {
return router.prefetch(new URL(href, get_base_uri(document)));
async function prefetch_(href) {
await router.prefetch(new URL(href, get_base_uri(document)));
}

/**
Expand Down
23 changes: 15 additions & 8 deletions packages/kit/src/runtime/client/renderer.js
Original file line number Diff line number Diff line change
Expand Up @@ -149,7 +149,7 @@ export class Renderer {
/** @type {Map<string, import('./types').NavigationResult>} */
this.cache = new Map();

/** @type {{id: string | null, promise: Promise<import('./types').NavigationResult> | null}} */
/** @type {{id: string | null, promise: Promise<import('./types').NavigationResult | undefined> | null}} */
this.loading = {
id: null,
promise: null
Expand Down Expand Up @@ -316,6 +316,11 @@ export class Renderer {
const token = (this.token = {});
let navigation_result = await this._get_navigation_result(info, no_cache);

if (!navigation_result) {
location.href = info.url.href;
return;
}

// abort if user navigated during update
if (token !== this.token) return;

Expand Down Expand Up @@ -411,7 +416,7 @@ export class Renderer {

/**
* @param {import('./types').NavigationInfo} info
* @returns {Promise<import('./types').NavigationResult>}
* @returns {Promise<import('./types').NavigationResult | undefined>}
*/
load(info) {
this.loading.promise = this._get_navigation_result(info, false);
Expand Down Expand Up @@ -471,7 +476,7 @@ export class Renderer {
/**
* @param {import('./types').NavigationInfo} info
* @param {boolean} no_cache
* @returns {Promise<import('./types').NavigationResult>}
* @returns {Promise<import('./types').NavigationResult | undefined>}
*/
async _get_navigation_result(info, no_cache) {
if (this.loading.id === info.id && this.loading.promise) {
Expand Down Expand Up @@ -504,11 +509,13 @@ export class Renderer {
if (result) return result;
}

return await this._load_error({
status: 404,
error: new Error(`Not found: ${info.url.pathname}`),
url: info.url
});
if (info.initial) {
return await this._load_error({
status: 404,
error: new Error(`Not found: ${info.url.pathname}`),
url: info.url
});
}
}

/**
Expand Down
8 changes: 6 additions & 2 deletions packages/kit/src/runtime/client/router.js
Original file line number Diff line number Diff line change
Expand Up @@ -66,6 +66,7 @@ export class Router {
renderer.router = this;

this.enabled = true;
this.initialized = false;

// make it possible to reset focus
document.body.setAttribute('tabindex', '-1');
Expand Down Expand Up @@ -257,6 +258,8 @@ export class Router {
);
}
});

this.initialized = true;
}

#update_scroll_positions() {
Expand All @@ -283,7 +286,8 @@ export class Router {
id: url.pathname + url.search,
routes: this.routes.filter(([pattern]) => pattern.test(path)),
url,
path
path,
initial: !this.initialized
};
}
}
Expand Down Expand Up @@ -333,7 +337,7 @@ export class Router {

/**
* @param {URL} url
* @returns {Promise<import('./types').NavigationResult>}
* @returns {Promise<import('./types').NavigationResult | undefined>}
*/
async prefetch(url) {
const info = this.parse(url);
Expand Down
1 change: 1 addition & 0 deletions packages/kit/src/runtime/client/types.d.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ export type NavigationInfo = {
routes: CSRRoute[];
url: URL;
path: string;
initial: boolean;
};

export type NavigationCandidate = {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,5 +7,6 @@
<a href="/routing/a">a</a>
<a href="/routing/ambiguous/ok.json" rel="external">ok</a>
<a href="http://localhost:{$page.url.searchParams.get('port')}">elsewhere</a>
<a href="/static.json">static.json</a>

<div class="hydrate-test" />
9 changes: 9 additions & 0 deletions packages/kit/test/apps/basics/test/test.js
Original file line number Diff line number Diff line change
Expand Up @@ -2112,6 +2112,15 @@ test.describe.parallel('Routing', () => {
await page.goto('/routing/rest/complex/prefix-one/two/three');
expect(await page.textContent('h1')).toBe('parts: one/two/three');
});

test('links to unmatched routes result in a full page navigation, not a 404', async ({
page,
clicknav
}) => {
await page.goto('/routing');
await clicknav('[href="/static.json"]');
expect(await page.textContent('body')).toBe('"static file"\n');
});
});

test.describe.parallel('Session', () => {
Expand Down

0 comments on commit 11d4a4b

Please sign in to comment.