Skip to content
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

fix: support non-default build conditions via the WRANGLER_BUILD_CONDITIONS flag #6743

Merged
merged 5 commits into from
Sep 19, 2024
Merged
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
9 changes: 9 additions & 0 deletions .changeset/eight-bikes-nail.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
---
"wrangler": patch
---

fix: ability to build tricky Node.js compat scenario Workers

Adds support for non-default build conditions and platform via the WRANGLER_BUILD_CONDITIONS and WRANGLER_BUILD_PLATFORM flags.

Fixes https://github.com/cloudflare/workers-sdk/issues/6742
4 changes: 3 additions & 1 deletion fixtures/isomorphic-random-example/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -3,8 +3,10 @@
"private": true,
"description": "Isomorphic secure-random library, demonstrating `exports` use with Workers",
"exports": {
"other": "./src/other.js",
"node": "./src/node.js",
"workerd": "./src/workerd.mjs"
"workerd": "./src/workerd.mjs",
"default": "./src/default.js"
},
"volta": {
"extends": "../../package.json"
Expand Down
4 changes: 4 additions & 0 deletions fixtures/isomorphic-random-example/src/default.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
// This entry point should only be used if no other build condition match.
export function randomBytes(length) {
return new Uint8Array([8, 9, 10, 11, 12, 13]);
}
4 changes: 4 additions & 0 deletions fixtures/isomorphic-random-example/src/other.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
// This entry point should only be used if the build condition contains `other`.
export function randomBytes(length) {
return new Uint8Array([1, 2, 3, 4, 5, 6]);
}
13 changes: 3 additions & 10 deletions fixtures/worker-app/src/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ import { logErrors } from "./log";

console.log("startup log");

console.log("The following error is a fake for testing");
console.error(
"*** Received structured exception #0xc0000005: access violation; stack: 7ffe71872f57 7ff7834b643b 7ff7834b643b"
);
Expand All @@ -21,6 +22,8 @@ export default {
console.log("request log");

const { pathname, origin, hostname, host } = new URL(request.url);
if (pathname.startsWith("/fav"))
return new Response("Not found", { status: 404 });
if (pathname === "/version_metadata") return Response.json(env.METADATA);
if (pathname === "/random") return new Response(hexEncode(randomBytes(8)));
if (pathname === "/error") throw new Error("Oops!");
Expand Down Expand Up @@ -92,13 +95,3 @@ export default {
ctx.waitUntil(Promise.resolve(event.cron));
},
};

// addEventListener("fetch", (event) => {
// event.respondWith(handleRequest(event.request));
// });

// async function handleRequest(request) {
// return new Response("Hello worker!", {
// headers: { "content-type": "text/plain" },
// });
// }
88 changes: 88 additions & 0 deletions fixtures/worker-app/tests/env-vars.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,88 @@
import { execFileSync } from "child_process";
import { mkdtempSync, readFileSync } from "fs";
import { tmpdir } from "os";
import { join, resolve } from "path";
import { beforeEach, describe, it } from "vitest";
import { wranglerEntryPath } from "../../shared/src/run-wrangler-long-lived";

describe("'wrangler dev' with WRANGLER_BUILD_CONDITIONS", () => {
let tempDir: string;

beforeEach(() => {
tempDir = mkdtempSync(join(tmpdir(), "c3-wrangler-init--from-dash-"));
});

it("should import from the `other` package export if that is in the conditions", async ({
expect,
}) => {
execFileSync(
"node",
[wranglerEntryPath, "deploy", "--dry-run", `--outdir=${tempDir}`],
{
env: {
...process.env,
WRANGLER_BUILD_CONDITIONS: "other,node,browser",
},
}
);
expect(readFileSync(resolve(tempDir, "index.js"), "utf8")).toContain(
"isomorphic-random-example/src/other.js"
);
});

it("should import from the `default` package export if the conditions are explicitly empty", async ({
expect,
}) => {
execFileSync(
"node",
[wranglerEntryPath, "deploy", "--dry-run", `--outdir=${tempDir}`],
{
env: {
...process.env,
WRANGLER_BUILD_CONDITIONS: "",
},
}
);
expect(readFileSync(resolve(tempDir, "index.js"), "utf8")).toContain(
"isomorphic-random-example/src/default.js"
);
});
});

describe("'wrangler build' with WRANGLER_BUILD_PLATFORM", () => {
it("should import from node imports if platform is set to 'node'", ({
expect,
}) => {
execFileSync(
"node",
[wranglerEntryPath, "deploy", "--dry-run", "--outdir=dist/node"],
{
env: {
...process.env,
WRANGLER_BUILD_PLATFORM: "node",
},
}
);
expect(
readFileSync(resolve(__dirname, "../dist/node/index.js"), "utf8")
).toContain("isomorphic-random-example/src/node.js");
});

it("should import from node imports if platform is set to 'browser'", ({
expect,
}) => {
execFileSync(
"node",
[wranglerEntryPath, "deploy", "--dry-run", "--outdir=dist/browser"],
{
env: {
...process.env,
WRANGLER_BUILD_PLATFORM: "browser",
},
}
);
expect(
readFileSync(resolve(__dirname, "../dist/browser/index.js"), "utf8")
).toContain("../isomorphic-random-example/src/workerd.mjs");
});
});
46 changes: 43 additions & 3 deletions packages/wrangler/src/deployment-bundle/bundle.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,10 @@ import * as path from "node:path";
import NodeGlobalsPolyfills from "@esbuild-plugins/node-globals-polyfill";
import NodeModulesPolyfills from "@esbuild-plugins/node-modules-polyfill";
import * as esbuild from "esbuild";
import {
getBuildConditionsFromEnv,
getBuildPlatformFromEnv,
} from "../environment-variables/misc-variables";
import { UserError } from "../errors";
import { getBasePath, getWranglerTmpDir } from "../paths";
import { applyMiddlewareLoaderFacade } from "./apply-middleware";
Expand Down Expand Up @@ -43,8 +47,43 @@ export const COMMON_ESBUILD_OPTIONS = {
loader: { ".js": "jsx", ".mjs": "jsx", ".cjs": "jsx" },
} as const;

// build conditions used by esbuild, and when resolving custom `import` calls
export const BUILD_CONDITIONS = ["workerd", "worker", "browser"];
/**
* Get the custom build conditions used by esbuild, and when resolving custom `import` calls.
*
* If we do not override these in an env var, we will set them to "workerd", "worker" and "browser".
* If we override in env vars then these will be provided to esbuild instead.
*
* Whether or not we set custom conditions the `default` condition will always be active.
* If the Worker is using ESM syntax, then the `import` condition will also be active.
*
* Moreover the following applies:
* - if the platform is set to `browser` (the default) then the `browser` condition will be active.
* - if the platform is set to `node` then the `node` condition will be active.
*
* See https://esbuild.github.io/api/#how-conditions-work for more info.
*/
export function getBuildConditions() {
const envVar = getBuildConditionsFromEnv();
if (envVar !== undefined) {
return envVar.split(",");
CarmenPopoviciu marked this conversation as resolved.
Show resolved Hide resolved
} else {
return ["workerd", "worker", "browser"];
}
}

function getBuildPlatform(): esbuild.Platform {
const platform = getBuildPlatformFromEnv();
if (
platform !== undefined &&
!["browser", "node", "neutral"].includes(platform)
) {
throw new UserError(
"Invalid esbuild platform configuration defined in the WRANGLER_BUILD_PLATFORM environment variable.\n" +
"Valid platform values are: 'browser', 'node' and 'neutral'."
);
}
return platform as esbuild.Platform;
}

/**
* Information about Wrangler's bundling process that needs passed through
Expand Down Expand Up @@ -367,7 +406,8 @@ export async function bundleWorker(
sourceRoot: destination,
minify,
metafile: true,
conditions: BUILD_CONDITIONS,
conditions: getBuildConditions(),
platform: getBuildPlatform(),
petebacondarwin marked this conversation as resolved.
Show resolved Hide resolved
...(process.env.NODE_ENV && {
define: {
...(defineNavigatorUserAgent
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ import { sync as resolveSync } from "resolve";
import { exports as resolveExports } from "resolve.exports";
import { UserError } from "../errors";
import { logger } from "../logger";
import { BUILD_CONDITIONS } from "./bundle";
import { getBuildConditions } from "./bundle";
import {
findAdditionalModules,
findAdditionalModuleWatchDirs,
Expand Down Expand Up @@ -309,7 +309,7 @@ export function createModuleCollector(props: {
packageJson,
args.path.replace(`${packageName}/`, ""),
{
conditions: BUILD_CONDITIONS,
conditions: getBuildConditions(),
}
);
if (testResolved) {
Expand Down
8 changes: 5 additions & 3 deletions packages/wrangler/src/environment-variables/factory.ts
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,9 @@ type VariableNames =
| "WRANGLER_TOKEN_URL"
| "WRANGLER_OUTPUT_FILE_DIRECTORY"
| "WRANGLER_OUTPUT_FILE_PATH"
| "WRANGLER_CI_MATCH_TAG";
| "WRANGLER_CI_MATCH_TAG"
| "WRANGLER_BUILD_CONDITIONS"
| "WRANGLER_BUILD_PLATFORM";

type DeprecatedNames =
| "CF_ACCOUNT_ID"
Expand Down Expand Up @@ -76,9 +78,9 @@ export function getEnvironmentVariableFactory({
}): () => string | undefined {
let hasWarned = false;
return () => {
if (process.env[variableName]) {
if (variableName in process.env) {
return process.env[variableName];
} else if (deprecatedName && process.env[deprecatedName]) {
} else if (deprecatedName && deprecatedName in process.env) {
if (!hasWarned) {
// Only show the warning once.
hasWarned = true;
Expand Down
25 changes: 25 additions & 0 deletions packages/wrangler/src/environment-variables/misc-variables.ts
Original file line number Diff line number Diff line change
Expand Up @@ -98,3 +98,28 @@ export const getOutputFilePathFromEnv = getEnvironmentVariableFactory({
export const getCIMatchTag = getEnvironmentVariableFactory({
variableName: "WRANGLER_CI_MATCH_TAG",
});

/**
* `WRANGLER_BUILD_CONDITIONS` specifies the "build conditions" to use when importing packages at build time.
petebacondarwin marked this conversation as resolved.
Show resolved Hide resolved
*
* See https://nodejs.org/api/packages.html#conditional-exports
* and https://esbuild.github.io/api/#how-conditions-work.
*
* If this is set, Wrangler will configure esbuild to use this list of conditions.
* The format is a string of comma separated conditions.
*/
export const getBuildConditionsFromEnv = getEnvironmentVariableFactory({
variableName: "WRANGLER_BUILD_CONDITIONS",
});

/**
* `WRANGLER_BUILD_PLATFORM` specifies the "build platform" to use when importing packages at build time.
*
* See https://esbuild.github.io/api/#platform
* and https://esbuild.github.io/api/#how-conditions-work.
*
* If this is set, Wrangler will configure esbuild to use this platform.
*/
export const getBuildPlatformFromEnv = getEnvironmentVariableFactory({
variableName: "WRANGLER_BUILD_PLATFORM",
});
Loading