fix(disable): fixed disable not working properly

DuCanhGH · DuCanhGH · commit e583be60114c · 2024-01-12T15:41:20.000+07:00
[bump]
diff --git a/.changeset/witty-needles-talk.md b/.changeset/witty-needles-talk.md
@@ -0,0 +1,8 @@
+---
+"@ducanh2912/next-pwa": patch
+---
+
+fix(disable): fixed `disable` not working properly
+
+- In 10.2.0 and 10.2.1, when `disable` is set, some code is still unintendedly run, causing the resulting app to contain some erroneous JavaScript.
+- This has been fixed in 10.2.2. Although we still run `parseOptions`, we only do so to get the default options.
diff --git a/examples/reproduction-template/package.json b/examples/reproduction-template/package.json
@@ -9,7 +9,7 @@
     "start": "next start"
   },
   "dependencies": {
-    "@ducanh2912/next-pwa": "workspace:*",
+    "@ducanh2912/next-pwa": "latest",
     "next": "canary",
     "react": "18.2.0",
     "react-dom": "18.2.0"
diff --git a/packages/next-pwa/__tests__/integration/disable/app/favicon.ico b/packages/next-pwa/__tests__/integration/disable/app/favicon.ico
diff --git a/packages/next-pwa/__tests__/integration/disable/app/layout.tsx b/packages/next-pwa/__tests__/integration/disable/app/layout.tsx
@@ -0,0 +1,8 @@
+const RootLayout = ({ children }: { children: React.ReactNode }) => (
+  <html lang="en">
+    <head />
+    <body>{children}</body>
+  </html>
+);
+
+export default RootLayout;
diff --git a/packages/next-pwa/__tests__/integration/disable/app/page.tsx b/packages/next-pwa/__tests__/integration/disable/app/page.tsx
@@ -0,0 +1,10 @@
+import Image from "next/image";
+
+const Page = () => (
+  <main>
+    <p id="welcome-text">This is a Next.js PWA!</p>
+    <Image src="/next.svg" alt="Next.js Logo" width={180} height={37} priority />
+  </main>
+);
+
+export default Page;
diff --git a/packages/next-pwa/__tests__/integration/disable/index.test.ts b/packages/next-pwa/__tests__/integration/disable/index.test.ts
@@ -0,0 +1,17 @@
+import { createDescribe } from "../../test-utils/index.ts";
+
+createDescribe("integration disable", { sourceDir: __dirname, skipInstall: false }, ({ next, testMode }) => {
+  it("should render, have notification, and show no error", async () => {
+    const $ = await next.render("/");
+    expect($("#welcome-text").text()).toBe("This is a Next.js PWA!");
+    expect(next.cliOutput.includes("PWA support is disabled.")).toBe(true);
+    expect(next.cliOutput.includes("Uncaught ReferenceError")).toBe(false);
+  });
+
+  it("should fetch image", async () => {
+    const image = await next.fetch("/next.svg");
+    expect(image.status).toBe(200);
+    const favicon = await next.fetch("/favicon.ico");
+    expect(favicon.status).toBe(200);
+  });
+});
diff --git a/packages/next-pwa/__tests__/integration/disable/next.config.mjs b/packages/next-pwa/__tests__/integration/disable/next.config.mjs
@@ -0,0 +1,12 @@
+// @ts-check
+import withPWAInit from "@ducanh2912/next-pwa";
+
+const withPWA = withPWAInit({
+  dest: "public",
+  disable: true,
+});
+
+/** @type {import('next').NextConfig} */
+const nextConfig = {};
+
+export default withPWA(nextConfig);
diff --git a/packages/next-pwa/__tests__/integration/disable/public/next.svg b/packages/next-pwa/__tests__/integration/disable/public/next.svg
@@ -0,0 +1 @@
+<svg xmlns="http://www.w3.org/2000/svg" fill="none" viewBox="0 0 394 80"><path fill="#000" d="M262 0h68.5v12.7h-27.2v66.6h-13.6V12.7H262V0ZM149 0v12.7H94v20.4h44.3v12.6H94v21h55v12.6H80.5V0h68.7zm34.3 0h-17.8l63.8 79.4h17.9l-32-39.7 32-39.6h-17.9l-23 28.6-23-28.6zm18.3 56.7-9-11-27.1 33.7h17.8l18.3-22.7z"/><path fill="#000" d="M81 79.3 17 0H0v79.3h13.6V17l50.2 62.3H81Zm252.6-.4c-1 0-1.8-.4-2.5-1s-1.1-1.6-1.1-2.6.3-1.8 1-2.5 1.6-1 2.6-1 1.8.3 2.5 1a3.4 3.4 0 0 1 .6 4.3 3.7 3.7 0 0 1-3 1.8zm23.2-33.5h6v23.3c0 2.1-.4 4-1.3 5.5a9.1 9.1 0 0 1-3.8 3.5c-1.6.8-3.5 1.3-5.7 1.3-2 0-3.7-.4-5.3-1s-2.8-1.8-3.7-3.2c-.9-1.3-1.4-3-1.4-5h6c.1.8.3 1.6.7 2.2s1 1.2 1.6 1.5c.7.4 1.5.5 2.4.5 1 0 1.8-.2 2.4-.6a4 4 0 0 0 1.6-1.8c.3-.8.5-1.8.5-3V45.5zm30.9 9.1a4.4 4.4 0 0 0-2-3.3 7.5 7.5 0 0 0-4.3-1.1c-1.3 0-2.4.2-3.3.5-.9.4-1.6 1-2 1.6a3.5 3.5 0 0 0-.3 4c.3.5.7.9 1.3 1.2l1.8 1 2 .5 3.2.8c1.3.3 2.5.7 3.7 1.2a13 13 0 0 1 3.2 1.8 8.1 8.1 0 0 1 3 6.5c0 2-.5 3.7-1.5 5.1a10 10 0 0 1-4.4 3.5c-1.8.8-4.1 1.2-6.8 1.2-2.6 0-4.9-.4-6.8-1.2-2-.8-3.4-2-4.5-3.5a10 10 0 0 1-1.7-5.6h6a5 5 0 0 0 3.5 4.6c1 .4 2.2.6 3.4.6 1.3 0 2.5-.2 3.5-.6 1-.4 1.8-1 2.4-1.7a4 4 0 0 0 .8-2.4c0-.9-.2-1.6-.7-2.2a11 11 0 0 0-2.1-1.4l-3.2-1-3.8-1c-2.8-.7-5-1.7-6.6-3.2a7.2 7.2 0 0 1-2.4-5.7 8 8 0 0 1 1.7-5 10 10 0 0 1 4.3-3.5c2-.8 4-1.2 6.4-1.2 2.3 0 4.4.4 6.2 1.2 1.8.8 3.2 2 4.3 3.4 1 1.4 1.5 3 1.5 5h-5.8z"/></svg>
diff --git a/packages/next-pwa/__tests__/integration/disable/tsconfig.json b/packages/next-pwa/__tests__/integration/disable/tsconfig.json
@@ -0,0 +1,29 @@
+{
+  "compilerOptions": {
+    "allowImportingTsExtensions": true,
+    "target": "es5",
+    "lib": ["dom", "dom.iterable", "esnext"],
+    "allowJs": true,
+    "skipLibCheck": true,
+    "strict": true,
+    "forceConsistentCasingInFileNames": true,
+    "noEmit": true,
+    "esModuleInterop": true,
+    "module": "esnext",
+    "moduleResolution": "node",
+    "resolveJsonModule": true,
+    "isolatedModules": true,
+    "jsx": "preserve",
+    "incremental": true,
+    "plugins": [
+      {
+        "name": "next"
+      }
+    ],
+    "paths": {
+      "@/*": ["./src/*"]
+    }
+  },
+  "include": ["next-env.d.ts", "**/*.ts", "**/*.tsx", ".next/types/**/*.ts"],
+  "exclude": ["node_modules"]
+}
diff --git a/packages/next-pwa/package.json b/packages/next-pwa/package.json
@@ -12,6 +12,9 @@
     "!dist/dts"
   ],
   "source": "./src/index.ts",
