Redirects in load functions break when running client-side

[WaltzingPenguin]

Describe the bug

There appears to be no way to use the new redirect method and disable SSR globally.

Reproduction

1. Create a new skeleton project
2. Create src/routes/page.ts with the contents:

import { redirect } from "@sveltejs/kit";

export function load() {
  throw redirect(307, "/about")
}

3. Create src/hooks.ts with the contents:

import type { Handle } from '@sveltejs/kit'

export const handle: Handle = async ({ event, resolve }) => {
  return resolve(event, { ssr: false })
}

4. Navigating to root now results in an error page, with the redirect class passed to it, instead of being redirected to "/about"

Repo: https://github.com/WaltzingPenguin/sveltekit-redirect-hooks

Logs

No response

System Info

System:
    OS: Windows 10 10.0.19043
    CPU: (12) x64 AMD Ryzen 5 1600 Six-Core Processor
    Memory: 7.50 GB / 15.95 GB
  Binaries:
    Node: 16.15.0 - C:\Program Files\nodejs\node.EXE
    Yarn: 1.22.10 - ~\AppData\Roaming\npm\yarn.CMD
    npm: 7.20.3 - C:\Program Files\nodejs\npm.CMD
  Browsers:
    Chrome: 104.0.5112.81
    Edge: Spartan (44.19041.1266.0), Chromium (104.0.1293.54)
    Internet Explorer: 11.0.19041.1566
  npmPackages:
    @sveltejs/adapter-auto: next => 1.0.0-next.64
    @sveltejs/kit: next => 1.0.0-next.413
    svelte: ^3.44.0 => 3.49.0
    vite: ^3.0.0 => 3.0.8

Severity

blocking an upgrade

Additional Information

No response

[denovodavid]

As a simple workaround I've done the following:

const location = '/login';
if (browser) return await goto(location);
else throw redirect(302, location);

This worked for me in browser and SSR, but yes ideally redirect() should work client side.

EDIT: may not work with links using sveltekit:prefetch

[fev4]

I'm experiencing the same situation without disabling SSR:

// file: src/routes/+layout.ts

import {
  envelopeGuard,
  envelopeGuardRedirect,
} from '$lib/utils/guards';
import { accounts } from '$lib/stores/accountsStore';
import { get } from 'svelte/store';

export async function load({ url }) {
  const { pathname } = url;
  const accountsArray = get(accounts);
  const isRedirect = envelopeGuard({ pathname, accountsArray });
  const location = '/';
  if (isRedirect) {
    throw envelopeGuardRedirect(location);
  } else {
    return {
      key: pathname,
    };
  }
}

// file: src/lib/utils/guards.ts

import { redirect } from '@sveltejs/kit';
export function envelopeGuard({ pathname, accountsArray }) {
  if (
    accountsArray &&
    accountsArray.length === 0 &&
    pathname === '/create-envelope'
  ) {
    return true;
  }
  return false;
}

export const envelopeGuardRedirect = (location: string) =>
  redirect(302, location);

Then when triggering the guard and navigating to /create-envelope I get:

Uncaught (in promise) Redirect {status: 302, location: '/'}

[WaltzingPenguin]

As a simple workaround I've done the following:

const location = '/login';
if (browser) return await goto(location);
else throw redirect(302, location);

This worked for me in browser and SSR, but yes ideally redirect() should work client side.

Using goto as a workaround breaks as soon as a link has sveltekit:prefetch enabled. Hovering over the link will immediately redirect without requiring a click.

[Rich-Harris]

