Add server redirect handling

lemonmade · Aug 24, 2024 · 408b582 · 408b582
1 parent 671a20a
commit 408b582
Show file tree

Hide file tree

Showing 3 changed files with 80 additions and 17 deletions.
diff --git a/packages/quilt/source/server/request-router.tsx b/packages/quilt/source/server/request-router.tsx
@@ -19,7 +19,7 @@ import {
   StyleAssetPreload,
 } from '@quilted/preact-browser/server';
 
-import {HTMLResponse, RedirectResponse} from '@quilted/request-router';
+import {HTMLResponse, EnhancedResponse} from '@quilted/request-router';
 
 import {ServerContext} from './ServerContext.tsx';
 
@@ -47,14 +47,14 @@ export interface RenderToResponseOptions {
 export async function renderToResponse(
   element: VNode<any>,
   options: RenderToResponseOptions,
-): Promise<HTMLResponse | RedirectResponse>;
+): Promise<EnhancedResponse>;
 export async function renderToResponse(
   options: RenderToResponseOptions,
-): Promise<HTMLResponse | RedirectResponse>;
+): Promise<EnhancedResponse>;
 export async function renderToResponse(
   optionsOrElement: VNode<any> | RenderToResponseOptions,
   definitelyOptions?: RenderToResponseOptions,
-) {
+): Promise<EnhancedResponse> {
   let element: VNode<any> | undefined;
   let options: RenderToResponseOptions;
 
@@ -94,10 +94,36 @@ export async function renderToResponse(
   let appStream: ReadableStream<any> | undefined;
 
   if (shouldStream === false && element != null) {
-    // TODO: handle redirect
-    const rendered = await renderToStringAsync(
-      <ServerContext browser={browserResponse}>{element}</ServerContext>,
-    );
+    let rendered: string;
+
+    try {
+      rendered = await renderToStringAsync(
+        <ServerContext browser={browserResponse}>{element}</ServerContext>,
+      );
+    } catch (error) {
+      if (error instanceof Response) {
+        const mergedHeaders = new Headers(browserResponse.headers);
+
+        // Copy headers from error response, potentially overwriting existing ones
+        for (const [key, value] of error.headers) {
+          if (key.toLowerCase() === 'set-cookie') continue;
+          mergedHeaders.set(key, value);
+        }
+
+        for (const setCookie of error.headers.getSetCookie()) {
+          mergedHeaders.append('Set-Cookie', setCookie);
+        }
+
+        const mergedResponse = new EnhancedResponse(error.body, {
+          status: Math.max(browserResponse.status.value, error.status),
+          headers: mergedHeaders,
+        });
+
+        return mergedResponse;
+      }
+
+      throw error;
+    }
 
     const appTransformStream = new TransformStream();
     const appWriter = appTransformStream.writable.getWriter();
@@ -115,7 +141,10 @@ export async function renderToResponse(
       const appWriter = appTransformStream.writable.getWriter();
 
       if (element != null) {
-        // TODO: handle redirect
+        // TODO: how could we handle redirects automatically? For now, if people want
+        // this, they will explicitly turn on streaming and will have to use some in-app
+        // to manually handle redirects (e.g., by rendering a script tag that uses JavaScript
+        // to redirect)
         const rendered = await renderToStringAsync(
           <ServerContext browser={browserResponse}>{element}</ServerContext>,
         );

diff --git a/tests/e2e/async.test.ts b/tests/e2e/async.test.ts
@@ -886,10 +886,7 @@ describe('async', () => {
   });
 
   it('can server render components using route loaders', async () => {
-    await using workspace = await createWorkspace({
-      fixture: 'basic-app',
-      debug: true,
-    });
+    await using workspace = await createWorkspace({fixture: 'basic-app'});
 
     await workspace.fs.write({
       'App.tsx': multiline`
@@ -911,7 +908,6 @@ describe('async', () => {
 
     const server = await startServer(workspace);
     const page = await server.openPage('/');
-    console.log(await page.innerHTML('html'));
 
     const content = await page.textContent('body');
     expect(content).toBe(`Data source: server`);

diff --git a/tests/e2e/routing.test.ts b/tests/e2e/routing.test.ts
@@ -6,8 +6,46 @@ import {
   startServer,
 } from './utilities.ts';
 
-describe.skip('routing', () => {
-  describe('scroll restoration', () => {
+describe('routing', () => {
+  it('can redirect between routes', async () => {
+    await using workspace = await createWorkspace({
+      fixture: 'basic-app',
+      debug: true,
+    });
+
+    await workspace.fs.write({
+      'App.tsx': multiline`
+        import {Navigation} from '@quilted/quilt/navigation';
+
+        const routes = [
+          {
+            match: '/',
+            redirect: '/redirected',
+          },
+          {
+            match: '/redirected',
+            render: 'Redirected',
+          },
+        ];
+        
+        export default function App() {
+          return <Navigation routes={routes} />;
+        }
+      `,
+    });
+
+    const server = await startServer(workspace);
+    const page = await server.openPage('/', {
+      javaScriptEnabled: false,
+    });
+
+    const content = await page.textContent('body');
+    expect(content).toBe(`Redirected`);
+
+    expect(page.url()).toBe(new URL('/redirected', server.url).href);
+  });
+
+  describe.skip('scroll restoration', () => {
     it('scrolls to the top when navigating to a new page, and back to its original position when navigating back', async () => {
       await using workspace = await createWorkspace({fixture: 'basic-app'});
 
@@ -325,7 +363,7 @@ describe.skip('routing', () => {
     });
   });
 
-  describe('navigation blocking', () => {
+  describe.skip('navigation blocking', () => {
     it('blocks until a synchronous blocking function calls to allow the navigation to proceed', async () => {
       await using workspace = await createWorkspace({fixture: 'basic-app'});