-
Notifications
You must be signed in to change notification settings - Fork 176
add new feature in config for copying files to a specified server function after the build is done #807
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
add new feature in config for copying files to a specified server function after the build is done #807
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -5,7 +5,9 @@ import url from "node:url"; | |
|
||
import type { BuildOptions as ESBuildOptions } from "esbuild"; | ||
import { build as buildAsync, buildSync } from "esbuild"; | ||
import { globSync } from "glob"; | ||
import type { | ||
CopyFile, | ||
DefaultOverrideOptions, | ||
OpenNextConfig, | ||
} from "types/open-next.js"; | ||
|
@@ -440,3 +442,57 @@ export async function isEdgeRuntime( | |
export function getPackagePath(options: BuildOptions) { | ||
return path.relative(options.monorepoRoot, options.appBuildOutputPath); | ||
} | ||
|
||
/** | ||
* Copy files that are specified in the `copyFiles` property into the server functions output directory. | ||
* | ||
* @param copyFiles - Array of files to copy. Each file should have a `srcPath` and `dstPath` property. | ||
* @param outputPath - Path to the output directory. | ||
*/ | ||
export function copyCustomFiles(copyFiles: CopyFile[], outputPath: string) { | ||
copyFiles.forEach(({ srcPath, dstPath }) => { | ||
// Find all files matching the pattern | ||
const matchedFiles = globSync(srcPath, { | ||
nodir: true, | ||
windowsPathsNoEscape: true, | ||
}); | ||
|
||
if (matchedFiles.length === 0) { | ||
logger.warn(`No files found for pattern: ${srcPath}`); | ||
return; | ||
} | ||
|
||
if (matchedFiles.length === 1) { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. If I want to copy There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
yeah your correct about that one. after some more thought i should probably think about a better way to handle this. im open for suggestions. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. One way to handle that could be to have exclusive There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Also I am not sure what use case you are solving with this PR. Maybe having a hook accepting a function taking the buildOptions as a parameter could be good enough - that would mean the options are part of the API and must be documented and ~stable. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
thats a good idea. probably something I would go for if we choose to proceed with this PR.
the idea was that sometimes you need or want a file that's not there. we've had a few cases where people were missing a file in the server function's output. i.e this discord thread however, if you guys think this wont be needed i can close it. Next does have
that is an excellent idea. im gonna think about this until tomorrow. with more thoroughly thought this PR might not be needed for any use-cases. |
||
// Single file match - use dstPath as it is | ||
const srcFile = matchedFiles[0]; | ||
const fullDstPath = path.join(outputPath, dstPath); | ||
|
||
copyFile(srcFile, fullDstPath); | ||
} else { | ||
// Multiple files matched, dstPath will become a directory | ||
matchedFiles.forEach((srcFile) => { | ||
const filename = path.basename(srcFile); | ||
const fullDstPath = path.join(outputPath, dstPath, filename); | ||
copyFile(srcFile, fullDstPath); | ||
}); | ||
} | ||
}); | ||
} | ||
/** | ||
* Copy a file to the destination path. | ||
* | ||
* @param srcFile - Path to the source file. | ||
* @param fullDstPath - Path to the destination file. | ||
*/ | ||
function copyFile(srcFile: string, fullDstPath: string) { | ||
const dstDir = path.dirname(fullDstPath); | ||
|
||
if (!fs.existsSync(dstDir)) { | ||
fs.mkdirSync(dstDir, { recursive: true }); | ||
} | ||
if (fs.existsSync(fullDstPath)) { | ||
logger.warn(`File already exists: ${fullDstPath}. It will be overwritten.`); | ||
} | ||
fs.copyFileSync(srcFile, fullDstPath); | ||
logger.debug(`Copied ${srcFile} to ${fullDstPath}`); | ||
} |
Original file line number | Diff line number | Diff line change | ||||
---|---|---|---|---|---|---|
@@ -1,4 +1,13 @@ | ||||||
import { compareSemver } from "@opennextjs/aws/build/helper.js"; | ||||||
import fs from "node:fs"; | ||||||
import path from "node:path"; | ||||||
|
||||||
import { | ||||||
compareSemver, | ||||||
copyCustomFiles, | ||||||
} from "@opennextjs/aws/build/helper.js"; | ||||||
import logger from "@opennextjs/aws/logger.js"; | ||||||
import mockFs from "mock-fs"; | ||||||
import { vi } from "vitest"; | ||||||
|
||||||
// We don't need to test canary versions, they are stripped out | ||||||
describe("compareSemver", () => { | ||||||
|
@@ -65,3 +74,175 @@ describe("compareSemver", () => { | |||||
expect(() => compareSemver("14.0.0", "!=" as any, "14.0.0")).toThrow(); | ||||||
}); | ||||||
}); | ||||||
|
||||||
const outputFolder = ".open-next/server-functions/default"; | ||||||
|
||||||
describe("copyFiles", () => { | ||||||
beforeEach(() => { | ||||||
mockFs({ | ||||||
"this/is/a/fake/dir": { | ||||||
"some-file24214.txt": "some content", | ||||||
"another-fil321313e.txt": "another content", | ||||||
"empty-file321441.txt": "", | ||||||
"important-js": { | ||||||
"another-important.js": "console.log('important!')", | ||||||
}, | ||||||
}, | ||||||
"this/is/a/real/dir": { | ||||||
"randomfile.txt": "some content", | ||||||
"another-dirfdsf": { | ||||||
"another-filedsfdsf.txt": "another content", | ||||||
}, | ||||||
"empty-file.txtfdsf": "", | ||||||
"imporant-files": { | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
|
||||||
"important.js": "console.log('important!')", | ||||||
"super-important.js": "console.log('super important!')", | ||||||
}, | ||||||
}, | ||||||
[`${outputFolder}/server`]: { | ||||||
"index.mjs": "globalThis.process.env = {}", | ||||||
}, | ||||||
}); | ||||||
|
||||||
vi.spyOn(fs, "copyFileSync"); | ||||||
vi.spyOn(fs, "mkdirSync"); | ||||||
vi.spyOn(fs, "readFileSync"); | ||||||
}); | ||||||
|
||||||
afterAll(() => { | ||||||
mockFs.restore(); | ||||||
vi.restoreAllMocks(); | ||||||
}); | ||||||
|
||||||
it("should work with a glob, dstPath should become a directory", () => { | ||||||
copyCustomFiles( | ||||||
[ | ||||||
{ | ||||||
srcPath: "**/*.js", | ||||||
dstPath: "functions", | ||||||
}, | ||||||
], | ||||||
outputFolder, | ||||||
); | ||||||
|
||||||
const dstDir = path.join(outputFolder, "functions"); | ||||||
expect(fs.copyFileSync).toHaveBeenCalledTimes(3); | ||||||
expect(fs.mkdirSync).toHaveBeenCalledWith(dstDir, { recursive: true }); | ||||||
expect(fs.mkdirSync).toHaveBeenCalledTimes(1); | ||||||
|
||||||
expect(fs.readdirSync(dstDir)).toEqual([ | ||||||
"another-important.js", | ||||||
"important.js", | ||||||
"super-important.js", | ||||||
]); | ||||||
|
||||||
expect( | ||||||
fs.readFileSync(path.join(dstDir, "important.js")).toString(), | ||||||
).toMatchInlineSnapshot(`"console.log('important!')"`); | ||||||
}); | ||||||
|
||||||
it("should copy a single file when srcPath matches one file", () => { | ||||||
copyCustomFiles( | ||||||
[ | ||||||
{ | ||||||
srcPath: "this/is/a/real/dir/randomfile.txt", | ||||||
dstPath: "randomfolder/randomfile.txt", | ||||||
}, | ||||||
], | ||||||
outputFolder, | ||||||
); | ||||||
|
||||||
const dstDir = path.join(outputFolder, "randomfolder"); | ||||||
expect(fs.mkdirSync).toHaveBeenCalledWith(dstDir, { recursive: true }); | ||||||
expect(fs.mkdirSync).toHaveBeenCalledTimes(1); | ||||||
|
||||||
expect(fs.copyFileSync).toHaveBeenCalledTimes(1); | ||||||
expect(fs.copyFileSync).toHaveBeenCalledWith( | ||||||
"this/is/a/real/dir/randomfile.txt", | ||||||
path.join(outputFolder, "randomfolder/randomfile.txt"), | ||||||
); | ||||||
|
||||||
expect( | ||||||
fs.readFileSync(path.join(outputFolder, "randomfolder/randomfile.txt"), { | ||||||
encoding: "utf-8", | ||||||
}), | ||||||
).toMatchInlineSnapshot(`"some content"`); | ||||||
}); | ||||||
|
||||||
it("should work with a glob in a sub directory", () => { | ||||||
copyCustomFiles( | ||||||
[ | ||||||
{ | ||||||
srcPath: "this/is/a/real/dir/imporant-files/**/*.js", | ||||||
dstPath: "super/functions", | ||||||
}, | ||||||
], | ||||||
outputFolder, | ||||||
); | ||||||
|
||||||
expect(fs.mkdirSync).toHaveBeenCalledWith( | ||||||
path.join(outputFolder, "super/functions"), | ||||||
{ recursive: true }, | ||||||
); | ||||||
expect(fs.mkdirSync).toHaveBeenCalledTimes(1); | ||||||
|
||||||
expect(fs.copyFileSync).toHaveBeenCalledTimes(2); | ||||||
expect(fs.copyFileSync).toHaveBeenCalledWith( | ||||||
"this/is/a/real/dir/imporant-files/important.js", | ||||||
path.join(outputFolder, "super/functions/important.js"), | ||||||
); | ||||||
expect(fs.copyFileSync).toHaveBeenCalledWith( | ||||||
"this/is/a/real/dir/imporant-files/super-important.js", | ||||||
path.join(outputFolder, "super/functions/super-important.js"), | ||||||
); | ||||||
|
||||||
expect(fs.readdirSync(path.join(outputFolder, "super/functions"))).toEqual([ | ||||||
"important.js", | ||||||
"super-important.js", | ||||||
]); | ||||||
expect( | ||||||
fs.readFileSync( | ||||||
path.join(outputFolder, "super/functions/super-important.js"), | ||||||
{ encoding: "utf-8" }, | ||||||
), | ||||||
).toMatchInlineSnapshot(`"console.log('super important!')"`); | ||||||
}); | ||||||
it("should warn when file already exists", () => { | ||||||
const logSpy = vi.spyOn(logger, "warn"); | ||||||
|
||||||
copyCustomFiles( | ||||||
[ | ||||||
{ | ||||||
srcPath: "this/is/a/fake/dir/some-file24214.txt", | ||||||
dstPath: "server/index.mjs", | ||||||
}, | ||||||
], | ||||||
outputFolder, | ||||||
); | ||||||
|
||||||
const fullDstPath = path.join(outputFolder, "server/index.mjs"); | ||||||
expect(logSpy).toHaveBeenCalledWith( | ||||||
`File already exists: ${fullDstPath}. It will be overwritten.`, | ||||||
); | ||||||
logSpy.mockRestore(); | ||||||
}); | ||||||
it("should warn when no files are found", () => { | ||||||
const logSpy = vi.spyOn(logger, "warn"); | ||||||
const srcPath = "path/to/dir/does-not-exist.txt"; | ||||||
|
||||||
copyCustomFiles( | ||||||
[ | ||||||
{ | ||||||
srcPath: srcPath, | ||||||
dstPath: "server/index.mjs", | ||||||
}, | ||||||
], | ||||||
outputFolder, | ||||||
); | ||||||
|
||||||
expect(logSpy).toHaveBeenCalledWith( | ||||||
`No files found for pattern: ${srcPath}`, | ||||||
); | ||||||
logSpy.mockRestore(); | ||||||
}); | ||||||
}); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
would that work with "relative" files?
and relative to what?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
yeah, and by default its relative to
process.cwd()
. perhaps i should change that in-case you are in a monorepo.