Skip to content

Commit

Permalink
fix: also handle the case with no_nodejs_compat_v2
Browse files Browse the repository at this point in the history
  • Loading branch information
petebacondarwin committed Sep 25, 2024
1 parent 6f95443 commit f5dd540
Show file tree
Hide file tree
Showing 3 changed files with 33 additions and 2 deletions.
7 changes: 6 additions & 1 deletion packages/miniflare/src/plugins/core/node-compat.ts
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,7 @@ export function getNodeCompatMode(
hasNodejsAlsFlag,
hasNodejsCompatFlag,
hasNodejsCompatV2Flag,
hasNoNodejsCompatV2Flag,
hasExperimentalNodejsCompatV2Flag,
} = parseNodeCompatibilityFlags(compatibilityFlags);

Expand All @@ -41,7 +42,9 @@ export function getNodeCompatMode(
let mode: NodeJSCompatMode = null;
if (
hasNodejsCompatV2Flag ||
(hasNodejsCompatFlag && compatibilityDate >= nodeCompatSwitchOverDate)
(hasNodejsCompatFlag &&
compatibilityDate >= nodeCompatSwitchOverDate &&
!hasNoNodejsCompatV2Flag)
) {
mode = "v2";
} else if (hasNodejsCompatFlag) {
Expand All @@ -57,6 +60,7 @@ export function getNodeCompatMode(
hasNodejsAlsFlag,
hasNodejsCompatFlag,
hasNodejsCompatV2Flag,
hasNoNodejsCompatV2Flag,
hasExperimentalNodejsCompatV2Flag,
};
}
Expand All @@ -66,6 +70,7 @@ function parseNodeCompatibilityFlags(compatibilityFlags: string[]) {
hasNodejsAlsFlag: compatibilityFlags.includes("nodejs_als"),
hasNodejsCompatFlag: compatibilityFlags.includes("nodejs_compat"),
hasNodejsCompatV2Flag: compatibilityFlags.includes("nodejs_compat_v2"),
hasNoNodejsCompatV2Flag: compatibilityFlags.includes("no_nodejs_compat_v2"),
hasExperimentalNodejsCompatV2Flag: compatibilityFlags.includes(
"experimental:nodejs_compat_v2"
),
Expand Down
26 changes: 26 additions & 0 deletions packages/wrangler/src/__tests__/deploy.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -9355,6 +9355,32 @@ export default{
`);
});

it("should recommend updating the compatibility date flag when using no_nodejs_compat and non-prefixed node builtins", async () => {
writeWranglerToml({
compatibility_date: "2024-09-23",
compatibility_flags: ["nodejs_compat", "no_nodejs_compat_v2"],
});
fs.writeFileSync("index.js", "import fs from 'path';");

await expect(
runWrangler("deploy index.js --dry-run").catch((e) =>
esbuild
.formatMessagesSync(e?.errors ?? [], { kind: "error" })
.join()
.trim()
)
).resolves.toMatchInlineSnapshot(`
"✘ [ERROR] Could not resolve \\"path\\"
index.js:1:15:
1 │ import fs from 'path';
╵ ~~~~~~
The package \\"path\\" wasn't found on the file system but is built into node.
- Make sure to prefix the module name with \\"node:\\" or update your compatibility_date to 2024-09-23 or later."
`);
});

it("should polyfill node builtins when enabled", async () => {
writeWranglerToml();
fs.writeFileSync(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ export function logBuildOutput(
build.onEnd(async ({ errors, warnings }) => {
if (errors.length > 0) {
if (nodejsCompatMode !== "legacy") {
rewriteNodeCompatBuildFailure(errors);
rewriteNodeCompatBuildFailure(errors, nodejsCompatMode);
}
logBuildFailure(errors, warnings);
return;
Expand Down

0 comments on commit f5dd540

Please sign in to comment.