meta: some improvements and cleanups and review comments

src/worker.ts

@@ -9,20 +9,24 @@ import {
 } from './util';
 import responses from './responses';
 
-export default {
-  async fetch(
-    request: Request,
-    env: Env,
-    ctx: ExecutionContext
-  ): Promise<Response> {
+interface Worker {
+  async fetch: (r: Request, e: Env, c: ExecutionContext) => Promise<Response>;
+}
+
+const cloudflareWorker: Worker = {
+  async fetch = (request, env, ctx) => {
     const cache = caches.default;
-    if (request.method !== 'GET' && request.method !== 'HEAD') {
+
+    const shouldServeCache = isCacheEnabled(env);
+
+    // Review: What about `OPTIONS` type?
+    if (['GET', 'HEAD'].includes(request.method) === false) {
       // This endpoint is called from the sync script to purge
       //  directories that are commonly updated so we don't need to
       //  wait for the cache to expire
       if (
-        isCacheEnabled(env) &&
-        env.PURGE_API_KEY !== undefined &&
+        shouldServeCache && // <-- simplify this big if condition either by separating this condition to variables or smaller parts that are easier to read
+        env.PURGE_API_KEY !== undefined && // <-- shouldn't this always be defined?  what's the point of verifying this env var? Maybe this check should be inside cachePurgeHandler :)
         request.method === 'POST' &&
         request.url === '/_cf/cache-purge'
       ) {
@@ -32,46 +36,52 @@ export default {
       return responses.METHOD_NOT_ALLOWED;
     }
 
-    if (isCacheEnabled(env)) {
+    if (shouldServeCache) {
       // Caching is enabled, let's see if the request is cached
       const response = await cache.match(request);
-      if (response !== undefined) {
-        console.log('Cache hit');
+
+      if (typeof response !== 'undefined') {
         response.headers.append('x-cache-status', 'hit');
+
         return response;
       }
-
-      console.log('Cache miss');
     }
 
-    const url = new URL(request.url);
-    let bucketPath = mapUrlPathToBucketPath(url, env);
-    if (bucketPath === undefined) {
+    const url = new URL(request.url); // <--- you should catch if URL is malformed. new URL can throw an Exception
+
+    const bucketPath = mapUrlPathToBucketPath(url, env);
+
+    if (typeof bucketPath === 'undefined') {
       // Directory listing is restricted and we're not on
       //  a supported path, block request
       return new Response('Unauthorized', { status: 401 });
     }
-    bucketPath = decodeURIComponent(bucketPath);
 
-    let response: Response;
-    if (isDirectoryPath(bucketPath)) {
+    // Review: Not a good practice to assign a variable to two completely different things
+    const bucketComponents = decodeURIComponent(bucketPath);
+    const isPathADirectory = isDirectoryPath(bucketPath);
+
+    if (isPathADirectory && env.DIRECTORY_LISTING === 'off') {
+      // File not found since we should only be allowing
+      //  file paths if directory listing is off
+      return responses.FILE_NOT_FOUND(request);
+    }
+
+    const response: Response = isPathADirectory ?
       // Directory requested, try listing it
-      if (env.DIRECTORY_LISTING === 'off') {
-        // File not found since we should only be allowing
-        //  file paths if directory listing is off
-        return responses.FILE_NOT_FOUND(request);
-      }
-      response = await directoryHandler(url, request, bucketPath, env);
-    } else {
+      await directoryHandler(url, request, bucketComponents, env) :
       // File requested, try to serve it
-      response = await fileHandler(url, request, bucketPath, env);
-    }
+      await fileHandler(url, request, bucketComponents, env);
 
     // Cache response if cache is enabled
-    if (isCacheEnabled(env) && response.status !== 304) {
+    if (shouldServeCache && response.status !== 304) {
       ctx.waitUntil(cache.put(request, response.clone()));
     }
+
     response.headers.append('x-cache-status', 'miss');
+
     return response;
   },
 };
+
+export default cloudflareWorker;