fix(core): fixed invalid precache manifest with assetPrefix

[bump]
DuCanhGH · Jan 31, 2024 · 07c8861 · 07c8861
1 parent 0ac7a0c
commit 07c8861
Show file tree

Hide file tree

Showing 12 changed files with 159 additions and 7 deletions.
diff --git a/.changeset/poor-pumpkins-protect.md b/.changeset/poor-pumpkins-protect.md
@@ -0,0 +1,7 @@
+---
+"@ducanh2912/next-pwa": patch
+---
+
+fix(core): fixed invalid precache manifest with `assetPrefix`
+
+- Turns out it is much more complex than we thought. To make this work with `assetPrefix`, `distDir`, and `basePath`, we now remove `${publicPath}${publicDirRelativeToOutputPath}` from public files in `manifestTransforms` because `assetPrefix` is not intended for files that are in the public directory and we also want to remove `/_next/${publicDirRelativeToOutputPath}` from the URL, since that is not how we resolve files in the public directory.
diff --git a/packages/next-pwa/__tests__/integration/asset-prefix/app/favicon.ico b/packages/next-pwa/__tests__/integration/asset-prefix/app/favicon.ico
diff --git a/packages/next-pwa/__tests__/integration/asset-prefix/app/layout.tsx b/packages/next-pwa/__tests__/integration/asset-prefix/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/asset-prefix/app/page.tsx b/packages/next-pwa/__tests__/integration/asset-prefix/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/asset-prefix/index.test.ts b/packages/next-pwa/__tests__/integration/asset-prefix/index.test.ts
@@ -0,0 +1,28 @@
+import { createDescribe } from "../../test-utils/index.ts";
+
+createDescribe("integration basePath", { sourceDir: __dirname, skipInstall: false }, ({ next, testMode }) => {
+  it("should render", async () => {
+    const $ = await next.render("/next-pwa");
+    expect($("#welcome-text").text()).toBe("This is a Next.js PWA!");
+  });
+
+  it("should fetch image", async () => {
+    const image = await next.fetch("/next-pwa/next.svg");
+    expect(image.status).toBe(200);
+    const favicon = await next.fetch("/next-pwa/favicon.ico");
+    expect(favicon.status).toBe(200);
+  });
+
+  it("should be able to fetch service worker", async () => {
+    const sw = await next.fetch("/next-pwa/sw.js");
+    expect(sw.status).toBe(200);
+    expect(sw.headers.get("Content-Type")?.includes("application/javascript")).toBe(true);
+    const swContent = await sw.text();
+    if (testMode === "start") {
+      expect(/url:\"\/next-pwa\/swe-worker-(.*?).js\"/.test(swContent)).toBe(true);
+      expect(/url:\"\/next-pwa\/_next\/..\/public\/swe-worker-(.*?).js\"/.test(swContent)).toBe(false);
+      expect(/url:\"https:\/\/example.com\/_next\/..\/public\/swe-worker-(.*?).js\"/.test(swContent)).toBe(false);
+      expect(/url:\"https:\/\/example.com\/_next\/static\/chunks\/main-app-(.*?).js\"/.test(swContent)).toBe(true);
+    }
+  });
+});
diff --git a/packages/next-pwa/__tests__/integration/asset-prefix/next.config.mjs b/packages/next-pwa/__tests__/integration/asset-prefix/next.config.mjs
@@ -0,0 +1,15 @@
+// @ts-check
+import withPWAInit from "@ducanh2912/next-pwa";
+
+const withPWA = withPWAInit({
+  dest: "public",
+  cacheOnFrontEndNav: true,
+});
+
+/** @type {import('next').NextConfig} */
+const nextConfig = {
+  assetPrefix: "https://example.com",
+  basePath: "/next-pwa",
+};
+
+export default withPWA(nextConfig);
diff --git a/packages/next-pwa/__tests__/integration/asset-prefix/public/next.svg b/packages/next-pwa/__tests__/integration/asset-prefix/public/next.svg
diff --git a/packages/next-pwa/__tests__/integration/asset-prefix/tsconfig.json b/packages/next-pwa/__tests__/integration/asset-prefix/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/src/context.ts b/packages/next-pwa/src/context.ts
@@ -4,9 +4,9 @@ import { fileURLToPath } from "node:url";
 import fg from "fast-glob";
 import type { NextConfig, NextConfigComplete, WebpackConfigContext } from "next/dist/server/config-shared.js";
 import type { TsConfigJson } from "type-fest";
-import type { Asset, Configuration as WebpackConfig, default as Webpack } from "webpack";
+import type { Asset, Compilation, Configuration as WebpackConfig, default as Webpack } from "webpack";
 
-import { loadTSConfig, logger } from "#utils/index.js";
+import { loadTSConfig, logger, relativeToOutputPath } from "#utils/index.js";
 
 import { RequireFields, WorkboxOptions } from "./private-types.js";
 import type { PluginOptions } from "./types.js";
@@ -68,8 +68,9 @@ export const parseOptions = (
 ): PluginOptionsComplete | { disable: true } => {
   if (disable) return { disable };
 
+  const publicDir = path.resolve(webpackContext.dir, "public");
+
   if (!additionalManifestEntries) {
-    const publicDir = path.resolve(webpackContext.dir, "public");
     additionalManifestEntries = fg
       .sync(
         [
@@ -159,11 +160,21 @@ export const parseOptions = (
       manifestTransforms: [
         ..._manifestTransforms,
         async (manifestEntries, compilation) => {
+          // This path always uses forward slashes, so it is safe to use it in the following string replace.
+          const publicDirRelativeOutput = relativeToOutputPath(compilation as Compilation, publicDir);
+          // `publicPath` is always `${assetPrefix}/_next/` for Next.js apps.
+          const publicFilesPrefix = `${publicPath}${publicDirRelativeOutput}`;
           const manifest = manifestEntries.map((m) => {
-            m.url = m.url
-              .replace("/_next//static/image", "/_next/static/image")
-              .replace("/_next//static/media", "/_next/static/media")
-              .replace("/_next/../public", "");
+            m.url = m.url.replace("/_next//static/image", "/_next/static/image").replace("/_next//static/media", "/_next/static/media");
+
+            // We remove `${publicPath}/${publicDirRelativeOutput}` because `assetPrefix`
+            // is not intended for files that are in the public directory and we also want
+            // to remove `/_next/${publicDirRelativeOutput}` from the URL, since that is not how
+            // we resolve files in the public directory.
+            if (m.url.startsWith(publicFilesPrefix)) {
+              m.url = path.posix.join(nextConfig.basePath, m.url.replace(publicFilesPrefix, ""));
+            }
+
             if (m.revision === null) {
               let key = m.url;
               if (typeof publicPath === "string" && key.startsWith(publicPath)) {

diff --git a/packages/next-pwa/src/utils/index.ts b/packages/next-pwa/src/utils/index.ts
@@ -3,3 +3,5 @@ export { findFirstTruthy } from "./find-first-truthy.js";
 export { getPackageVersion } from "./get-package-version.js";
 export { loadTSConfig } from "./load-tsconfig.js";
 export * as logger from "./logger.js";
+export { normalizePathSep } from "./normalize-path-sep.js";
+export { relativeToOutputPath } from "./relative-to-output-path.js";
diff --git a/packages/next-pwa/src/utils/normalize-path-sep.ts b/packages/next-pwa/src/utils/normalize-path-sep.ts
@@ -0,0 +1,9 @@
+/**
+ * For a given path, this function ensures that there is no backslash
+ * escaping slashes in the path.
+ *
+ * @param path
+ */
+export const normalizePathSep = (path: string): string => {
+  return path.replace(/\\/g, "/");
+};
diff --git a/packages/next-pwa/src/utils/relative-to-output-path.ts b/packages/next-pwa/src/utils/relative-to-output-path.ts
@@ -0,0 +1,32 @@
+/*
+  Copyright 2018 Google LLC
+
+  Use of this source code is governed by an MIT-style
+  license that can be found in the LICENSE file or at
+  https://opensource.org/licenses/MIT.
+*/
+
+import path from "node:path";
+
+import type { Compilation } from "webpack";
+
+import { normalizePathSep } from "./normalize-path-sep.js";
+
+/**
+ * @param compilation The webpack compilation.
+ * @param pathValue The original path value.
+ *
+ * @returns If path was not absolute, the returns path as-is.
+ * Otherwise, returns path relative to the compilation's output path.
+ *
+ * @private
+ */
+export function relativeToOutputPath(compilation: Compilation, pathValue: string): string {
+  // See https://github.com/jantimon/html-webpack-plugin/pull/266/files#diff-168726dbe96b3ce427e7fedce31bb0bcR38
+  if (path.resolve(pathValue) === path.normalize(pathValue)) {
+    return normalizePathSep(path.relative(compilation.options.output.path!, pathValue));
+  }
+
+  // Otherwise, return swDest as-is.
+  return normalizePathSep(pathValue);
+}