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/pre/quiet-preload-redirect.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
'@sveltejs/kit': patch
---

fix: don't replay a preloaded redirect when a later hop of the navigation returns to the route
3 changes: 3 additions & 0 deletions packages/kit/src/runtime/client/client.js
Original file line number Diff line number Diff line change
Expand Up @@ -2072,6 +2072,9 @@ 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) {
// a preloaded redirect has been consumed; a later hop back to this route must load afresh
if (load_cache?.id === intent?.id) discard_load_cache();

await navigate({
type,
url: new URL(navigation_result.location, url),
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
import { redirect } from '@sveltejs/kit';

/** @type {import('./$types').PageServerLoad} */
export function load({ cookies, params }) {
if (params.step === 'resolve') {
cookies.set('preload-redirect-resolved', 'true', { path: '/routing/preloading/redirect' });
redirect(303, '/routing/preloading/redirect/target');
}

if (!cookies.get('preload-redirect-resolved')) {
redirect(303, '/routing/preloading/redirect/resolve');
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
<h1>Redirect resolved</h1>
18 changes: 18 additions & 0 deletions packages/kit/test/apps/basics/test/cross-platform/client.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -148,7 +148,7 @@
});

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

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

View workflow job for this annotation

GitHub Actions / e2e (test:cross-platform:build, windows-latest, 2)

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 @@ -207,7 +207,7 @@
expect(await page.innerHTML('pre')).toBe('1 true leave');
});

test('beforeNavigate is not triggered on redirect', async ({ page, baseURL }) => {

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

View workflow job for this annotation

GitHub Actions / e2e (test:cross-platform:build, windows-latest, 2)

flaky test: beforeNavigate is not triggered on redirect

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

await page.click('[href="/navigation-lifecycle/before-navigate/redirect"]');
Expand Down Expand Up @@ -390,7 +390,7 @@
expect(afterNav.type).toBe('link');
});

test('scroll state is provided on popstate navigation', async ({ page, clicknav, scroll_to }) => {

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

View workflow job for this annotation

GitHub Actions / e2e (test:cross-platform:build, windows-latest, 2)

flaky test: scroll state is provided on popstate navigation

retries: 2
await page.goto('/navigation-lifecycle/scroll-state/a');
await scroll_to(0, 500);

Expand Down Expand Up @@ -697,7 +697,7 @@
}
});

test('Scroll position is correct after going back from a shallow route', async ({

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

View workflow job for this annotation

GitHub Actions / e2e (test:server-side-route-resolution:dev, js, 1/2)

flaky test: Scroll position is correct after going back from a shallow route

retries: 2
page,
scroll_to
}) => {
Expand Down Expand Up @@ -807,7 +807,7 @@
);
});

test('Root 404 redirects somewhere due to root layout', async ({ page, baseURL, clicknav }) => {

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

View workflow job for this annotation

GitHub Actions / e2e (test:server-side-route-resolution:build, js, 1/2)

flaky test: Root 404 redirects somewhere due to root layout

retries: 2
await page.goto('/errors/error-html');
await clicknav('button:text-is("Redirect")');
expect(page.url()).toBe(baseURL + '/load');
Expand Down Expand Up @@ -909,7 +909,7 @@
expect(requests.filter((r) => r.includes('__route.js'))).toEqual([]);
});

test('preloadCode caches endpoint-only route ids', async ({ page, app }) => {

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

View workflow job for this annotation

GitHub Actions / e2e (test:cross-platform:build, windows-latest, 2)

flaky test: preloadCode caches endpoint-only route ids

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

// first call discovers that `/set-cookie` exists but has no `+page`
Expand Down Expand Up @@ -1023,6 +1023,24 @@
);
});

test('does not replay a preloaded redirect when a later hop returns to the route', async ({
page,
app
}) => {
await page.goto('/routing/a');

// the cookie is unset, so the preload resolves to a redirect to /resolve
const target = '/routing/preloading/redirect/target';
expect(await app.preloadData(target)).toMatchObject({
type: 'redirect',
location: '/routing/preloading/redirect/resolve'
});

// /resolve sets the cookie and redirects back to /target, which must now load afresh
await app.goto(target);
await expect(page.locator('h1')).toHaveText('Redirect resolved');
});

test('chooses correct route when hash route is preloaded but regular route is clicked', async ({
app,
page
Expand Down Expand Up @@ -1060,7 +1078,7 @@
await expect(page.locator('p')).toHaveText('Loaded 1 times.');
});

test('does rerun load when preload errored', async ({ app, page }) => {

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

View workflow job for this annotation

GitHub Actions / e2e (test:cross-platform:build, windows-latest, 2)

flaky test: does rerun load when preload errored

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

await app.preloadData('/routing/preloading/preload-error');
Expand Down Expand Up @@ -1357,14 +1375,14 @@
});

test.describe('cookies', () => {
test('etag forwards cookies', async ({ page }) => {

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

View workflow job for this annotation

GitHub Actions / e2e (test:cross-platform:build, windows-latest, 2)

flaky test: etag forwards cookies

retries: 2
await page.goto('/cookies/forwarded-in-etag');
await expect(page.locator('p')).toHaveText('foo=bar');
await page.locator('button').click();
await expect(page.locator('p')).toHaveText('foo=bar');
});

test('etag forwards multiple cookies', async ({ page }) => {

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

View workflow job for this annotation

GitHub Actions / e2e (test:kit:build:basics, js, 1/2)

flaky test: etag forwards multiple cookies

retries: 2
await page.goto('/cookies/forwarded-in-etag-multiple');
await expect(page.locator('p')).toHaveText('one=1; three=3; two=2');
await page.locator('button').click();
Expand Down
Loading