refactor: logic for cacheControl and cacheImmutable (#2254)

Author: alexander-akait

README.md

@@ -72,7 +72,7 @@ See [below](#other-servers) for an example of use with fastify.
 |               **[`etag`](#tag)**                |   `boolean\| "weak"\| "strong"`   |                  `undefined`                  | Enable or disable etag generation.                                                                                   |
 |       **[`lastModified`](#lastmodified)**       |             `boolean`             |                  `undefined`                  | Enable or disable `Last-Modified` header. Uses the file system's last modified value.                                |
 |       **[`cacheControl`](#cachecontrol)**       | `boolean\|number\|string\|Object` |                  `undefined`                  | Enable or disable setting `Cache-Control` response header.                                                           |
-|     **[`cacheImmutable`](#cacheimmutable)**     |            `boolean\`             |                  `undefined`                  | Enable or disable setting `Cache-Control: public, max-age=31536000, immutable` response header for immutable assets. |
+|     **[`cacheImmutable`](#cacheimmutable)**     |             `boolean`             |                  `undefined`                  | Enable or disable setting `Cache-Control: public, max-age=31536000, immutable` response header for immutable assets. |
 |         **[`publicPath`](#publicpath)**         |             `string`              |                  `undefined`                  | The public path that the middleware is bound to.                                                                     |
 |              **[`stats`](#stats)**              |     `boolean\|string\|Object`     |        `stats` (from a configuration)         | Stats options object or preset name.                                                                                 |
 |   **[`serverSideRender`](#serversiderender)**   |             `boolean`             |                  `undefined`                  | Instructs the module to enable or disable the server-side rendering mode.                                            |

src/middleware.js

@@ -571,36 +571,36 @@ function wrapper(context) {
       }
 
       if (!getResponseHeader(res, "Cache-Control")) {
-        const hasCacheImmutable =
-          context.options.cacheImmutable === undefined
-            ? true
-            : context.options.cacheImmutable;
-
-        let { cacheControl } = context.options;
-
-        // Normalize cacheControl to object
-        if (typeof cacheControl === "string") {
-          setResponseHeader(res, "Cache-Control", cacheControl);
-        } else if (hasCacheImmutable && extra.immutable) {
-          cacheControl = { immutable: true };
+        const { cacheControl, cacheImmutable } = context.options;
+
+        let cacheControlValue;
+
+        if (
+          (cacheImmutable === undefined || cacheImmutable) &&
+          extra.immutable
+        ) {
+          cacheControlValue = `public, max-age=${Math.floor(MAX_MAX_AGE / 1000)}, immutable`;
         } else if (typeof cacheControl === "boolean") {
-          cacheControl = { maxAge: MAX_MAX_AGE };
+          cacheControlValue = `public, max-age=${Math.floor(MAX_MAX_AGE / 1000)}`;
         } else if (typeof cacheControl === "number") {
-          cacheControl = { maxAge: cacheControl };
-        }
-
-        if (cacheControl && typeof cacheControl === "object") {
+          const maxAge = Math.min(Math.max(0, cacheControl), MAX_MAX_AGE);
+          cacheControlValue = `public, max-age=${Math.floor(maxAge / 1000)}`;
+        } else if (typeof cacheControl === "string") {
+          cacheControlValue = cacheControl;
+        } else if (cacheControl) {
           const maxAge =
             cacheControl.maxAge !== undefined
               ? Math.min(Math.max(0, cacheControl.maxAge), MAX_MAX_AGE)
               : MAX_MAX_AGE;
 
-          let cacheControlValue = `public, max-age=${Math.floor(maxAge / 1000)}`;
+          cacheControlValue = `public, max-age=${Math.floor(maxAge / 1000)}`;
 
-          if (cacheControl.immutable && hasCacheImmutable) {
+          if (cacheControl.immutable) {
             cacheControlValue += ", immutable";
           }
+        }
 
+        if (cacheControlValue) {
           setResponseHeader(res, "Cache-Control", cacheControlValue);
         }
       }

test/middleware.test.js

@@ -6393,7 +6393,7 @@ describe.each([
         });
       });
 
-      describe("should use cacheControl object option (with only immutable: true) when cacheImmutable is false, and not add 'immutable' to Cache-Control header", () => {
+      describe("should use cacheControl object option (with only immutable: true) when cacheImmutable is false, and add 'immutable' to Cache-Control header", () => {
         beforeEach(async () => {
           const compiler = getCompiler({
             ...webpackConfigImmutable,
@@ -6420,7 +6420,7 @@ describe.each([
           expect(response.statusCode).toBe(200);
           expect(response.headers["cache-control"]).toBeDefined();
           expect(response.headers["cache-control"]).toBe(
-            "public, max-age=31536000",
+            "public, max-age=31536000, immutable",
           );
         });
 
@@ -6430,7 +6430,7 @@ describe.each([
           expect(response.statusCode).toBe(200);
           expect(response.headers["cache-control"]).toBeDefined();
           expect(response.headers["cache-control"]).toBe(
-            "public, max-age=31536000",
+            "public, max-age=31536000, immutable",
           );
         });
       });