Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 10 additions & 0 deletions .changeset/fix-copytraced-package-json-suffix.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
---
"@opennextjs/aws": patch
---

`copyTracedFiles`: only register a traced file as a node package when its
basename is exactly `package.json`, not when its filename merely ends with
`package.json` (e.g. `care-package.json`). The previous suffix-only check
caused harmless but noisy `Failed to copy <dir>` errors in the Cloudflare
adapter's workerd-package copy step whenever a project traced a JSON data
file whose name happened to end in `-package.json`.
12 changes: 11 additions & 1 deletion packages/open-next/src/build/copyTracedFiles.ts
Original file line number Diff line number Diff line change
Expand Up @@ -85,6 +85,16 @@ export function isNonLinuxPlatformPackage(srcPath: string): boolean {
return nonLinuxPlatformRegex.test(srcPath);
}

// `path.basename` only splits on the OS-native separator, so we explicitly
// look at both `/` and `\` to handle paths produced under either platform.
export function isPackageJson(modulePath: string): boolean {
const lastSep = Math.max(
modulePath.lastIndexOf("/"),
modulePath.lastIndexOf("\\"),
);
return modulePath.slice(lastSep + 1) === "package.json";
}

Comment on lines +88 to +97

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We should avoid using OS specific constructs as much as possible.
I don't think this is needed here.

function copyPatchFile(outputDir: string) {
const patchFile = path.join(__dirname, "patch", "patchedAsyncStorage.js");
const outputPatchFile = path.join(outputDir, "patchedAsyncStorage.cjs");
Expand Down Expand Up @@ -153,7 +163,7 @@ export async function copyTracedFiles({
filesToCopy.set(src, dst);

const module = path.join(dotNextDir, subDir, tracedPath);
if (module.endsWith("package.json")) {
if (isPackageJson(module)) {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Would that work instead?

Suggested change
if (isPackageJson(module)) {
if (path.basename(module) === "package.json") {

nodePackages.set(path.dirname(module), path.dirname(dst));
}
});
Expand Down
25 changes: 25 additions & 0 deletions packages/tests-unit/tests/build/copyTracedFiles.test.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
import {
isExcluded,
isNonLinuxPlatformPackage,
isPackageJson,
} from "@opennextjs/aws/build/copyTracedFiles.js";

describe("isExcluded", () => {
Expand Down Expand Up @@ -129,3 +130,27 @@ describe("isNonLinuxPlatformPackage", () => {
).toBe(false);
});
});

describe("isPackageJson", () => {
test("should detect package.json", () => {
expect(isPackageJson("/project/node_modules/next/package.json")).toBe(true);
expect(isPackageJson("package.json")).toBe(true);
});

test("should not treat files whose names end with 'package.json' as package.json", () => {
// Regression: traced JSON data files in user code can have names like
// `care-package.json` that share the "package.json" suffix without being
// real package manifests. The previous `endsWith("package.json")` check
// mis-classified them, causing `Failed to copy <dir>` errors in the
// Cloudflare adapter's workerd-package copy step.
expect(isPackageJson("/project/data/tables/care-package.json")).toBe(false);
expect(isPackageJson("/project/fixtures/my-package.json")).toBe(false);
});

test("should handle Windows-style path separators", () => {
expect(isPackageJson("C:\\project\\node_modules\\next\\package.json")).toBe(
true,
);
expect(isPackageJson("C:\\project\\data\\care-package.json")).toBe(false);
});
});