Skip to content

Commit

Permalink
nodejs compat functions build (#3133)
Browse files Browse the repository at this point in the history
  • Loading branch information
dario-piotrowicz committed May 5, 2023
1 parent 5079f47 commit d078800
Show file tree
Hide file tree
Showing 7 changed files with 32 additions and 5 deletions.
5 changes: 5 additions & 0 deletions .changeset/metal-buckets-fly.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
"wrangler": patch
---

fix pages building not taking into account the nodejs_compat flag (and improve the related error message)
6 changes: 6 additions & 0 deletions packages/wrangler/src/__tests__/pages/functions-build.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -448,6 +448,12 @@ export default {
"Build failed with 1 error:
hello.js:2:36: ERROR: Could not resolve \\"node:async_hooks\\""
`);
expect(std.err).toContain(
'The package "node:async_hooks" wasn\'t found on the file system but is built into node.'
);
expect(std.err).toContain(
'Add the "nodejs_compat" compatibility flag to your Pages project to enable Node.js compatibility.'
);
});

it("should compile a _worker.js/ directory", async () => {
Expand Down
2 changes: 1 addition & 1 deletion packages/wrangler/src/__tests__/publish.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7224,7 +7224,7 @@ export default{
expect(
esbuild.formatMessagesSync(err?.errors ?? [], { kind: "error" }).join()
).toMatch(
/The package "path" wasn't found on the file system but is built into node\.\s+Add "node_compat = true" to your wrangler\.toml file to enable Node compatibility\./
/The package "path" wasn't found on the file system but is built into node\.\s+Add "node_compat = true" to your wrangler\.toml file to enable Node.js compatibility\./
);
});

Expand Down
20 changes: 16 additions & 4 deletions packages/wrangler/src/bundle.ts
Original file line number Diff line number Diff line change
Expand Up @@ -77,15 +77,25 @@ export function isBuildFailure(err: unknown): err is esbuild.BuildFailure {
* Rewrites esbuild BuildFailures for failing to resolve Node built-in modules
* to suggest enabling Node compat as opposed to `platform: "node"`.
*/
export function rewriteNodeCompatBuildFailure(err: esbuild.BuildFailure) {
export function rewriteNodeCompatBuildFailure(
err: esbuild.BuildFailure,
forPages = false
) {
for (const error of err.errors) {
const match = nodeBuiltinResolveErrorText.exec(error.text);
if (match !== null) {
const issue = `The package "${match[1]}" wasn't found on the file system but is built into node.`;

const instructionForUser = `${
forPages
? 'Add the "nodejs_compat" compatibility flag to your Pages project'
: 'Add "node_compat = true" to your wrangler.toml file'
} to enable Node.js compatibility.`;

error.notes = [
{
location: null,
text: `The package "${match[1]}" wasn't found on the file system but is built into node.
Add "node_compat = true" to your wrangler.toml file to enable Node compatibility.`,
text: `${issue}\n${instructionForUser}`,
},
];
}
Expand Down Expand Up @@ -148,6 +158,7 @@ export async function bundleWorker(
// TODO: Rip these out https://github.com/cloudflare/workers-sdk/issues/2153
disableModuleCollection?: boolean;
isOutfile?: boolean;
forPages?: boolean;
}
): Promise<BundleResult> {
const {
Expand Down Expand Up @@ -179,6 +190,7 @@ export async function bundleWorker(
plugins,
disableModuleCollection,
isOutfile,
forPages,
} = options;

// We create a temporary directory for any oneoff files we
Expand Down Expand Up @@ -412,7 +424,7 @@ export async function bundleWorker(
result = await esbuild.build(buildOptions);
} catch (e) {
if (!legacyNodeCompat && isBuildFailure(e))
rewriteNodeCompatBuildFailure(e);
rewriteNodeCompatBuildFailure(e, forPages);
throw e;
}

Expand Down
1 change: 1 addition & 0 deletions packages/wrangler/src/pages/build.ts
Original file line number Diff line number Diff line change
Expand Up @@ -233,6 +233,7 @@ export const Handler = async (args: PagesBuildArgs) => {
sourcemap,
watch,
betaD1Shims: d1Databases,
nodejsCompat,
});
}
} else {
Expand Down
1 change: 1 addition & 0 deletions packages/wrangler/src/pages/functions/buildPlugin.ts
Original file line number Diff line number Diff line change
Expand Up @@ -108,6 +108,7 @@ export function buildPlugin({
targetConsumer: local ? "dev" : "publish",
local,
experimentalLocal: false,
forPages: true,
}
);
}
2 changes: 2 additions & 0 deletions packages/wrangler/src/pages/functions/buildWorker.ts
Original file line number Diff line number Diff line change
Expand Up @@ -153,6 +153,7 @@ export function buildWorker({
targetConsumer: local ? "dev" : "publish",
local,
experimentalLocal: false,
forPages: true,
}
);
}
Expand Down Expand Up @@ -227,6 +228,7 @@ export function buildRawWorker({
targetConsumer: local ? "dev" : "publish",
local,
experimentalLocal: false,
forPages: true,
}
);
}
Expand Down

0 comments on commit d078800

Please sign in to comment.