refactor: make traverseFiles callbacks consistent (opennextjs#623)

Author: vicb

packages/open-next/src/build/createAssets.ts

@@ -74,10 +74,10 @@ export function createCacheAssets(options: buildHelper.BuildOptions) {
   const htmlPages = buildHelper.getHtmlPages(dotNextPath);
   buildHelper.removeFiles(
     outputPath,
-    (file) =>
-      file.endsWith(".js") ||
-      file.endsWith(".js.nft.json") ||
-      (file.endsWith(".html") && htmlPages.has(file)),
+    ({ relativePath }) =>
+      relativePath.endsWith(".js") ||
+      relativePath.endsWith(".js.nft.json") ||
+      (relativePath.endsWith(".html") && htmlPages.has(relativePath)),
   );
 
   // Merge cache files into a single file
@@ -95,21 +95,21 @@ export function createCacheAssets(options: buildHelper.BuildOptions) {
   buildHelper.traverseFiles(
     outputPath,
     () => true,
-    (filepath) => {
-      const ext = path.extname(filepath);
+    ({ absolutePath }) => {
+      const ext = path.extname(absolutePath);
       switch (ext) {
         case ".meta":
         case ".html":
         case ".json":
         case ".body":
         case ".rsc":
-          const newFilePath =
-            filepath
-              .substring(0, filepath.length - ext.length)
-              .replace(/\.prefetch$/, "") + ".cache";
+          const newFilePath = absolutePath
+            .substring(0, absolutePath.length - ext.length)
+            .replace(/\.prefetch$/, "")
+            .concat(".cache");
 
           cacheFilesPath[newFilePath] = {
-            [ext.slice(1)]: filepath,
+            [ext.slice(1)]: absolutePath,
             ...cacheFilesPath[newFilePath],
           };
           break;
@@ -161,9 +161,9 @@ export function createCacheAssets(options: buildHelper.BuildOptions) {
     // Traverse files inside cache to find all meta files and cache tags associated with them
     buildHelper.traverseFiles(
       outputPath,
-      (file) => file.endsWith(".meta"),
-      (filePath) => {
-        const fileContent = fs.readFileSync(filePath, "utf8");
+      ({ absolutePath }) => absolutePath.endsWith(".meta"),
+      ({ absolutePath, relativePath }) => {
+        const fileContent = fs.readFileSync(absolutePath, "utf8");
         const fileData = JSON.parse(fileContent);
         if (fileData.headers?.["x-next-cache-tags"]) {
           fileData.headers["x-next-cache-tags"]
@@ -175,7 +175,7 @@ export function createCacheAssets(options: buildHelper.BuildOptions) {
                 path: {
                   S: path.posix.join(
                     buildId,
-                    path.relative(outputPath, filePath).replace(".meta", ""),
+                    relativePath.replace(".meta", ""),
                   ),
                 },
                 // We don't care about the revalidation time here, we just need to make sure it's there
@@ -199,17 +199,14 @@ export function createCacheAssets(options: buildHelper.BuildOptions) {
       buildHelper.traverseFiles(
         fetchCachePath,
         () => true,
-        (filepath) => {
-          const fileContent = fs.readFileSync(filepath, "utf8");
+        ({ absolutePath, relativePath }) => {
+          const fileContent = fs.readFileSync(absolutePath, "utf8");
           const fileData = JSON.parse(fileContent);
           fileData?.tags?.forEach((tag: string) => {
             metaFiles.push({
               tag: { S: path.posix.join(buildId, tag) },
               path: {
-                S: path.posix.join(
-                  buildId,
-                  path.relative(fetchCachePath, filepath),
-                ),
+                S: path.posix.join(buildId, relativePath),
               },
               revalidatedAt: { N: "1" },
             });
@@ -235,7 +232,10 @@ export function createCacheAssets(options: buildHelper.BuildOptions) {
   }
 
   // We need to remove files later because we need the metafiles for dynamodb tags cache
-  buildHelper.removeFiles(outputPath, (file) => !file.endsWith(".cache"));
+  buildHelper.removeFiles(
+    outputPath,
+    ({ relativePath }) => !relativePath.endsWith(".cache"),
+  );
 
   return { useTagCache };
 }

packages/open-next/src/build/createServerBundle.ts

@@ -65,16 +65,14 @@ export async function createServerBundle(options: buildHelper.BuildOptions) {
     const appPath = path.join(serverPath, "app");
     buildHelper.traverseFiles(
       appPath,
-      (file) => {
-        if (file.endsWith("page.js") || file.endsWith("route.js")) {
-          const route = `app/${file.replace(/\.js$/, "")}`;
-          if (!foundRoutes.has(route)) {
-            remainingRoutes.add(route);
-          }
+      ({ relativePath }) =>
+        relativePath.endsWith("page.js") || relativePath.endsWith("route.js"),
+      ({ relativePath }) => {
+        const route = `app/${relativePath.replace(/\.js$/, "")}`;
+        if (!foundRoutes.has(route)) {
+          remainingRoutes.add(route);
         }
-        return false;
       },
-      () => {},
     );
   }
 
@@ -83,16 +81,13 @@ export async function createServerBundle(options: buildHelper.BuildOptions) {
     const pagePath = path.join(serverPath, "pages");
     buildHelper.traverseFiles(
       pagePath,
-      (file) => {
-        if (file.endsWith(".js")) {
-          const route = `pages/${file.replace(/\.js$/, "")}`;
-          if (!foundRoutes.has(route)) {
-            remainingRoutes.add(route);
-          }
+      ({ relativePath }) => relativePath.endsWith(".js"),
+      ({ relativePath }) => {
+        const route = `pages/${relativePath.replace(/\.js$/, "")}`;
+        if (!foundRoutes.has(route)) {
+          remainingRoutes.add(route);
         }
-        return false;
       },
-      () => {},
     );
   }
 

packages/open-next/src/build/helper.ts

@@ -167,51 +167,62 @@ export async function esbuildAsync(
   }
 }
 
-export function removeFiles(
-  root: string,
-  conditionFn: (file: string) => boolean,
-  searchingDir: string = "",
-) {
-  traverseFiles(
-    root,
-    conditionFn,
-    (filePath) => fs.rmSync(filePath, { force: true }),
-    searchingDir,
-  );
-}
+/**
+ *  Type of the parameter of `traverseFiles` callbacks
+ */
+export type TraversePath = {
+  absolutePath: string;
+  relativePath: string;
+};
 
 /**
  * Recursively traverse files in a directory and call `callbackFn` when `conditionFn` returns true
+ *
+ * The callbacks are passed both the absolute and relative (to root) path to files.
+ *
  * @param root - Root directory to search
- * @param conditionFn - Called to determine if `callbackFn` should be called
- * @param callbackFn - Called when `conditionFn` returns true
+ * @param conditionFn - Called to determine if `callbackFn` should be called.
+ * @param callbackFn - Called when `conditionFn` returns true.
  * @param searchingDir - Directory to search (used for recursion)
  */
 export function traverseFiles(
   root: string,
-  conditionFn: (file: string) => boolean,
-  callbackFn: (filePath: string) => void,
+  conditionFn: (paths: TraversePath) => boolean,
+  callbackFn: (paths: TraversePath) => void,
   searchingDir: string = "",
 ) {
   fs.readdirSync(path.join(root, searchingDir)).forEach((file) => {
-    const filePath = path.join(root, searchingDir, file);
-
-    if (fs.statSync(filePath).isDirectory()) {
-      traverseFiles(
-        root,
-        conditionFn,
-        callbackFn,
-        path.join(searchingDir, file),
-      );
+    const relativePath = path.join(searchingDir, file);
+    const absolutePath = path.join(root, relativePath);
+
+    if (fs.statSync(absolutePath).isDirectory()) {
+      traverseFiles(root, conditionFn, callbackFn, relativePath);
       return;
     }
 
-    if (conditionFn(path.join(searchingDir, file))) {
-      callbackFn(filePath);
+    if (conditionFn({ absolutePath, relativePath })) {
+      callbackFn({ absolutePath, relativePath });
     }
   });
 }
 
+/**
+ * Recursively delete files.
+ *
+ * @see `traverseFiles`.
+ *
+ * @param root Root directory to search.
+ * @param conditionFn Predicate used to delete the files.
+ */
+export function removeFiles(
+  root: string,
+  conditionFn: (paths: TraversePath) => boolean,
+) {
+  traverseFiles(root, conditionFn, ({ absolutePath }) =>
+    fs.rmSync(absolutePath, { force: true }),
+  );
+}
+
 export function getHtmlPages(dotNextPath: string) {
   // Get a list of HTML pages
   //