Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions .changeset/gold-horses-return.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
'@sveltejs/kit': patch
---

fix: fulfil `beforeNavigate` `complete` when redirected
10 changes: 7 additions & 3 deletions packages/kit/src/runtime/client/client.js
Original file line number Diff line number Diff line change
Expand Up @@ -461,7 +461,7 @@ export async function _goto(url, options, redirect_count, nav_token) {
load_cache = null;
}

const result = await navigate({
await navigate({
type: 'goto',
url: resolve_url(url),
keepfocus: options.keepFocus,
Expand All @@ -481,6 +481,7 @@ export async function _goto(url, options, redirect_count, nav_token) {
}
}
});

if (options.invalidateAll) {
// TODO the ticks shouldn't be necessary, something inside Svelte itself is buggy
// when a query in a layout that still exists after page change is refreshed earlier than this
Expand All @@ -496,7 +497,6 @@ export async function _goto(url, options, redirect_count, nav_token) {
});
});
}
return result;
}

/** @param {import('./types.js').NavigationIntent} intent */
Expand Down Expand Up @@ -1283,6 +1283,7 @@ async function load_root_error_page({ status, error, url, route }) {
});
} catch (error) {
if (error instanceof Redirect) {
// @ts-expect-error TODO investigate this
return _goto(new URL(error.location, location.href), {}, 0);
}

Expand Down Expand Up @@ -1577,7 +1578,7 @@ async function navigate({
if (navigation_result.type === 'redirect') {
// whatwg fetch spec https://fetch.spec.whatwg.org/#http-redirect-fetch says to error after 20 redirects
if (redirect_count < 20) {
return navigate({
await navigate({
type,
url: new URL(navigation_result.location, url),
popped,
Expand All @@ -1588,6 +1589,9 @@ async function navigate({
redirect_count: redirect_count + 1,
nav_token
});

nav.fulfil(undefined);
return;
}

navigation_result = await load_root_error_page({
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
<script>
import { beforeNavigate } from '$app/navigation';

beforeNavigate(({ complete }) => {
complete.then(() => {
console.log('complete');
});
});
</script>

<a href="/navigation-lifecycle/before-navigate/redirect">redirect</a>
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@
test.describe.configure({ mode: 'parallel' });

test.describe('a11y', () => {
test('resets focus', async ({ page, clicknav, browserName }) => {

Check warning on line 12 in packages/kit/test/apps/basics/test/cross-platform/client.test.js

View workflow job for this annotation

GitHub Actions / test-kit-cross-browser (18, windows-latest, chromium, dev)

flaky test: resets focus

retries: 2
const tab = browserName === 'webkit' ? 'Alt+Tab' : 'Tab';

await page.goto('/accessibility/a');
Expand All @@ -33,7 +33,7 @@
expect(await page.evaluate(() => document.documentElement.getAttribute('tabindex'))).toBe(null);
});

test('applies autofocus after a navigation', async ({ page, clicknav }) => {

Check warning on line 36 in packages/kit/test/apps/basics/test/cross-platform/client.test.js

View workflow job for this annotation

GitHub Actions / test-kit-cross-browser (18, windows-latest, chromium, dev)

flaky test: applies autofocus after a navigation

retries: 2
await page.goto('/accessibility/autofocus/a');

await clicknav('[href="/accessibility/autofocus/b"]');
Expand Down Expand Up @@ -124,7 +124,7 @@
});

test.describe('Navigation lifecycle functions', () => {
test('beforeNavigate prevents navigation triggered by link click', async ({ page, baseURL }) => {

Check warning on line 127 in packages/kit/test/apps/basics/test/cross-platform/client.test.js

View workflow job for this annotation

GitHub Actions / test-kit-cross-browser (18, macOS-latest, webkit, dev)

flaky test: beforeNavigate prevents navigation triggered by link click

retries: 2
await page.goto('/navigation-lifecycle/before-navigate/prevent-navigation');

await page.click('[href="/navigation-lifecycle/before-navigate/a"]');
Expand Down Expand Up @@ -240,6 +240,12 @@
await expect(page.locator('pre')).toHaveText('1 false link');
});

test("beforeNavigate's complete fulfills after redirect", async ({ page, clicknav }) => {
await page.goto('/navigation-lifecycle/before-navigate/complete');
clicknav('a[href="/navigation-lifecycle/before-navigate/redirect"]');
expect(await page.waitForEvent('console', (msg) => msg.text() === 'complete')).toBeTruthy();
});

test('afterNavigate calls callback', async ({ page, clicknav }) => {
await page.goto('/navigation-lifecycle/after-navigate/a');
expect(await page.textContent('h1')).toBe(
Expand Down Expand Up @@ -915,7 +921,7 @@
await expect(page.locator('h1')).toHaveText('target: 1');
});

test('responds to <form method="GET"> submission without reload', async ({ page }) => {

Check warning on line 924 in packages/kit/test/apps/basics/test/cross-platform/client.test.js

View workflow job for this annotation

GitHub Actions / test-kit-server-side-route-resolution (build)

flaky test: responds to <form method="GET"> submission without reload

retries: 2
await page.goto('/routing/form-get');

expect(await page.textContent('h1')).toBe('...');
Expand Down Expand Up @@ -950,7 +956,7 @@
expect(tabs.length > 1);
});

test('responds to <button formtarget="_blank" submission with new tab', async ({ page }) => {

Check warning on line 959 in packages/kit/test/apps/basics/test/cross-platform/client.test.js

View workflow job for this annotation

GitHub Actions / test-kit-cross-browser (18, ubuntu-latest, firefox, build)

flaky test: responds to <button formtarget="_blank" submission with new tab

retries: 2
await page.goto('/routing/form-target-blank');

let tabs = page.context().pages();
Expand Down
Loading