Skip to content

Commit

Permalink
Revert "feat: add support for custom unenv resolve path" (#7583)
Browse files Browse the repository at this point in the history
* Revert "feat: add support for custom unenv resolve path (#7522)"

This reverts commit 6403e41.

* Create thick-ties-glow.md

* Revert "chore(wrangler): update unenv dependency version (#7541)"

This reverts commit ca9410a.

* fix lockfile
  • Loading branch information
penalosa authored Dec 18, 2024
1 parent c74195c commit 8def8c9
Show file tree
Hide file tree
Showing 10 changed files with 41 additions and 89 deletions.
5 changes: 5 additions & 0 deletions .changeset/thick-ties-glow.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
"wrangler": patch
---

Revert support for custom unenv resolve path to address an issue with Wrangler failing to deploy Pages projects with `nodejs_compat_v2` in some cases
2 changes: 1 addition & 1 deletion fixtures/nodejs-hybrid-app/tests/index.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -64,7 +64,7 @@ describe("nodejs compat", () => {
}
});

test("require unenv aliased packages", async ({ expect }) => {
test("import unenv aliased packages", async ({ expect }) => {
const { ip, port, stop } = await runWranglerDev(
resolve(__dirname, "../src"),
["--port=0", "--inspector-port=0"]
Expand Down
2 changes: 1 addition & 1 deletion packages/wrangler/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -83,7 +83,7 @@
"resolve": "^1.22.8",
"selfsigned": "^2.0.1",
"source-map": "^0.6.1",
"unenv": "npm:[email protected]20241212-153011-af71c96",
"unenv": "npm:[email protected]20241204-140205-a5d5190",
"workerd": "1.20241205.0",
"xxhash-wasm": "^1.0.1"
},
Expand Down
4 changes: 2 additions & 2 deletions packages/wrangler/src/__tests__/deploy.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -10310,7 +10310,7 @@ export default{
fs.writeFileSync(
"index.js",
`
import path from 'node:path';
import path from 'path';
console.log(path);
export default {}
`
Expand All @@ -10331,7 +10331,7 @@ export default{
}
`);
expect(fs.readFileSync("dist/index.js", { encoding: "utf-8" })).toContain(
`import path from "node:path";`
`import path from "path";`
);
});

Expand Down
8 changes: 1 addition & 7 deletions packages/wrangler/src/deployment-bundle/bundle.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,6 @@ import * as esbuild from "esbuild";
import {
getBuildConditionsFromEnv,
getBuildPlatformFromEnv,
getUnenvResolvePathsFromEnv,
} from "../environment-variables/misc-variables";
import { UserError } from "../errors";
import { getFlag } from "../experimental-flags";
Expand Down Expand Up @@ -391,8 +390,6 @@ export async function bundleWorker(
},
};

const unenvResolvePaths = getUnenvResolvePathsFromEnv()?.split(",");

const buildOptions: esbuild.BuildOptions & { metafile: true } = {
// Don't use entryFile here as the file may have been changed when applying the middleware
entryPoints: [entry.file],
Expand Down Expand Up @@ -438,10 +435,7 @@ export async function bundleWorker(
plugins: [
aliasPlugin,
moduleCollector.plugin,
...getNodeJSCompatPlugins({
mode: nodejsCompatMode ?? null,
unenvResolvePaths,
}),
...getNodeJSCompatPlugins(nodejsCompatMode ?? null),
cloudflareInternalPlugin,
buildResultPlugin,
...(plugins || []),
Expand Down
Original file line number Diff line number Diff line change
@@ -1,45 +1,25 @@
import { builtinModules } from "node:module";
import nodePath from "node:path";
import dedent from "ts-dedent";
import { cloudflare, defineEnv } from "unenv";
import { cloudflare, env, nodeless } from "unenv";
import { getBasePath } from "../../paths";
import type { Plugin, PluginBuild } from "esbuild";

const REQUIRED_NODE_BUILT_IN_NAMESPACE = "node-built-in-modules";
const REQUIRED_UNENV_ALIAS_NAMESPACE = "required-unenv-alias";

/**
* ESBuild plugin to apply the unenv preset.
*
* @param unenvResolvePaths Root paths used to resolve absolute paths.
* @returns ESBuild plugin
*/
export function nodejsHybridPlugin(unenvResolvePaths?: string[]): Plugin {
// Get the resolved environment.
const { env } = defineEnv({
nodeCompat: true,
presets: [cloudflare],
resolve: {
paths: unenvResolvePaths,
},
});
const { alias, inject, external } = env;
// Get the unresolved alias.
const unresolvedAlias = defineEnv({
nodeCompat: true,
presets: [cloudflare],
resolve: false,
}).env.alias;
export const nodejsHybridPlugin: () => Plugin = () => {
const { alias, inject, external } = env(nodeless, cloudflare);
return {
name: "hybrid-nodejs_compat",
setup(build) {
errorOnServiceWorkerFormat(build);
handleRequireCallsToNodeJSBuiltins(build);
handleUnenvAliasedPackages(build, unresolvedAlias, alias, external);
handleUnenvAliasedPackages(build, alias, external);
handleNodeJSGlobals(build, inject);
},
};
}
};

const NODEJS_MODULES_RE = new RegExp(`^(node:)?(${builtinModules.join("|")})$`);

Expand Down Expand Up @@ -107,41 +87,45 @@ function handleRequireCallsToNodeJSBuiltins(build: PluginBuild) {
);
}

/**
* Handles aliased NPM packages.
*
* @param build ESBuild PluginBuild.
* @param unresolvedAlias Unresolved aliases from the presets.
* @param alias Aliases resolved to absolute paths.
* @param external external modules.
*/
function handleUnenvAliasedPackages(
build: PluginBuild,
unresolvedAlias: Record<string, string>,
alias: Record<string, string>,
external: string[]
) {
const UNENV_ALIAS_RE = new RegExp(`^(${Object.keys(alias).join("|")})$`);
// esbuild expects alias paths to be absolute
const aliasAbsolute: Record<string, string> = {};
for (const [module, unresolvedAlias] of Object.entries(alias)) {
try {
aliasAbsolute[module] = require
.resolve(unresolvedAlias)
.replace(/\.cjs$/, ".mjs");
} catch (e) {
// this is an alias for package that is not installed in the current app => ignore
}
}

const UNENV_ALIAS_RE = new RegExp(
`^(${Object.keys(aliasAbsolute).join("|")})$`
);

build.onResolve({ filter: UNENV_ALIAS_RE }, (args) => {
const unresolved = unresolvedAlias[args.path];
const unresolvedAlias = alias[args.path];
// Convert `require()` calls for NPM packages to a virtual ES Module that can be imported avoiding the require calls.
// Note: Does not apply to Node.js packages that are handled in `handleRequireCallsToNodeJSBuiltins`
if (
args.kind === "require-call" &&
(unresolved.startsWith("unenv/runtime/npm/") ||
unresolved.startsWith("unenv/runtime/mock/"))
(unresolvedAlias.startsWith("unenv/runtime/npm/") ||
unresolvedAlias.startsWith("unenv/runtime/mock/"))
) {
return {
path: args.path,
namespace: REQUIRED_UNENV_ALIAS_NAMESPACE,
};
}

// Resolve the alias to its absolute path and potentially mark it as external
return {
path: alias[args.path],
external: external.includes(unresolved),
path: aliasAbsolute[args.path],
external: external.includes(unresolvedAlias),
};
});

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -10,13 +10,7 @@ import type { NodeJSCompatMode } from "miniflare";
/**
* Returns the list of ESBuild plugins to use for a given compat mode.
*/
export function getNodeJSCompatPlugins({
mode,
unenvResolvePaths,
}: {
mode: NodeJSCompatMode;
unenvResolvePaths?: string[];
}): Plugin[] {
export function getNodeJSCompatPlugins(mode: NodeJSCompatMode): Plugin[] {
switch (mode) {
case "als":
return [asyncLocalStoragePlugin, nodejsCompatPlugin(mode)];
Expand All @@ -30,7 +24,7 @@ export function getNodeJSCompatPlugins({
case "v1":
return [nodejsCompatPlugin(mode)];
case "v2":
return [nodejsHybridPlugin(unenvResolvePaths)];
return [nodejsHybridPlugin()];
case null:
return [nodejsCompatPlugin(mode)];
}
Expand Down
1 change: 0 additions & 1 deletion packages/wrangler/src/environment-variables/factory.ts
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,6 @@ type VariableNames =
| "WRANGLER_CI_MATCH_TAG"
| "WRANGLER_BUILD_CONDITIONS"
| "WRANGLER_BUILD_PLATFORM"
| "WRANGLER_UNENV_RESOLVE_PATHS"
| "WRANGLER_REGISTRY_PATH";

type DeprecatedNames =
Expand Down
13 changes: 0 additions & 13 deletions packages/wrangler/src/environment-variables/misc-variables.ts
Original file line number Diff line number Diff line change
Expand Up @@ -126,19 +126,6 @@ export const getBuildPlatformFromEnv = getEnvironmentVariableFactory({
variableName: "WRANGLER_BUILD_PLATFORM",
});

/**
* `WRANGLER_UNENV_RESOLVE_PATHS` lists the paths used to resolve unenv.
*
* Note: multiple comma separated paths can be specified.
*
* By default wrangler uses the unenv preset version installed from the package.json.
*
* Setting root paths allow to use a different version of the preset.
*/
export const getUnenvResolvePathsFromEnv = getEnvironmentVariableFactory({
variableName: "WRANGLER_UNENV_RESOLVE_PATHS",
});

/**
* `WRANGLER_REGISTRY_PATH` specifies the file based dev registry folder
*/
Expand Down
19 changes: 4 additions & 15 deletions pnpm-lock.yaml

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

0 comments on commit 8def8c9

Please sign in to comment.