Skip to content

Commit 033b6f1

Browse files
fix: navigate from the redirect carrier, not a Location string sniff
Server-function redirects arrive as X-Server-Function-Redirect with the target resolved to an absolute url (solidjs/solid#3102), so the soft/hard split becomes an origin comparison: same-origin navigates softly with replace: true (the target takes the submission's place in history, as HTTP gives a form post), cross-origin hard-navigates. The old branch decided a redirect's fate by whether its target was spelled absolutely (solidjs/solid#3107). A locally-produced redirect() still navigates from its real 3xx + Location; a Location on any other status (a 201's created-at) is data and never navigates. Co-authored-by: Cursor <cursoragent@cursor.com>
1 parent 7ff74bf commit 033b6f1

5 files changed

Lines changed: 99 additions & 19 deletions

File tree

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
---
2+
"@solidjs/router": patch
3+
---
4+
5+
Navigate from the redirect carrier instead of sniffing Location. Server-function redirects now arrive as `X-Server-Function-Redirect: <status> <resolved-url>` (solidjs/solid#3102), so the soft/hard split is a real origin comparison — same-origin targets navigate softly with `replace: true` (the target takes the submission's place in history, matching HTTP's form-post semantics), anything else hard-navigates — never a guess from how the author spelled the target, which sent relative and absolute spellings down different navigation paths (solidjs/solid#3107). A locally-produced `redirect()` (a client-side action) still navigates from its real 3xx + Location; a `Location` on any other status is the author's data and never navigates.

src/data/action.ts

Lines changed: 37 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -2,8 +2,10 @@ import { $TRACK, action as createSolidAction, createMemo, onCleanup, getOwner }
22
import { isResponseEnvelope, isServer, REVALIDATE_HEADER, type JSX } from "@solidjs/web";
33
import {
44
createServerReference,
5+
decodeRedirectHeaderValue,
56
decodeResponsePayload,
67
parseServerFunctionUrl,
8+
REDIRECT_HEADER,
79
subscribeFlightData
810
} from "@solidjs/web/server-functions";
911
// The explicit /server specifier is safe here: the only call site is
@@ -414,6 +416,11 @@ async function settleActionResult<T>(result: T | Promise<T> | AsyncIterable<T>)
414416
// again and wipe the freshly seeded cache).
415417
let flightApplications = 0;
416418

419+
// The statuses fetch follows (Fetch §2.2.3) — the set the server masks into
420+
// the redirect carrier for scripted calls, and the set a locally-produced
421+
// redirect() envelope wears for real.
422+
const REDIRECT_STATUSES = new Set([301, 302, 303, 307, 308]);
423+
417424
/**
418425
* Registers the router as the single-flight consumer of the server function
419426
* transport. Subscribing is the opt-in: while registered, the transport
@@ -434,10 +441,11 @@ export function setupFlightDataConsumer(router: RouterContext) {
434441

435442
/**
436443
* Applies a server function response's integration metadata: `X-Revalidate`
437-
* keys invalidate, `Location` navigates (hard for absolute urls), flight
438-
* data seeds the query cache, and matching entries revalidate. Shared by
439-
* the flight-data consumer and the action response path (which still sees
440-
* metadata-bearing responses when no flight data was collected).
444+
* keys invalidate, the redirect carrier navigates (soft when same-origin,
445+
* hard otherwise), flight data seeds the query cache, and matching entries
446+
* revalidate. Shared by the flight-data consumer and the action response
447+
* path (which still sees metadata-bearing responses when no flight data was
448+
* collected).
441449
*/
442450
function applyResponseMetadata(
443451
metadata: Response | undefined,
@@ -448,12 +456,32 @@ function applyResponseMetadata(
448456
if (metadata) {
449457
if (metadata.headers.has(REVALIDATE_HEADER))
450458
keys = metadata.headers.get(REVALIDATE_HEADER)!.split(",");
451-
if (metadata.headers.has("Location")) {
452-
const locationUrl = metadata.headers.get("Location") || "/";
453-
if (locationUrl.startsWith("http")) {
454-
window.location.href = locationUrl;
459+
// The carrier delivers the target RESOLVED to an absolute url
460+
// (solidjs/solid#3102), so the soft/hard split is a real origin
461+
// comparison — never a guess from how the author spelled the target,
462+
// which sent `redirect("/")` and `redirect(new URL("/", url).href)`
463+
// down different navigation paths (solidjs/solid#3107). A redirect
464+
// produced locally (a client-side action's `redirect()`) never crossed
465+
// the wire, so no carrier was attached: it is the real 3xx with its
466+
// Location, resolved against the page it runs in. A `Location` on any
467+
// other status is the author's data (a 201's created-at) and never
468+
// navigates. Same-origin targets navigate softly under the router;
469+
// anything else leaves the app, so the document goes with it.
470+
// `replace` matches what HTTP gives a form post: the target takes the
471+
// submission's place in history rather than stacking on it.
472+
const carried = decodeRedirectHeaderValue(metadata.headers.get(REDIRECT_HEADER));
473+
const local =
474+
!carried && REDIRECT_STATUSES.has(metadata.status) && metadata.headers.get("Location");
475+
const target = carried
476+
? new URL(carried.url)
477+
: local
478+
? new URL(local, window.location.href)
479+
: undefined;
480+
if (target) {
481+
if (target.origin === window.location.origin) {
482+
navigate(target.pathname + target.search + target.hash, { replace: true });
455483
} else {
456-
navigate(locationUrl);
484+
window.location.href = target.href;
457485
}
458486
}
459487
}

test/data/action.spec.ts

Lines changed: 14 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -222,15 +222,17 @@ describe("action", () => {
222222
const navigate = vi.fn();
223223
mockRouterContext.navigatorFactory = () => navigate;
224224

225+
// a real redirect: 302 + Location, what redirect() produces — a
226+
// Location on a non-redirect status is data and never navigates
225227
const redirectAction = action(
226-
async () => new Response(null, { headers: { Location: "/next" } }),
228+
async () => new Response(null, { status: 302, headers: { Location: "/next" } }),
227229
{ name: "redirect-settled-test" }
228230
).onSettled(onSettled);
229231

230232
const boundAction = useAction(redirectAction);
231233
await boundAction();
232234

233-
expect(navigate).toHaveBeenCalledWith("/next");
235+
expect(navigate).toHaveBeenCalledWith("/next", { replace: true });
234236
expect(onSettled).toHaveBeenCalledTimes(1);
235237
expect(mockRouterContext.submissions[0]()).toHaveLength(0);
236238
});
@@ -799,7 +801,16 @@ describe("generic server actions", () => {
799801
return {};
800802
}) as any;
801803
originalFetch = global.fetch;
802-
fetchMock = vi.fn(async () => new Response(null, { headers: { Location: "/after" } }));
804+
// the wire shape of a scripted redirect: masked 200 with the carrier
805+
// holding the author's status and the resolved target
806+
fetchMock = vi.fn(
807+
async () =>
808+
new Response(null, {
809+
headers: {
810+
"X-Server-Function-Redirect": `302 ${new URL("/after", window.location.href).href}`
811+
}
812+
})
813+
);
803814
global.fetch = fetchMock as any;
804815
});
805816

test/data/flight-consumer.spec.ts

Lines changed: 22 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -7,9 +7,27 @@ import { createMockRouter } from "../helpers.js";
77
// transport's part and deliver single-flight payloads to it directly.
88
let consumer: FlightDataConsumer<Record<string, any>> | undefined;
99

10+
// The wire shape of a scripted redirect: masked 200 with the carrier holding
11+
// the author's status and the target resolved to an absolute url.
12+
const carrier = (target: string, status = 302) => ({
13+
"X-Server-Function-Redirect": `${status} ${new URL(target, window.location.href).href}`
14+
});
15+
1016
vi.mock("@solidjs/web/server-functions", () => ({
1117
decodeResponse: vi.fn(),
1218
decodeResponsePayload: vi.fn(),
19+
// the redirect carrier, mirrored from the runtime (wire format:
20+
// "<status> <absolute-url>")
21+
REDIRECT_HEADER: "X-Server-Function-Redirect",
22+
decodeRedirectHeaderValue: (value: string | null | undefined) => {
23+
if (typeof value !== "string") return undefined;
24+
const at = value.indexOf(" ");
25+
if (at < 0) return undefined;
26+
const status = Number(value.slice(0, at));
27+
const url = value.slice(at + 1);
28+
if (!Number.isInteger(status) || !url) return undefined;
29+
return { status, url };
30+
},
1331
// consumed by data/query.ts, which shares this module graph
1432
isServerFunction: () => false,
1533
getServerFunctionMetadata: () => undefined,
@@ -59,9 +77,9 @@ describe("setupFlightDataConsumer", () => {
5977
setupFlightDataConsumer(router);
6078
await consumer!(
6179
{ "notes[]": ["destination data"] },
62-
{ response: new Response(null, { headers: { Location: "/notes" } }) }
80+
{ response: new Response(null, { headers: carrier("/notes") }) }
6381
);
64-
expect(navigate).toHaveBeenCalledWith("/notes");
82+
expect(navigate).toHaveBeenCalledWith("/notes", { replace: true });
6583
expect(query.get("notes[]")).toEqual(["destination data"]);
6684
});
6785

@@ -110,13 +128,13 @@ describe("setupFlightDataConsumer", () => {
110128
const save = async () => {
111129
await consumer!(
112130
{ "layout[]": "fresh-layout" },
113-
{ response: new Response(null, { headers: { Location: "/dash/b" } }) }
131+
{ response: new Response(null, { headers: carrier("/dash/b") }) }
114132
);
115133
return "saved";
116134
};
117135
await action(save, "keyless-save").call({ r: router });
118136

119-
expect(navigate).toHaveBeenCalledWith("/dash/b");
137+
expect(navigate).toHaveBeenCalledWith("/dash/b", { replace: true });
120138
expect(await layout()).toBe("fresh-layout");
121139
expect(fetchLayout).toHaveBeenCalledTimes(1);
122140
});

test/data/flight-redirect.spec.ts

Lines changed: 21 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,18 @@ let consumer: FlightDataConsumer<Record<string, any>> | undefined;
1010
vi.mock("@solidjs/web/server-functions", () => ({
1111
decodeResponse: vi.fn(),
1212
decodeResponsePayload: vi.fn(),
13+
// the redirect carrier, mirrored from the runtime (wire format:
14+
// "<status> <absolute-url>")
15+
REDIRECT_HEADER: "X-Server-Function-Redirect",
16+
decodeRedirectHeaderValue: (value: string | null | undefined) => {
17+
if (typeof value !== "string") return undefined;
18+
const at = value.indexOf(" ");
19+
if (at < 0) return undefined;
20+
const status = Number(value.slice(0, at));
21+
const url = value.slice(at + 1);
22+
if (!Number.isInteger(status) || !url) return undefined;
23+
return { status, url };
24+
},
1325
// consumed by data/query.ts, which shares this module graph
1426
isServerFunction: () => false,
1527
getServerFunctionMetadata: () => undefined,
@@ -21,6 +33,12 @@ vi.mock("@solidjs/web/server-functions", () => ({
2133
}
2234
}));
2335

36+
// The wire shape of a scripted redirect: masked 200 with the carrier holding
37+
// the author's status and the target resolved to an absolute url.
38+
const carrier = (target: string, status = 302) => ({
39+
"X-Server-Function-Redirect": `${status} ${new URL(target, window.location.href).href}`
40+
});
41+
2442
// Spy on the sweep: these tests pin the ordering the action layer applies to
2543
// a flight response — invalidate, seed, navigate, then sweep synchronously —
2644
// not the query cache mechanics (covered by query.spec.ts and
@@ -70,7 +88,7 @@ describe("redirecting flight responses", () => {
7088
});
7189
await consumer!(
7290
{ "note[0]": { title: "fresh" } },
73-
{ response: new Response(null, { headers: { Location: "/notes/0" } }) }
91+
{ response: new Response(null, { headers: carrier("/notes/0") }) }
7492
);
7593
expect(sweptDuringApply).toBe(true);
7694
expect(sweepSpy).toHaveBeenCalledTimes(1);
@@ -82,7 +100,7 @@ describe("redirecting flight responses", () => {
82100
{ "notes[]": ["fresh"] },
83101
{
84102
response: new Response(null, {
85-
headers: { Location: "/notes", "X-Revalidate": "notes" }
103+
headers: { ...carrier("/notes"), "X-Revalidate": "notes" }
86104
})
87105
}
88106
);
@@ -109,7 +127,7 @@ describe("redirecting flight responses", () => {
109127
await consumer!(
110128
{},
111129
{
112-
response: new Response(null, { headers: { Location: "https://elsewhere.example/" } })
130+
response: new Response(null, { headers: carrier("https://elsewhere.example/") })
113131
}
114132
);
115133
expect(navigate).not.toHaveBeenCalled();

0 commit comments

Comments
 (0)