This is happening because of a failed instanceof Redirect check (SvelteKit's internals need to know if the object you threw from your load is a redirect). It's failing because the module that declares Redirect is being imported twice.

This line in client.js...

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

Line 19 in eedc333

import { HttpError, Redirect } from '../../index/private.js';

...is being transformed by Vite into this:

import { HttpError, Redirect } from '/node_modules/.pnpm/@[email protected][email protected][email protected]/node_modules/@sveltejs/kit/src/index/private.js';

Later, when your code gets imported, it's turned into this:

import { base } from "/node_modules/.pnpm/@[email protected][email protected][email protected]/node_modules/@sveltejs/kit/src/runtime/app/paths.js";
import { redirect } from "/node_modules/.vite/deps/@sveltejs_kit.js?v=5bea743a";
export function load() {
  throw redirect(307, base + "/about");
}

That "/node_modules/.vite/deps/@sveltejs_kit.js?v=5bea743a" module is an esbuild bundle that includes the Redirect class. So the Redirect you have a hold of is totally different to the one imported by client.js.

We can prevent the esbuild stuff from happening by adding optimizeDeps.exclude: ['@sveltejs/kit'] to the Vite config, via the SvelteKit plugin. But we still end up with two copies of the module:

// from client.js
import { HttpError, Redirect } from '/node_modules/.pnpm/@[email protected][email protected][email protected]/node_modules/@sveltejs/kit/src/index/private.js';

// from index.js?v=e48ba282, which is imported by your `+page.ts` (it exports `redirect`)
import { HttpError, Redirect } from '/node_modules/.pnpm/@[email protected][email protected][email protected]/node_modules/@sveltejs/kit/src/index/private.js?v=e48ba282';

You'll notice it's the same URL, but with a cachebusting query parameter. I'm at the limits of my Vite knowledge, but maybe @bluwy or @dominikg would understand how to ensure that the query parameter is always present or always absent? I suspect it's related to the fact that we're importing client/start.js in the SSR'd HTML without going via transformIndexHtml.

[benmccann]

This is outside the portions of Vite I'm familiar with. Hopefully @bluwy or @dominikg will know. Leaving a few pointers here of what I did find. It looks to me like it has something to do with optimized deps as you mentioned:

https://github.com/vitejs/vite/blob/05712323b292492e9161a6ff7b20bfce43097dfb/packages/vite/src/node/optimizer/optimizer.ts#L115

Also, there's a method that can be used for removing it:

https://github.com/vitejs/vite/blob/05712323b292492e9161a6ff7b20bfce43097dfb/packages/vite/src/node/plugins/importAnalysis.ts#L493

As to whether it should always or never be there, I'm not sure.

[enyo]

As a simple workaround I created a function for now:

import { browser } from '$app/env'
import { redirect } from '@sveltejs/kit'
import { goto } from '$app/navigation'

const redirectWorkaround: typeof redirect = (status, location) => {
  if (!browser) throw redirect(status, location)
  else {
    goto(location)
  }
}

EDIT: as @madeleineostoja pointed out: make sure you don't have sveltekit:prefetch enabled for these routes if you use this workaround.

[madeleineostoja]

as @WaltzingPenguin said, using goto as a workaround breaks with sveltekit:prefetch which will instantly trigger the redirect

[bdanoit]

It would be nice if Redirect also prevents child load functions that are awaiting the current load function from executing

[Rich-Harris]

@bdanoit agree, but please don't piggyback on unrelated issues — something like that deserves a new issue with a repro

[bluwy]

You'll notice it's the same URL, but with a cachebusting query parameter. I'm at the limits of my Vite knowledge, but maybe @bluwy or @dominikg would understand how to ensure that the query parameter is always present or always absent? I suspect it's related to the fact that we're importing client/start.js in the SSR'd HTML without going via transformIndexHtml.

IFAIA ?v= is usually added for prebundled deps only, and since we've excluded it from prebundling, plus it's in SSR, it's even weird that this is happening when it shouldn't.

I'm not sure yet if transformIndexHtml affects things here (I'm guessing unlikely), but this part could be the culprit from looking around:

kit/packages/kit/src/vite/dev/index.js

Line 66 in eecbbfc

file: `/@fs${runtime_prefix}/client/start.js`,

I might have to spin up locally to see what's up.

[dummdidumm]

With regards to Windows: Maybe this needs the same treatment as this?

kit/packages/kit/src/vite/dev/index.js

Lines 290 to 295 in eecbbfc

	const runtime_base = runtime_directory.startsWith(process.cwd())
	? `/${path.relative('.', runtime_directory)}`
	: `/@fs${
	// Windows/Linux separation - Windows starts with a drive letter, we need a / in front there
	runtime_directory.startsWith('/') ? '' : '/'
	}${runtime_directory}`;

[Rich-Harris]

this part could be the culprit from looking around:

a hunch i haven't yet tested is that if we used the same runtime_base logic for client/start.js as we do for other things, everything would get resolved the same way.

The logic is basically this: if @sveltejs/kit is installed inside the project, do this...

import { start } from '/node_modules/@sveltejs/kit/src/runtime/client/start.js';

