solidjs / solid-start

SolidStart, the Solid app framework
https://start.solidjs.com
MIT License
4.93k stars 371 forks source link

[Bug?]: throwing redirect in route load function is not handled #1543

Closed maksimsemenov closed 1 week ago

maksimsemenov commented 2 weeks ago

Duplicates

Latest version

Current behavior 😯

I'm trying to throw a redirect in the route load function:


const loadData = () => {
    'use server'
    throw redirect('/about')
}

export const route = {
    load: () => loadData()
}

When I try to navigate to that page, instead of redirecting, it throws a server error:

Error: Unknown error
[cause]: _Response [Response] {
    [Symbol(realm)]: { settingsObject: {} },
    [Symbol(state)]: {
      aborted: false,
      rangeRequested: false,
      timingAllowPassed: false,
      requestIncludesCredentials: false,
      type: 'default',
      status: 302,
      timingInfo: null,
      cacheState: '',
      statusText: '',
      headersList: [_HeadersList],
      urlList: []
    },
    [Symbol(headers)]: _HeadersList {
      cookies: null,
      [Symbol(headers map)]: [Map],
      [Symbol(headers map sorted)]: [Array]
    }
  }

Expected behavior 🤔

It should redirect me to the page in the redirect

Steps to reproduce 🕹

I created a repository with an example: https://github.com/maksimsemenov/solid-start-redirect?tab=readme-ov-file

Steps to reproduce:

  1. Install dependencies
  2. Run solid-start
  3. Navigate to the / route.

Context 🔦

I initiated project with npm init solid@latest.

I also tried just to return redirect instead of throwing it. In this case there is no server error, but no redirect either.

I found a few similar issues that are are resolved. But it seems that the issue is back

Your environment 🌎

npm packages:
    "@solidjs/meta": "^0.29.4",
    "@solidjs/router": "^0.13.3",
    "@solidjs/start": "^1.0.1",
    "solid-js": "^1.8.17",
    "vinxi": "^0.3.11"

node: v20.9.0
npm: v10.1.0
brenelz commented 1 week ago

So a couple things you need to do. You need to return instead of throw the redirect. Secondly you need to wrap in the cache function. Also you need to call the function in createAsync()

const loadData = cache(() => {
  'use server'
  return redirect('/about')
}, "load-data");

export const route = {
  load: () => loadData()
}

export default function Home() {
  const data = createAsync(() => loadData());
}
Brendonovich commented 1 week ago

Throwing the redirect also works, but the important part is calling loadData outside of the load function, since thrown/returned redirects will be ignored when called from load. https://stackblitz.com/edit/github-fk8exj-mcht9p?file=src%2Froutes%2Findex.tsx

brenelz commented 1 week ago

I feel this is intended behavior at the moment. Thinking we can probably close this issue?

maksimsemenov commented 1 week ago

What if I don't want to cache the request? For example, if I want to validate some auth-link with some hash parameter, and then want to redirect user. I don't think I want to cache the response in this case.

Brendonovich commented 1 week ago

Use action, it's for pretty much everything that isn't a data fetch

maksimsemenov commented 1 week ago

Can I use action on page load? The use-case I'm trying to solve is:

  1. I send email to the user with a unique link
  2. User clicks the link and is taken to the link validation page
  3. I validate link on the server
  4. If validation is successful, I redirect the user
  5. If validation is not successful, I want to show some error state with some next action, so user is not lost.

I also, don't want to cash that response, so if link is clicked twice, it is no longer valid

Brendonovich commented 1 week ago

Yeah that should work fine

maksimsemenov commented 1 week ago

So should it be like this:

const loadData = action(() => {
  'use server'
  return redirect('/about')
});

export const route = {
  load: () => loadData()
}

export default function Home() {
  const data = createAsync(() => loadData());
}
ryansolid commented 1 week ago

Yeah this is by design since the load function is more of like a lifecycle hook and the server function is just an async function. One could catch the errored response themselves and then call the client side navigation APIs with it but I don't know if that is what you are going for. The name cache is probably misleading because on the server it only lasts the lifetime of the request. It is possible it would just work. We're reviewing some of the naming.

I think an action is fine, but I think cache more fits the semantics because it is getting is validated on a hash. But either probably is fine here.