Fix/error redirect

[bleucitron]

Fixes #1574

• server redirect on errors
• server redirect on 404
• client redirect on errors
• client redirect on 404
• client redirect only on browser (edgy)

Tests

• Run the tests with pnpm test and lint the project with pnpm lint and pnpm check

Changesets

• If your PR makes a change that should be noted in one or more packages' changelogs, generate a changeset by running pnpx changeset and following the prompts. All changesets should be patch until SvelteKit 1.0

[changeset-bot]

🦋 Changeset detected

Latest commit: 7c39318

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 1 package

Name	Type
@sveltejs/kit	Patch

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

[bleucitron]

I'm trying to handle the "edgy" case, which is client redirection on start, when no SSR is active.

It happens here:

kit/packages/kit/src/runtime/client/renderer.js

Lines 191 to 196 in c3ca3fb

	if (result.redirect) {
	// this is a real edge case — `load` would need to return
	// a redirect but only in the browser
	location.href = new URL(result.redirect, location.href).href;
	return;
	}

But in the end, i don't believe this is a "real" case in this PR, since by that point, the node is already loaded correctly with the work done in the previous commits.

[benmccann]

yeah, it looks like you've probably covered all the cases. basically just need to rebase and add tests at this point

[bleucitron]

I'm struggling with writing the most simple test :/

The nested __error.svelte page i added for redirections is not taken into account, basics/routes/__error.svelte is instead, which makes my simple test fail, obviously...

Am i missing something ?

[bleucitron]

The nested __error.svelte page i added for redirections is not taken into account, basics/routes/__error.svelte is instead, which makes my simple test fail, obviously...

Actually, #1576 is the reason why my missing-page tests do not work :/

[benmccann]

One thing I realized is that people can actually handle the 404 case themselves today. You can just create a file like src/routes/[zzz_catchall].svelte:

<script context="module">
	/**
	 * @type {import('@sveltejs/kit').Load}
	 */
	export async function load() {
		return {
			status: 307,
			redirect: '/'
		};
	}
</script>

[bleucitron]

One thing I realized is that people can actually handle the 404 case themselves today. You can just create a file like src/routes/[zzz_catchall].svelte:

That's indeed an alternative. But I think in my case I'd rather handle it the __error page.

[bleucitron]

All tests should be passing now. I'm not sure why deploys are failing.

[benmccann]

Deploy failures are unrelated to your change. We need to fix. I think Netlify is caching old versions of the Netlify adapter

[benmccann]

Thank you so much for all the effort you put into this PR. I talked about it with the rest of the maintainers at today's maintainer's meeting and the consensus was that isn't something folks wanted to add. I put more details about that on the issue here: #1574 (comment)

I'm terribly sorry as I know this must be a disappointing outcome given all the effort you've put into this PR and it would have been a lot nicer to have clarified that up front. I really appreciate everything you did to move this forward and help us get to a resolution on it though! Even though this didn't get merged it is nice that we now have an answer for folks on how they can accomplish this and we were able to knock an item off the 1.0 list thanks to the discussion this spurred

[bleucitron]

I'd be lying if i said i'm not a little bit disappointed, but I'll get over it :)

Happy to help, for sure I'll try to find another issue to knock down, maybe something more consensual :D

[benmccann]

that would be awesome. I went through and labeled the issues with "help wanted" where I think we're fairly agreed on a solution and it just needs to be implemented

thanks again for all the help in looking at this one!