Skip to content

Commit 1b2e3ef

Browse files
Varixowmertens
authored andcommitted
fix: correctly handle redirect
1 parent ad96d9d commit 1b2e3ef

File tree

8 files changed

+144
-38
lines changed

8 files changed

+144
-38
lines changed

packages/qwik-router/src/middleware/request-handler/handlers/loader-handler.ts

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -49,7 +49,6 @@ export function loaderDataHandler(routeLoaders: LoaderInternal[]): RequestHandle
4949
maxAge: 365 * 24 * 60 * 60, // 1 year
5050
});
5151

52-
// return loader data: id and route
5352
const loaderData = routeLoaders.map((l) => {
5453
const loaderId = l.__id;
5554
let loaderRoute = qwikRouterConfig.loaderIdToRoute[loaderId];

packages/qwik-router/src/middleware/request-handler/handlers/path-handler.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,7 @@ import { HttpStatus } from '../http-status-codes';
55
export function fixTrailingSlash(ev: RequestEvent) {
66
const { basePathname, originalUrl, sharedMap } = ev;
77
const { pathname, search } = originalUrl;
8-
const isQData = isQDataRequestBasedOnSharedMap(sharedMap);
8+
const isQData = isQDataRequestBasedOnSharedMap(sharedMap, ev.request.headers);
99
if (!isQData && pathname !== basePathname && !pathname.endsWith('.html')) {
1010
// only check for slash redirect on pages
1111
if (!globalThis.__NO_TRAILING_SLASH__) {

packages/qwik-router/src/middleware/request-handler/handlers/path-handler.unit.ts

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -26,6 +26,9 @@ describe('fixTrailingSlash', () => {
2626
originalUrl: new URL('http://localhost/test'),
2727
sharedMap: new Map(),
2828
redirect: vi.fn(),
29+
request: {
30+
headers: {},
31+
},
2932
} as unknown as Mocked<RequestEvent>;
3033

3134
// Set up default mocks

packages/qwik-router/src/middleware/request-handler/handlers/redirect-handler.ts

Lines changed: 7 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,9 +1,12 @@
11
import { RedirectMessage, RequestEvent } from '@qwik.dev/router/middleware/request-handler';
2-
import { OriginalQDataName } from '../user-response';
32
import { isQDataRequestBasedOnSharedMap } from '../resolve-request-handlers';
3+
import { QDATA_JSON, QManifestHash } from '../user-response';
44

55
export async function handleRedirect(requestEv: RequestEvent) {
6-
const isPageDataReq = isQDataRequestBasedOnSharedMap(requestEv.sharedMap);
6+
const isPageDataReq = isQDataRequestBasedOnSharedMap(
7+
requestEv.sharedMap,
8+
requestEv.request.headers
9+
);
710
if (!isPageDataReq) {
811
return;
912
}
@@ -40,7 +43,8 @@ function makeQDataPath(href: string, sharedMap: Map<string, unknown>) {
4043
if (href.startsWith('/')) {
4144
const url = new URL(href, 'http://localhost');
4245
const pathname = url.pathname.endsWith('/') ? url.pathname.slice(0, -1) : url.pathname;
43-
const append = sharedMap.get(OriginalQDataName) as string;
46+
const manifestHash = sharedMap.get(QManifestHash) as string;
47+
const append = manifestHash ? `/q-loader-data.${manifestHash}.json` : QDATA_JSON;
4448

4549
if (!append) {
4650
return undefined;

packages/qwik-router/src/middleware/request-handler/request-event.ts

Lines changed: 7 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -38,12 +38,12 @@ import {
3838
IsQLoader,
3939
IsQLoaderData,
4040
LOADER_REGEX,
41-
OriginalQDataName,
4241
Q_LOADER_DATA_REGEX,
4342
QActionId,
4443
QDATA_JSON,
4544
QDATA_JSON_LEN,
4645
QLoaderId,
46+
QManifestHash,
4747
} from './user-response';
4848

4949
const RequestEvLoaders = Symbol('RequestEvLoaders');
@@ -85,7 +85,9 @@ export function createRequestEvent(
8585
const requestRecognized = recognizeRequest(url.pathname);
8686
if (requestRecognized) {
8787
sharedMap.set(requestRecognized.type, true);
88-
sharedMap.set(OriginalQDataName, requestRecognized.originalQDataName);
88+
if (requestRecognized.manifestHash) {
89+
sharedMap.set(QManifestHash, requestRecognized.manifestHash);
90+
}
8991
if (requestRecognized.type === IsQLoader && requestRecognized.data) {
9092
sharedMap.set(QLoaderId, requestRecognized.data.loaderId);
9193
}
@@ -487,7 +489,7 @@ export function recognizeRequest(pathname: string) {
487489
return {
488490
type: IsQData,
489491
trimLength: QDATA_JSON_LEN,
490-
originalQDataName: QDATA_JSON,
492+
manifestHash: null,
491493
data: null,
492494
};
493495
}
@@ -498,7 +500,7 @@ export function recognizeRequest(pathname: string) {
498500
return {
499501
type: IsQLoaderData,
500502
trimLength: loaderDataMatch[0].length,
501-
originalQDataName: loaderDataMatch[1],
503+
manifestHash: loaderDataMatch[2],
502504
data: null,
503505
};
504506
}
@@ -508,7 +510,7 @@ export function recognizeRequest(pathname: string) {
508510
return {
509511
type: IsQLoader,
510512
trimLength: loaderMatch[0].length,
511-
originalQDataName: loaderMatch[1],
513+
manifestHash: loaderMatch[3],
512514
data: { loaderId: loaderMatch[2] },
513515
};
514516
}

packages/qwik-router/src/middleware/request-handler/resolve-request-handlers.ts

Lines changed: 15 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -28,7 +28,7 @@ import {
2828
} from './request-event';
2929
import { getQwikRouterServerData } from './response-page';
3030
import type { RequestEvent, RequestEventBase, RequestHandler } from './types';
31-
import { IsQData, IsQLoader, IsQLoaderData, QActionId } from './user-response';
31+
import { IsQAction, IsQData, IsQLoader, IsQLoaderData, QActionId } from './user-response';
3232

3333
export const resolveRequestHandlers = (
3434
serverPlugins: RouteModule[] | undefined,
@@ -75,7 +75,6 @@ export const resolveRequestHandlers = (
7575
requestHandlers.unshift(csrfCheckMiddleware);
7676
}
7777
}
78-
requestHandlers.push(handleRedirect);
7978
if (isPageRoute) {
8079
if (method === 'POST' || method === 'GET') {
8180
// server$
@@ -87,6 +86,7 @@ export const resolveRequestHandlers = (
8786
ev.sharedMap.set(RequestRouteName, routeName);
8887
});
8988
requestHandlers.push(fixTrailingSlash);
89+
requestHandlers.push(handleRedirect);
9090
requestHandlers.push(loaderDataHandler(routeLoaders));
9191
requestHandlers.push(loaderHandler(routeLoaders));
9292
requestHandlers.push(actionHandler(routeActions));
@@ -170,8 +170,15 @@ export const checkBrand = (obj: any, brand: string) => {
170170
return obj && typeof obj === 'function' && obj.__brand === brand;
171171
};
172172

173-
export function isQDataRequestBasedOnSharedMap(sharedMap: Map<string, unknown>) {
174-
return sharedMap.has(IsQData) || sharedMap.has(IsQLoaderData) || sharedMap.has(IsQLoader);
173+
export function isQDataRequestBasedOnSharedMap(sharedMap: Map<string, unknown>, headers: Headers) {
174+
return (
175+
sharedMap.has(IsQData) ||
176+
sharedMap.has(IsQLoaderData) ||
177+
sharedMap.has(IsQLoader) ||
178+
(sharedMap.has(IsQAction) &&
179+
// we need to ignore actions without JS enabled and render the page
180+
headers.get('accept')?.includes('application/json'))
181+
);
175182
}
176183

177184
export function verifySerializable(data: any, qrl: QRL) {
@@ -219,7 +226,10 @@ export function renderQwikMiddleware(render: Render) {
219226
if (requestEv.headersSent) {
220227
return;
221228
}
222-
const isPageDataReq = isQDataRequestBasedOnSharedMap(requestEv.sharedMap);
229+
const isPageDataReq = isQDataRequestBasedOnSharedMap(
230+
requestEv.sharedMap,
231+
requestEv.request.headers
232+
);
223233
if (isPageDataReq) {
224234
return;
225235
}

packages/qwik-router/src/middleware/request-handler/user-response.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -188,7 +188,7 @@ export const IsQAction = '@isQAction';
188188
export const QLoaderId = '@loaderId';
189189
export const QActionId = '@actionId';
190190
export const IsQLoaderData = '@isQLoaderData';
191-
export const OriginalQDataName = '@originalQDataName';
191+
export const QManifestHash = '@manifestHash';
192192
export const QDATA_JSON = '/q-data.json';
193193
export const QDATA_JSON_LEN = QDATA_JSON.length;
194194
export const Q_LOADER_DATA_REGEX = /\/(q-loader-data\.(.+)\.json)$/;

packages/qwik-router/src/runtime/src/use-endpoint.ts

Lines changed: 110 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -1,17 +1,29 @@
1-
import { _deserialize } from '@qwik.dev/core/internal';
1+
import { _deserialize, isDev } from '@qwik.dev/core/internal';
22
import type { QData } from '../../middleware/request-handler/handlers/qdata-handler';
33
import { preloadRouteBundles } from './client-navigate';
44
import { QACTION_KEY } from './constants';
55
import type { ClientPageData, RouteActionValue } from './types';
66

7+
class ShouldRedirect<T> {
8+
constructor(
9+
public location: string,
10+
public data: T
11+
) {}
12+
}
13+
714
interface LoaderDataResponse {
815
id: string;
916
route: string;
1017
}
1118

19+
interface RedirectContext {
20+
promise: Promise<unknown> | undefined;
21+
}
22+
1223
export const loadClientLoaderData = async (url: URL, loaderId: string, manifestHash: string) => {
1324
const pagePathname = url.pathname.endsWith('/') ? url.pathname : url.pathname + '/';
14-
return fetchLoader(loaderId, pagePathname, manifestHash);
25+
const abortController = new AbortController();
26+
return fetchLoader(loaderId, pagePathname, manifestHash, abortController, { promise: undefined });
1527
};
1628

1729
export const loadClientData = async (
@@ -20,6 +32,7 @@ export const loadClientData = async (
2032
opts?: {
2133
action?: RouteActionValue;
2234
loaderIds?: string[];
35+
redirectData?: ShouldRedirect<LoaderDataResponse[]>;
2336
clearCache?: boolean;
2437
preloadRouteBundles?: boolean;
2538
isPrefetch?: boolean;
@@ -34,33 +47,69 @@ export const loadClientData = async (
3447
let resolveFn: () => void | undefined;
3548
let actionData: unknown;
3649
if (opts?.action) {
37-
const actionResult = await fetchActionData(opts.action, pagePathname, url.searchParams);
38-
actionData = actionResult.data;
39-
resolveFn = () => {
40-
opts.action!.resolve!({ status: actionResult.status, result: actionData });
41-
};
50+
try {
51+
const actionResult = await fetchActionData(opts.action, pagePathname, url.searchParams);
52+
actionData = actionResult.data;
53+
resolveFn = () => {
54+
opts.action!.resolve!({ status: actionResult.status, result: actionData });
55+
};
56+
} catch (e) {
57+
if (e instanceof ShouldRedirect) {
58+
const newUrl = new URL(e.location, url);
59+
const newOpts = {
60+
...opts,
61+
action: undefined,
62+
loaderIds: undefined,
63+
redirectData: e,
64+
};
65+
return loadClientData(newUrl, manifestHash, newOpts);
66+
} else {
67+
throw e;
68+
}
69+
}
4270
} else {
4371
let loaderData: LoaderDataResponse[] = [];
44-
if (!opts?.loaderIds) {
45-
// we need to load all the loaders
46-
// first we need to get the loader urls
47-
loaderData = (await fetchLoaderData(pagePathname, manifestHash)).loaderData;
48-
} else {
72+
if (opts && opts.loaderIds) {
4973
loaderData = opts.loaderIds.map((loaderId) => {
5074
return {
5175
id: loaderId,
5276
route: pagePathname,
5377
};
5478
});
79+
} else if (opts?.redirectData?.data) {
80+
loaderData = opts.redirectData.data;
81+
} else {
82+
// we need to load all the loaders
83+
// first we need to get the loader urls
84+
loaderData = (await fetchLoaderData(pagePathname, manifestHash)).loaderData;
5585
}
5686
if (loaderData.length > 0) {
5787
// load specific loaders
58-
const loaderPromises = loaderData.map((loader) =>
59-
fetchLoader(loader.id, loader.route, manifestHash)
60-
);
61-
const loaderResults = await Promise.all(loaderPromises);
62-
for (let i = 0; i < loaderData.length; i++) {
63-
loaders[loaderData[i].id] = loaderResults[i];
88+
const abortController = new AbortController();
89+
const redirectContext: RedirectContext = { promise: undefined };
90+
try {
91+
const loaderPromises = loaderData.map((loader) =>
92+
fetchLoader(loader.id, loader.route, manifestHash, abortController, redirectContext)
93+
);
94+
const loaderResults = await Promise.all(loaderPromises);
95+
for (let i = 0; i < loaderData.length; i++) {
96+
loaders[loaderData[i].id] = loaderResults[i];
97+
}
98+
} catch (e) {
99+
if (e instanceof ShouldRedirect) {
100+
const newUrl = new URL(e.location, url);
101+
const newOpts = {
102+
...opts,
103+
action: undefined,
104+
loaderIds: undefined,
105+
redirectData: e,
106+
};
107+
return loadClientData(newUrl, manifestHash, newOpts);
108+
} else if (e instanceof Error && e.name === 'AbortError') {
109+
// Expected, do nothing
110+
} else {
111+
throw e;
112+
}
64113
}
65114
}
66115
}
@@ -118,24 +167,57 @@ export async function fetchLoaderData(
118167
): Promise<{ loaderData: LoaderDataResponse[] }> {
119168
const url = `${routePath}q-loader-data.${manifestHash}.json`;
120169
const response = await fetch(url);
170+
171+
if (!response.ok) {
172+
if (isDev) {
173+
throw new Error(`Failed to load loader data for ${routePath}: ${response.status}`);
174+
}
175+
return { loaderData: [] };
176+
}
121177
return response.json();
122178
}
123179

124180
export async function fetchLoader(
125181
loaderId: string,
126182
routePath: string,
127-
manifestHash: string
183+
manifestHash: string,
184+
abortController: AbortController,
185+
redirectContext: RedirectContext
128186
): Promise<unknown> {
129187
const url = `${routePath}q-loader-${loaderId}.${manifestHash}.json`;
130188

131-
const response = await fetch(url);
189+
const response = await fetch(url, {
190+
signal: abortController.signal,
191+
});
192+
193+
if (response.redirected) {
194+
if (!redirectContext.promise) {
195+
redirectContext.promise = response.json();
196+
197+
abortController.abort();
198+
199+
const data = await redirectContext.promise;
200+
// remove the q-loader-XY.json from the url and keep the rest of the url
201+
// the url is like this: https://localhost:3000/q-loader-XY.json
202+
// we need to remove the q-loader-XY.json and keep the rest of the url
203+
// the new url is like this: https://localhost:3000/
204+
const newUrl = new URL(response.url);
205+
newUrl.pathname = newUrl.pathname.replace(`q-loader-data.${manifestHash}.json`, '');
206+
throw new ShouldRedirect(
207+
newUrl.pathname,
208+
(data as { loaderData: LoaderDataResponse[] }).loaderData
209+
);
210+
}
211+
}
132212
if (!response.ok) {
133-
throw new Error(`Failed to load ${loaderId}: ${response.status}`);
213+
if (isDev) {
214+
throw new Error(`Failed to load ${loaderId}: ${response.status}`);
215+
}
216+
return undefined;
134217
}
135218

136219
const text = await response.text();
137220
const [data] = _deserialize(text, document.documentElement) as [Record<string, unknown>];
138-
139221
return data;
140222
}
141223

@@ -159,6 +241,12 @@ export async function fetchActionData(
159241
action.data = undefined;
160242
const response = await fetch(url, fetchOptions);
161243

244+
if (response.redirected) {
245+
const newUrl = new URL(response.url);
246+
newUrl.pathname = newUrl.pathname.replace('q-data.json', '');
247+
throw new ShouldRedirect(newUrl.pathname, undefined);
248+
}
249+
162250
const text = await response.text();
163251
const [data] = _deserialize(text, document.documentElement) as [Record<string, unknown>];
164252

0 commit comments

Comments
 (0)