From 7d16e4ea0fcc0fd0adc8b651778d403ccb94336c Mon Sep 17 00:00:00 2001 From: Victor Berchet Date: Fri, 8 Nov 2024 11:41:29 +0100 Subject: [PATCH 1/4] refactor: make callbacks consistent --- packages/open-next/src/build/createAssets.ts | 40 +++++++------- .../open-next/src/build/createServerBundle.ts | 27 ++++------ packages/open-next/src/build/helper.ts | 53 +++++++++++-------- 3 files changed, 62 insertions(+), 58 deletions(-) diff --git a/packages/open-next/src/build/createAssets.ts b/packages/open-next/src/build/createAssets.ts index e2d380dda..cdd16c20e 100644 --- a/packages/open-next/src/build/createAssets.ts +++ b/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,8 +95,8 @@ 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": @@ -104,12 +104,12 @@ export function createCacheAssets(options: buildHelper.BuildOptions) { case ".body": case ".rsc": const newFilePath = - filepath - .substring(0, filepath.length - ext.length) + absolutePath + .substring(0, absolutePath.length - ext.length) .replace(/\.prefetch$/, "") + ".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 }; } diff --git a/packages/open-next/src/build/createServerBundle.ts b/packages/open-next/src/build/createServerBundle.ts index 3590de236..54c6bb22d 100644 --- a/packages/open-next/src/build/createServerBundle.ts +++ b/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; }, - () => {}, ); } diff --git a/packages/open-next/src/build/helper.ts b/packages/open-next/src/build/helper.ts index 29d57c680..c2f8de4de 100644 --- a/packages/open-next/src/build/helper.ts +++ b/packages/open-next/src/build/helper.ts @@ -167,47 +167,56 @@ export async function esbuildAsync( } } +/** + * 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: (file: string) => boolean, - searchingDir: string = "", + conditionFn: (paths: { + absolutePath: string; + relativePath: string; + }) => boolean, ) { - traverseFiles( - root, - conditionFn, - (filePath) => fs.rmSync(filePath, { force: true }), - searchingDir, + traverseFiles(root, conditionFn, ({ absolutePath }) => + fs.rmSync(absolutePath, { force: true }), ); } /** * 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: { + absolutePath: string; + relativePath: string; + }) => boolean, + callbackFn: (paths: { absolutePath: string; relativePath: string }) => 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 }); } }); } From c63d6c9ad7389f5155b6d1459cddd0c5ca11fb26 Mon Sep 17 00:00:00 2001 From: Victor Berchet Date: Fri, 8 Nov 2024 11:48:30 +0100 Subject: [PATCH 2/4] fixup! format --- packages/open-next/src/build/helper.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/open-next/src/build/helper.ts b/packages/open-next/src/build/helper.ts index c2f8de4de..119f7a022 100644 --- a/packages/open-next/src/build/helper.ts +++ b/packages/open-next/src/build/helper.ts @@ -171,7 +171,7 @@ export async function esbuildAsync( * Recursively delete files. * * @see `traverseFiles`. - *   + * * @param root Root directory to search. * @param conditionFn Predicate used to delete the files. */ From 39aa29778cdea8c456deccc1a8079c5c9d973e1d Mon Sep 17 00:00:00 2001 From: Victor Berchet Date: Fri, 8 Nov 2024 12:04:00 +0100 Subject: [PATCH 3/4] fixup! --- packages/open-next/src/build/createAssets.ts | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/packages/open-next/src/build/createAssets.ts b/packages/open-next/src/build/createAssets.ts index cdd16c20e..baf8774e3 100644 --- a/packages/open-next/src/build/createAssets.ts +++ b/packages/open-next/src/build/createAssets.ts @@ -103,10 +103,10 @@ export function createCacheAssets(options: buildHelper.BuildOptions) { case ".json": case ".body": case ".rsc": - const newFilePath = - absolutePath - .substring(0, absolutePath.length - ext.length) - .replace(/\.prefetch$/, "") + ".cache"; + const newFilePath = absolutePath + .substring(0, absolutePath.length - ext.length) + .replace(/\.prefetch$/, "") + .concat(".cache"); cacheFilesPath[newFilePath] = { [ext.slice(1)]: absolutePath, From c78455e4b58fc21023e680f53f4929eb965bdfb0 Mon Sep 17 00:00:00 2001 From: Victor Berchet Date: Fri, 8 Nov 2024 16:04:00 +0100 Subject: [PATCH 4/4] fixup! add TraversePath --- packages/open-next/src/build/helper.ts | 46 ++++++++++++++------------ 1 file changed, 24 insertions(+), 22 deletions(-) diff --git a/packages/open-next/src/build/helper.ts b/packages/open-next/src/build/helper.ts index 119f7a022..4d1365e55 100644 --- a/packages/open-next/src/build/helper.ts +++ b/packages/open-next/src/build/helper.ts @@ -168,24 +168,12 @@ export async function esbuildAsync( } /** - * Recursively delete files. - * - * @see `traverseFiles`. - * - * @param root Root directory to search. - * @param conditionFn Predicate used to delete the files. + * Type of the parameter of `traverseFiles` callbacks */ -export function removeFiles( - root: string, - conditionFn: (paths: { - absolutePath: string; - relativePath: string; - }) => boolean, -) { - traverseFiles(root, conditionFn, ({ absolutePath }) => - fs.rmSync(absolutePath, { force: true }), - ); -} +export type TraversePath = { + absolutePath: string; + relativePath: string; +}; /** * Recursively traverse files in a directory and call `callbackFn` when `conditionFn` returns true @@ -199,11 +187,8 @@ export function removeFiles( */ export function traverseFiles( root: string, - conditionFn: (paths: { - absolutePath: string; - relativePath: string; - }) => boolean, - callbackFn: (paths: { absolutePath: string; relativePath: string }) => void, + conditionFn: (paths: TraversePath) => boolean, + callbackFn: (paths: TraversePath) => void, searchingDir: string = "", ) { fs.readdirSync(path.join(root, searchingDir)).forEach((file) => { @@ -221,6 +206,23 @@ export function traverseFiles( }); } +/** + * 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 //