...otherwise (i.e. it's in a workspace root), use /@fs:

import { start } from '/@fs/path/to/repo/node_modules/@sveltejs/kit/src/runtime/client/start.js';

As I understand it that would match Vite's internal resolution logic more closely. As things stand we're doing some things with /node_modules/... and other things with /@fs, which might be at least part of the problem here.

[benmccann]

We should watch to see if this fixes it: vitejs/vite#9730 and vitejs/vite#9759

update from blu: it doesn't fix it

[madeleineostoja]

Is there any way to mark a route as “never prefetch” with goto as a workaround in the meantime? This has pretty thoroughly broken the large app I was halfway through migrating/refactoring so I’d be all for a hacky solution to duct tape it together for now

[f-elix]

I think calling goto in the +page.svelte file should work. Its far from ideal but if it works it can be a temporary workaround.

[meteorlxy]

You'll notice it's the same URL, but with a cachebusting query parameter. I'm at the limits of my Vite knowledge, but maybe @bluwy or @dominikg would understand how to ensure that the query parameter is always present or always absent?

@Rich-Harris This looks quite similar with vitejs/vite#7621. We are using absolute fs paths to import modules, too, and the version hash made the same module to be different in the browser.

The reason is something like:

• When a module is imported via "bare import" (e.g. import '@sveltejs/kit'), it will be recognized as a dependency and the version hash will be injected.
• When a module is imported via absolute file path (e.g. import '/path/to/node_modules/@sveltejs/kit'), it will not be recognized as a dep so the version hash will not be injected.

If the version hash is the root cause, I think vitejs/vite#9730 might help. But @benmccann said no, so there might have some other issues here 🤔

[WaltzingPenguin]

Is it worth a temporary patch to remove the use of instanceof from SvelteKit until the correct solution can be figured out with Vite? I have encountered a similar issue in the past and just used Symbols to work around it, instead of digging into Vite internals.

const redirect_symbol = Symbol.for('sveltekit:redirect')

export class Redirect {
	[redirect_symbol] = true;
	constructor(status, location) {
		this.status = status;
		this.location = location;
	}
}

export function is_redirect(obj) {
	return (redirect_symbol in obj)
}

[f-elix]

I agree, this bug is preventing me from migrating my app because it breaks key functionalities.

[Rich-Harris]

I've opened an issue on the Vite repo:

• a script tag referencing node_modules does not get browserHash injected causing double-instantiation vitejs/vite#9828
• https://github.com/Rich-Harris/vite-browserhash-repro

Would love to try and get it fixed properly rather than bodging in a workaround, as part of the reason for the instanceof checks is that TypeScript throws a hissy fit without them

[Rich-Harris]

Progress update — there's now an PR against Vite that fixes this issue: vitejs/vite#9848

[maximedupre]

vitejs/vite#9848

It's merged 🥹🎉

[madeleineostoja]

Just bumped all my deps and it's working perfectly for me, looks like this issue can be closed 🎉

[WaltzingPenguin]

Just bumped all my deps and it's working perfectly for me, looks like this issue can be closed 🎉

Retested the initial reproduction with these packages:

npmPackages:
    @sveltejs/adapter-auto: next => 1.0.0-next.67
    @sveltejs/kit: next => 1.0.0-next.445
    svelte: ^3.49.0 => 3.49.0
    vite: ^3.0.9 => 3.0.9

Error is still present.

[timothycohen]

npmPackages:
    @sveltejs/adapter-auto: next => 1.0.0-next.67
    @sveltejs/kit: next => 1.0.0-next.445
    svelte: ^3.49.0 => 3.49.0
    vite: ^3.0.9 => 3.0.9

Error is still present.

It's been merged to main, but not released.

[madeleineostoja]

Oh, then I have no idea why my env started working. Sorry for the false positive

[madeleineostoja]

Okay NOW it’s released, but in the 3.1.0-beta.1 so unless sveltekit wants to update a peer dep to a beta we’ll have to wait for a mainline 3.1 release.

It might be worth bumping vite now while leaving this open to track, since it’s such a breaking issue on sveltekit. Or at least mentioning it in the warning in the docs so users can override the version themselves

[ShravanKumar-dev97]

There should be at update/warning update for users about vite new version(3.1.0-beta.1).
yes, vite beta version casing GitHub action fail.

[Graciasjr]

Just do a server side redirect