Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix(router): redirect handling #4543

Merged
merged 3 commits into from
Jun 20, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
2 changes: 1 addition & 1 deletion packages/qwik-city/buildtime/build-layout.unit.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ import { testAppSuite } from '../utils/test-suite';
const test = testAppSuite('Build Layout');

test('total layouts', ({ layouts }) => {
equal(layouts.length, 8, JSON.stringify(layouts, null, 2));
equal(layouts.length, 10, JSON.stringify(layouts, null, 2));
});

test('nested named layout', ({ assertLayout }) => {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -63,6 +63,7 @@ export const resolveRequestHandlers = (
requestHandlers.push(fixTrailingSlash);
requestHandlers.push(renderQData);
}
requestHandlers.push(handleRedirect);
_resolveRequestHandlers(
routeLoaders,
routeActions,
Expand Down Expand Up @@ -458,7 +459,7 @@ export function renderQwikMiddleware(render: Render) {
};
}

export async function renderQData(requestEv: RequestEvent) {
export async function handleRedirect(requestEv: RequestEvent) {
const isPageDataReq = requestEv.sharedMap.has(IsQData);
if (isPageDataReq) {
try {
Expand All @@ -474,7 +475,6 @@ export async function renderQData(requestEv: RequestEvent) {

const status = requestEv.status();
const location = requestEv.headers.get('Location');
const trailingSlash = getRequestTrailingSlash(requestEv);
const isRedirect = status >= 301 && status <= 308 && location;
if (isRedirect) {
const adaptedLocation = makeQDataPath(location);
Expand All @@ -487,6 +487,21 @@ export async function renderQData(requestEv: RequestEvent) {
requestEv.headers.delete('Location');
}
}
}
}

export async function renderQData(requestEv: RequestEvent) {
const isPageDataReq = requestEv.sharedMap.has(IsQData);
if (isPageDataReq) {
await requestEv.next();

if (requestEv.headersSent || requestEv.exited) {
return;
}

const status = requestEv.status();
const location = requestEv.headers.get('Location');
const trailingSlash = getRequestTrailingSlash(requestEv);

const requestHeaders: Record<string, string> = {};
requestEv.request.headers.forEach((value, key) => (requestHeaders[key] = value));
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,5 +2,5 @@ import type { RequestHandler } from '@builder.io/qwik-city';

export const onRequest: RequestHandler<void> = async (onRequestArgs) => {
const { redirect, url } = onRequestArgs;
throw redirect(302, `${url.pathname}route/`);
throw redirect(302, `${url.pathname}/route/`);
};
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
import { component$ } from '@builder.io/qwik';

export default component$(() => (
<div>
<div id="route">
welcome to <code>/broken/route</code>
</div>
));
8 changes: 6 additions & 2 deletions starters/apps/qwikcity-test/src/routes/issue4502/index.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -4,10 +4,14 @@ import { Link } from '@builder.io/qwik-city';
export default component$(() => (
<div>
<p>
<Link href="/qwikcity-test/issue4502/broken">Broken Link</Link>
<Link id="link" href="/qwikcity-test/issue4502/broken">
Link
</Link>
</p>
<p>
<a href="/qwikcity-test/issue4502/broken">Working a</a>
<a id="anchor" href="/qwikcity-test/issue4502/broken">
Anchor
</a>
</p>
</div>
));
13 changes: 13 additions & 0 deletions starters/apps/qwikcity-test/src/routes/issue4502/layout.tsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
import { Slot, component$, useSignal } from '@builder.io/qwik';

export default component$(() => {
const count = useSignal(0);
return (
<div>
<button id="count" onClick$={() => count.value++}>
Count: {count.value}
</button>
<Slot />
</div>
);
});
3 changes: 3 additions & 0 deletions starters/apps/qwikcity-test/src/routes/issue4531/index.tsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
import { component$ } from '@builder.io/qwik';

export default component$(() => <div id="route">should render</div>);
6 changes: 6 additions & 0 deletions starters/apps/qwikcity-test/src/routes/issue4531/layout.tsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
import type { RequestHandler } from '@builder.io/qwik-city';

export const onRequest: RequestHandler<void> = async (onRequestArgs) => {
const { headers } = onRequestArgs;
headers.set('x-qwikcity-test', 'issue4531');
};
25 changes: 25 additions & 0 deletions starters/e2e/qwikcity/nav.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -125,6 +125,18 @@ test.describe('actions', () => {
expect(toPath(page.url())).toEqual('/qwikcity-test/scroll-restoration/page-short/');
expect(await getWindowScrollXY(page)).toStrictEqual([0, scrollHeightShort]);
});

test('issue4502 (link)', async ({ page }) => {
await page.goto('/qwikcity-test/issue4502/');
const count = page.locator('#count');
await expect(count).toHaveText('Count: 0');
await count.click();
await expect(count).toHaveText('Count: 1');
await page.locator('#link').click();
await page.waitForURL('/qwikcity-test/issue4502/broken/route/');
await expect(page.locator('#route')).toHaveText('welcome to /broken/route');
await expect(count).toHaveText('Count: 1');
});
});
}

Expand Down Expand Up @@ -251,6 +263,19 @@ test.describe('actions', () => {
});
});
});

test('issue4502 (anchor)', async ({ page }) => {
await page.goto('/qwikcity-test/issue4502/');
await page.locator('#anchor').click();
await page.waitForURL('/qwikcity-test/issue4502/broken/route/');
await expect(page.locator('#route')).toHaveText('welcome to /broken/route');
});

test('issue4531', async ({ page }) => {
const res = await page.goto('/qwikcity-test/issue4531/');
await expect(page.locator('#route')).toHaveText('should render');
expect(await res?.headerValue('X-Qwikcity-Test')).toEqual('issue4531');
});
}
});

Expand Down