solidjs / solid-router

A universal router for Solid inspired by Ember and React Router
MIT License
1.1k stars 137 forks source link

Forward absolute redirects inside `cache` from server to client #439

Closed Brendonovich closed 1 month ago

Brendonovich commented 1 month ago

Currently, absolute redirects done inside cache on the server are (funnily enough) only performed when the cached function is also called inside load. I'm not exactly sure how load is involved to make that the case, but this PR seems to fix it.

Instead of server-side absolute redirects just being ignored in handleResponse by an early return, which results in the redirect Response not being serialized and processed on the client inside cache, the early return will only be done if the response has actually been handled. This change results in load not having any effect on the redirect behaviour, and absolute redirects will always be passed through from the server to be handled on the client.

Forcing absolute redirects onto the client works fine and avoids any CORS cross-origin redirect problems, but should there be an option for server-side absolute redirects to be properly supported?

Closes #438

changeset-bot[bot] commented 1 month ago

🦋 Changeset detected

Latest commit: 826311565f32d8c615274c69f5ba019973260664

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

This PR includes changesets to release 1 package | Name | Type | | --------------- | ----- | | @solidjs/router | 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

ryansolid commented 1 month ago

I wonder if we could just support it? This code was copied from old SolidStart. But like router does send back redirects now so maybe that clause could just be removed? Have you tried it?

Brendonovich commented 4 weeks ago

Yeah copying how navigateFromRoute does server redirects seems to work. This does introduce the possibility for some server redirects to get blocked by CORS bc cross-origin, but that's probably something the developer should deal with.

else if (isServer) {
  const e = getRequestEvent()!;
  e.response = { status: 302, headers: new Headers({ Location: url }) };
  return;
}