Improve typings, avoid returning response if unnecessary and add uni…

…t tests (#138)

* Cleanup response returns

* Improve typings and avoid returning response if unnecessary

* More tests

* Go

* Go

* Do not fail-fast

* Prettier :)

* ...

* ..

.github/workflows/tests.yml

@@ -45,6 +45,7 @@ jobs:
     strategy:
       matrix:
         node-version: [14, 16, 18]
+      fail-fast: false
     steps:
       - name: Checkout Master
         uses: actions/checkout@v3

packages/server/README.md

@@ -119,7 +119,7 @@ app.route({
   handler: async (req, reply) => {
     const response = await myServerAdapter.handleNodeRequest(req, {
       req,
-      reply,
+      reply
     })
     response.headers.forEach((value, key) => {
       reply.header(key, value)

packages/server/src/index.ts

@@ -2,7 +2,8 @@
 
 import type { RequestListener, ServerResponse } from 'node:http';
 import {
-  isReadable,
+  isFetchEvent,
+  isNodeRequest,
   isRequestInit,
   isServerResponse,
   NodeRequest,
@@ -46,21 +47,23 @@ export interface ServerAdapterObject<
   /**
    * This function takes Node's request object and returns a WHATWG Fetch spec compliant `Response` object.
    **/
-  handleNodeRequest(nodeRequest: NodeRequest, ctx: TServerContext): Promise<Response> | Response;
+  handleNodeRequest(nodeRequest: NodeRequest, ...ctx: Partial<TServerContext>[]): Promise<Response> | Response;
   /**
    * A request listener function that can be used with any Node server variation.
    */
   requestListener: RequestListener;
-  /**
-   * Proxy to requestListener to mimic Node middlewares
-   */
-  handle: ServerAdapterObject<TServerContext, TBaseObject>['requestListener'] &
-    ServerAdapterObject<TServerContext, TBaseObject>['fetch'];
+
+  handle(req: NodeRequest, res: ServerResponse, ...ctx: Partial<TServerContext>[]): Promise<void>;
+  handle(request: Request, ...ctx: Partial<TServerContext>[]): Promise<Response> | Response;
+  handle(fetchEvent: FetchEvent & Partial<TServerContext>, ...ctx: Partial<TServerContext>[]): void;
+  handle(
+    container: { request: Request } & Partial<TServerContext>,
+    ...ctx: Partial<TServerContext>[]
+  ): Promise<Response> | Response;
 }
 
 export type ServerAdapter<TServerContext, TBaseObject extends ServerAdapterBaseObject<TServerContext>> = TBaseObject &
-  ServerAdapterObject<TServerContext, TBaseObject>['requestListener'] &
-  ServerAdapterObject<TServerContext, TBaseObject>['fetch'] &
+  ServerAdapterObject<TServerContext, TBaseObject>['handle'] &
   ServerAdapterObject<TServerContext, TBaseObject>;
 
 async function handleWaitUntils(waitUntilPromises: Promise<unknown>[]) {
@@ -111,20 +114,26 @@ function createServerAdapter<
   const handleRequest =
     typeof serverAdapterBaseObject === 'function' ? serverAdapterBaseObject : serverAdapterBaseObject.handle;
 
-  function handleNodeRequest(nodeRequest: NodeRequest, ctx: TServerContext) {
+  function handleNodeRequest(nodeRequest: NodeRequest, ...ctx: Partial<TServerContext>[]) {
+    const serverContext = ctx.length > 1 ? Object.assign({}, ...ctx) : ctx[0];
     const request = normalizeNodeRequest(nodeRequest, RequestCtor);
-    return handleRequest(request, ctx);
+    return handleRequest(request, serverContext);
   }
 
-  async function requestListener(nodeRequest: NodeRequest, serverResponse: ServerResponse) {
+  async function requestListener(
+    nodeRequest: NodeRequest,
+    serverResponse: ServerResponse,
+    ...ctx: Partial<TServerContext>[]
+  ) {
     const waitUntilPromises: Promise<unknown>[] = [];
-    const response = await handleNodeRequest(nodeRequest, {
+    const defaultServerContext = {
       req: nodeRequest,
       res: serverResponse,
-      waitUntil(p) {
+      waitUntil(p: Promise<unknown>) {
         waitUntilPromises.push(p);
       },
-    } as TServerContext & DefaultServerAdapterContext);
+    };
+    const response = await handleNodeRequest(nodeRequest, defaultServerContext as any, ...ctx);
     if (response) {
       await sendNodeResponse(response, serverResponse);
     } else {
@@ -136,35 +145,27 @@ function createServerAdapter<
     if (waitUntilPromises.length > 0) {
       await handleWaitUntils(waitUntilPromises);
     }
-    return response;
   }
 
-  function handleEvent(event: FetchEvent, ...ctx: Partial<TServerContext>[]) {
+  function handleEvent(event: FetchEvent, ...ctx: Partial<TServerContext>[]): void {
     if (!event.respondWith || !event.request) {
       throw new TypeError(`Expected FetchEvent, got ${event}`);
     }
-    let serverContext = {} as TServerContext;
-    if (ctx?.length > 0) {
-      serverContext = Object.assign({}, serverContext, ...ctx);
-    }
+    const serverContext = ctx.length > 0 ? Object.assign({}, event, ...ctx) : event;
     const response$ = handleRequest(event.request, serverContext);
     event.respondWith(response$);
-    return response$;
   }
 
-  function handleRequestWithWaitUntil(request: Request, ctx: TServerContext) {
-    const extendedCtx = ctx as TServerContext & { waitUntil?: (p: Promise<unknown>) => void };
+  function handleRequestWithWaitUntil(request: Request, ...ctx: Partial<TServerContext>[]) {
+    const serverContext: TServerContext = ctx.length > 1 ? Object.assign({}, ...ctx) : ctx[0] || {};
     if ('process' in globalThis && process.versions?.['bun'] != null) {
       // This is required for bun
       request.text();
     }
-    if (!extendedCtx.waitUntil) {
+    if (!('waitUntil' in serverContext)) {
       const waitUntilPromises: Promise<unknown>[] = [];
-      extendedCtx.waitUntil = (p: Promise<unknown>) => {
-        waitUntilPromises.push(p);
-      };
       const response$ = handleRequest(request, {
-        ...extendedCtx,
+        ...serverContext,
         waitUntil(p: Promise<unknown>) {
           waitUntilPromises.push(p);
         },
@@ -174,67 +175,53 @@ function createServerAdapter<
       }
       return response$;
     }
-    return handleRequest(request, extendedCtx);
+    return handleRequest(request, serverContext);
   }
 
   const fetchFn: ServerAdapterObject<TServerContext, TBaseObject>['fetch'] = (
     input,
-    initOrCtx,
-    ...ctx: Partial<TServerContext>[]
+    ...maybeCtx: Partial<TServerContext>[]
   ) => {
-    let init;
-    let serverContext = {} as TServerContext;
-    if (isRequestInit(initOrCtx)) {
-      init = initOrCtx;
-    } else {
-      init = {};
-      serverContext = Object.assign({}, serverContext, initOrCtx);
-    }
-    if (ctx?.length > 0) {
-      serverContext = Object.assign({}, serverContext, ...ctx);
-    }
     if (typeof input === 'string' || input instanceof URL) {
-      return handleRequestWithWaitUntil(new RequestCtor(input, init), serverContext);
+      const [initOrCtx, ...restOfCtx] = maybeCtx;
+      if (isRequestInit(initOrCtx)) {
+        return handleRequestWithWaitUntil(new RequestCtor(input.toString(), initOrCtx), ...restOfCtx);
+      }
+      return handleRequestWithWaitUntil(new RequestCtor(input.toString()), ...maybeCtx);
     }
-    return handleRequestWithWaitUntil(input, serverContext);
+    return handleRequestWithWaitUntil(input, ...maybeCtx);
   };
 
-  const genericRequestHandler: ServerAdapterObject<TServerContext, TBaseObject>['handle'] = (
-    input,
-    initOrCtxOrRes,
-    ...ctx: Partial<TServerContext>[]
-  ) => {
+  const genericRequestHandler = (
+    input: Request | FetchEvent | NodeRequest | ({ request: Request } & Partial<TServerContext>),
+    ...maybeCtx: Partial<TServerContext>[]
+  ): Promise<Response> | Response | Promise<void> | void => {
     // If it is a Node request
-    if (isReadable(input) && isServerResponse(initOrCtxOrRes)) {
-      return requestListener(input, initOrCtxOrRes);
+    const [initOrCtxOrRes, ...restOfCtx] = maybeCtx;
+    if (isNodeRequest(input)) {
+      if (!isServerResponse(initOrCtxOrRes)) {
+        throw new TypeError(`Expected ServerResponse, got ${initOrCtxOrRes}`);
+      }
+      return requestListener(input, initOrCtxOrRes, ...restOfCtx);
     }
+
     if (isServerResponse(initOrCtxOrRes)) {
       throw new Error('Got Node response without Node request');
     }
 
     // Is input a container object over Request?
     if (typeof input === 'object' && 'request' in input) {
       // Is it FetchEvent?
-      if ('respondWith' in input) {
-        return handleEvent(input, isRequestInit(initOrCtxOrRes) ? {} : initOrCtxOrRes, ...ctx);
+      if (isFetchEvent(input)) {
+        return handleEvent(input, ...maybeCtx);
       }
       // In this input is also the context
-      return fetchFn(
-        // @ts-expect-error input can indeed be a Request
-        (input as any as { request: Request }).request,
-        initOrCtxOrRes,
-        ...ctx
-      );
+      return handleRequestWithWaitUntil(input.request, input, ...maybeCtx);
     }
 
     // Or is it Request itself?
     // Then ctx is present and it is the context
-    return fetchFn(
-      // @ts-expect-error input can indeed string | Request | URL
-      input,
-      initOrCtxOrRes,
-      ...ctx
-    );
+    return fetchFn(input, ...maybeCtx);
   };
 
   const adapterObj: ServerAdapterObject<TServerContext, TBaseObject> = {
@@ -243,7 +230,7 @@ function createServerAdapter<
     handleNodeRequest,
     requestListener,
     handleEvent,
-    handle: genericRequestHandler,
+    handle: genericRequestHandler as ServerAdapterObject<TServerContext, TBaseObject>['handle'],
   };
 
   return new Proxy(genericRequestHandler, {
@@ -280,8 +267,8 @@ function createServerAdapter<
         }
       }
     },
-    apply(_, __, [input, ctx]: Parameters<typeof genericRequestHandler>) {
-      return genericRequestHandler(input, ctx);
+    apply(_, __, args: Parameters<ServerAdapterObject<TServerContext, TBaseObject>['handle']>) {
+      return genericRequestHandler(...args);
     },
   }) as any; // 😡
 }

packages/server/src/utils.ts

@@ -6,14 +6,14 @@ function isAsyncIterable(body: any): body is AsyncIterable<any> {
   return body != null && typeof body === 'object' && typeof body[Symbol.asyncIterator] === 'function';
 }
 
-export interface NodeRequest {
+export interface NodeRequest extends Readable {
   protocol?: string;
   hostname?: string;
   body?: any;
   url?: string;
   originalUrl?: string;
   method?: string;
-  headers: any;
+  headers?: any;
   req?: IncomingMessage;
   raw?: IncomingMessage;
   socket?: Socket;
@@ -141,9 +141,19 @@ export function isReadable(stream: any): stream is Readable {
   return stream.read != null;
 }
 
+export function isNodeRequest(request: any): request is NodeRequest {
+  return isReadable(request);
+}
+
 export function isServerResponse(stream: any): stream is ServerResponse {
   // Check all used functions are defined
-  return stream.setHeader != null && stream.end != null && stream.once != null && stream.write != null;
+  return (
+    stream != null && stream.setHeader != null && stream.end != null && stream.once != null && stream.write != null
+  );
+}
+
+export function isFetchEvent(event: any): event is FetchEvent {
+  return event != null && event.request != null && event.respondWith != null;
 }
 
 export async function sendNodeResponse(
@@ -164,6 +174,7 @@ export async function sendNodeResponse(
     } else if (isReadable(body)) {
       serverResponse.once('close', () => {
         body.destroy();
+        resolve();
       });
       body.pipe(serverResponse);
     } else if (isAsyncIterable(body)) {

packages/server/test/fetch-event-listener.spec.ts

@@ -0,0 +1,50 @@
+import { CustomEvent } from '@whatwg-node/events';
+import { Request, Response } from '@whatwg-node/fetch';
+import { createServerAdapter } from '../src';
+
+describe('FetchEvent listener', () => {
+  it('should not return a promise to event listener', () => {
+    const response = new Response();
+    const response$ = Promise.resolve(response);
+    const adapter = createServerAdapter(() => response$);
+    const respondWith = jest.fn();
+    const waitUntil = jest.fn();
+    const fetchEvent = Object.assign(new CustomEvent('fetch'), {
+      request: new Request('http://localhost:8080'),
+      respondWith,
+      waitUntil,
+    });
+    const returnValue = adapter(fetchEvent);
+    expect(returnValue).toBeUndefined();
+    expect(respondWith).toHaveBeenCalledWith(response$);
+  });
+  it('should expose FetchEvent as server context', () => {
+    const handleRequest = jest.fn();
+    const adapter = createServerAdapter(handleRequest);
+    const respondWith = jest.fn();
+    const waitUntil = jest.fn();
+    const fetchEvent = Object.assign(new CustomEvent('fetch'), {
+      request: new Request('http://localhost:8080'),
+      respondWith,
+      waitUntil,
+    });
+    adapter(fetchEvent);
+    expect(handleRequest).toHaveBeenCalledWith(fetchEvent.request, fetchEvent);
+  });
+  it('should accept additional parameters as server context', () => {
+    const handleRequest = jest.fn();
+    const adapter = createServerAdapter<{
+      foo: string;
+    }>(handleRequest);
+    const respondWith = jest.fn();
+    const waitUntil = jest.fn();
+    const fetchEvent = Object.assign(new CustomEvent('fetch'), {
+      request: new Request('http://localhost:8080'),
+      respondWith,
+      waitUntil,
+    });
+    const additionalCtx = { foo: 'bar' };
+    adapter(fetchEvent, additionalCtx);
+    expect(handleRequest).toHaveBeenCalledWith(fetchEvent.request, expect.objectContaining(additionalCtx));
+  });
+});