+  "imports": {
+    "#utils/*.js": "./src/utils/*.ts"
+  },
   "exports": {
     ".": {
       "require": {
diff --git a/packages/next-pwa/rollup.config.js b/packages/next-pwa/rollup.config.js
@@ -5,6 +5,7 @@
  *  "input" | "output" | "plugins"
  * > & {
  *   external?: (string | RegExp)[] | string | RegExp;
+ *   aliases?: Record<string, string[]>;
  * }} FileEntry
  */
 import alias from "@rollup/plugin-alias";
diff --git a/packages/next-pwa/src/context.ts b/packages/next-pwa/src/context.ts
@@ -6,19 +6,24 @@ import type { NextConfig, NextConfigComplete, WebpackConfigContext } from "next/
 import type { TsConfigJson } from "type-fest";
 import type { Asset, Configuration as WebpackConfig, default as Webpack } from "webpack";
 
-import { loadTSConfig } from "$utils/index.js";
+import { loadTSConfig, logger } from "#utils/index.js";
 
+import { RequireFields, WorkboxOptions } from "./private-types.js";
 import type { PluginOptions } from "./types.js";
 import { getFileHash } from "./utils.js";
 import { getDefaultDocumentPage } from "./webpack/builders/get-default-document-page.js";
 
 const __dirname = fileURLToPath(new URL(".", import.meta.url));
 
-export type PluginOptionsComplete = Required<PluginOptions>;
+export interface PluginOptionsComplete extends Required<PluginOptions> {
+  disable: false;
+  workboxOptions: RequireFields<WorkboxOptions, "swDest" | "additionalManifestEntries" | "exclude" | "modifyURLPrefix" | "manifestTransforms">;
+}
 
 type PublicPath = NonNullable<NonNullable<WebpackConfig["output"]>["publicPath"]>;
 
 export type NextPwaContext = {
+  disabled: false;
   publicPath: PublicPath | undefined;
   nextConfig: NextConfigComplete;
   webpack: typeof Webpack;
@@ -61,7 +66,9 @@ export const parseOptions = (
     } = {},
     extendDefaultRuntimeCaching = false,
   }: PluginOptions,
-): PluginOptionsComplete => {
+): PluginOptionsComplete | { disable: true } => {
+  if (disable) return { disable };
+
   if (!additionalManifestEntries) {
     const publicDir = path.resolve(webpackContext.dir, "public");
     additionalManifestEntries = fg
@@ -183,7 +190,7 @@ export const createContext = (
   userNextConfig: NextConfig,
   webpackConfig: WebpackConfig,
   userOptions: PluginOptions,
-): NextPwaContext => {
+): NextPwaContext | { disabled: true; webpackConfig: WebpackConfig } => {
   // Do NOT use resolvedNextConfig here.
   // Its `webpack` function is the resulting function of calling all plugins,
   // including ours. Calling it will cause an infinitely recursive call.
@@ -195,11 +202,18 @@ export const createContext = (
     basePath: options.config.basePath || "/",
   } satisfies NextConfigComplete;
   const publicPath = webpackConfig.output?.publicPath;
-  const tsConfig = loadTSConfig(options.dir, resolvedNextConfig.typescript.tsconfigPath);
   const resolvedOptions = parseOptions(options, publicPath, resolvedNextConfig, userOptions);
   if (!webpackConfig.plugins) {
     webpackConfig.plugins = [];
   }
+  if (resolvedOptions.disable) {
+    !options.isServer && logger.info("PWA support is disabled.");
+    return {
+      disabled: true,
+      webpackConfig,
+    };
+  }
+  const tsConfig = loadTSConfig(options.dir, resolvedNextConfig.typescript.tsconfigPath);
   webpackConfig.plugins.push(
     new webpack.DefinePlugin({
       __PWA_SW__: `'${resolvedOptions.sw}'`,
@@ -232,6 +246,7 @@ export const createContext = (
     return entries;
   };
   return {
+    disabled: false,
     publicPath,
     nextConfig: resolvedNextConfig,
     webpack,
diff --git a/packages/next-pwa/src/index.ts b/packages/next-pwa/src/index.ts
@@ -4,7 +4,7 @@ import { CleanWebpackPlugin } from "clean-webpack-plugin";
 import type { NextConfig } from "next";
 import type { Configuration } from "webpack";
 
-import { logger } from "$utils/index.js";
+import { logger } from "#utils/index.js";
 
 import defaultCache from "./cache.js";
 import { createContext } from "./context.js";
@@ -18,8 +18,7 @@ const withPWAInit = (pluginOptions: PluginOptions = {}): ((nextConfig?: NextConf
     webpack(config: Configuration, options) {
       const ctx = createContext(options.webpack, options, nextConfig, config, pluginOptions);
 
-      if (ctx.options.disable) {
-        options.isServer && logger.info("PWA support is disabled.");
+      if (ctx.disabled) {
         return ctx.webpackConfig;
       }
 
diff --git a/packages/next-pwa/src/private-types.ts b/packages/next-pwa/src/private-types.ts
@@ -1,7 +1,26 @@
 import type { WebpackInjectManifestOptions, WebpackPartial } from "workbox-build";
 import type { GenerateSWConfig } from "workbox-webpack-plugin";
 
-type Impossible<K extends keyof any> = { [P in K]?: never };
+export type Impossible<K extends keyof any> = { [P in K]?: never };
+
+/**
+ * Make certain fields in a object type required
+ *
+ * @example
+ *     interface A {
+ *         a?: string;
+ *         b?: string;
+ *         c?: string;
+ *     }
+ *     type B = RequireFields<A, "b" | "c">;
+ *     const b: B = {
+ *         b: "hehe",
+ *         c: "hehe",
+ *     }; //valid
+ *     const b: B = { a: "hehe" }; //invalid
+ *     const c: B = { a: "hehe", b: "hehe" }; //invalid
+ */
+export type RequireFields<T, U extends keyof T> = T & Required<Pick<T, U>>;
 
 export type SharedWorkboxOptionsKeys = keyof GenerateSWConfig & keyof WebpackInjectManifestOptions;
 
@@ -58,9 +77,6 @@ export type InjectManifestOptions = WebpackInjectManifestOptions &
   WebpackOptions &
   Impossible<Exclude<keyof GenerateSWConfig, SharedWorkboxOptionsKeys>>;
 
-export type WorkboxTypes = {
-  GenerateSW: GenerateSWOptions;
-  InjectManifest: InjectManifestOptions;
-};
+export type WorkboxOptions = GenerateSWOptions | InjectManifestOptions;
 
 export type StringKeyOf<BaseType> = `${Extract<keyof BaseType, string | number>}`;
diff --git a/packages/next-pwa/src/resolve-runtime-caching.ts b/packages/next-pwa/src/resolve-runtime-caching.ts
@@ -1,6 +1,6 @@
 import type { RuntimeCaching } from "workbox-build";
 
-import { logger } from "$utils/index.js";
+import { logger } from "#utils/index.js";
 
 import defaultCache from "./cache.js";
 
diff --git a/packages/next-pwa/src/resolve-workbox-plugin.ts b/packages/next-pwa/src/resolve-workbox-plugin.ts
@@ -3,7 +3,7 @@ import path from "node:path";
 import type { RuntimeCaching } from "workbox-build";
 import WorkboxPlugin from "workbox-webpack-plugin";
 
-import { logger } from "$utils/index.js";
+import { logger } from "#utils/index.js";
 
 import { NextPwaContext } from "./context.js";
 import { resolveRuntimeCaching } from "./resolve-runtime-caching.js";
diff --git a/packages/next-pwa/src/types.ts b/packages/next-pwa/src/types.ts
@@ -1,4 +1,4 @@
-import type { WorkboxTypes } from "./private-types.js";
+import type { WorkboxOptions } from "./private-types.js";
 
 export interface PluginOptions {
   /**
@@ -157,7 +157,7 @@ export interface PluginOptions {
    * `workbox-webpack-plugin`'s own JSDoc, so some information may not be
    * exactly correct.
    */
-  workboxOptions?: WorkboxTypes[keyof WorkboxTypes];
+  workboxOptions?: WorkboxOptions;
 }
 
 export interface FallbackRoutes {
diff --git a/packages/next-pwa/src/utils.ts b/packages/next-pwa/src/utils.ts
@@ -3,9 +3,9 @@ import fs from "node:fs";
 
 import type { GenerateSW, InjectManifest } from "workbox-webpack-plugin";
 
-import { logger } from "$utils/index.js";
+import { logger } from "#utils/index.js";
 
-import type { WorkboxTypes } from "./private-types.js";
+import type { InjectManifestOptions, WorkboxOptions } from "./private-types.js";
 
 export const overrideAfterCalledMethod = (workboxPlugin: InjectManifest | GenerateSW) => {
   Object.defineProperty(workboxPlugin, "alreadyCalled", {
@@ -18,8 +18,8 @@ export const overrideAfterCalledMethod = (workboxPlugin: InjectManifest | Genera
   });
 };
 
-export const isInjectManifestConfig = (config: WorkboxTypes[keyof WorkboxTypes] | undefined): config is WorkboxTypes["InjectManifest"] => {
-  return typeof config !== "undefined" && typeof (config as any).swSrc === "string";
+export const isInjectManifestConfig = (config: WorkboxOptions | undefined): config is InjectManifestOptions => {
+  return typeof config !== "undefined" && typeof config.swSrc === "string";
 };
 
 /**
diff --git a/packages/next-pwa/src/webpack/builders/build-custom-worker.ts b/packages/next-pwa/src/webpack/builders/build-custom-worker.ts
@@ -2,7 +2,7 @@ import path from "node:path";
 
 import fg from "fast-glob";
 
-import { logger } from "$utils/index.js";
+import { logger } from "#utils/index.js";
 
 import { NextPwaContext } from "../../context.js";
 import { getContentHash } from "../../utils.js";
diff --git a/packages/next-pwa/src/webpack/builders/get-default-document-page.ts b/packages/next-pwa/src/webpack/builders/get-default-document-page.ts
@@ -1,7 +1,7 @@
 import fs from "node:fs";
 import path from "node:path";
 
-import { findFirstTruthy } from "$utils/index.js";
+import { findFirstTruthy } from "#utils/index.js";
 
 export const getDefaultDocumentPage = (baseDir: string, pageExtensions: string[]) => {
   const pagesDir = findFirstTruthy(["pages", "src/pages"], (dir) => {
diff --git a/packages/next-pwa/src/webpack/builders/get-fallback-envs.ts b/packages/next-pwa/src/webpack/builders/get-fallback-envs.ts
@@ -1,6 +1,6 @@
 import path from "node:path";
 
-import { logger } from "$utils/index.js";
+import { logger } from "#utils/index.js";
 
 import type { FallbackRoutes } from "../../types.js";
 
diff --git a/packages/next-pwa/tsconfig.json b/packages/next-pwa/tsconfig.json
@@ -1,10 +1,7 @@
 {
   "extends": "../../tsconfig.json",
   "compilerOptions": {
-    "module": "NodeNext",
-    "paths": {
-      "$utils/*": ["./src/utils/*"]
-    }
+    "module": "NodeNext"
   },
   "include": ["globals.d.ts", "**/*.ts", "**/*.tsx"],
   "exclude": ["node_modules", "dist", "__tests__"]
diff --git a/pnpm-lock.yaml b/pnpm-lock.yaml
