Skip to content

Commit 8def8c9

Browse files
authored
Revert "feat: add support for custom unenv resolve path" (#7583)
* 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
1 parent c74195c commit 8def8c9

File tree

10 files changed

+41
-89
lines changed

10 files changed

+41
-89
lines changed

.changeset/thick-ties-glow.md

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
---
2+
"wrangler": patch
3+
---
4+
5+
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

fixtures/nodejs-hybrid-app/tests/index.test.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -64,7 +64,7 @@ describe("nodejs compat", () => {
6464
}
6565
});
6666

67-
test("require unenv aliased packages", async ({ expect }) => {
67+
test("import unenv aliased packages", async ({ expect }) => {
6868
const { ip, port, stop } = await runWranglerDev(
6969
resolve(__dirname, "../src"),
7070
["--port=0", "--inspector-port=0"]

packages/wrangler/package.json

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -83,7 +83,7 @@
8383
"resolve": "^1.22.8",
8484
"selfsigned": "^2.0.1",
8585
"source-map": "^0.6.1",
86-
"unenv": "npm:[email protected]20241212-153011-af71c96",
86+
"unenv": "npm:[email protected]20241204-140205-a5d5190",
8787
"workerd": "1.20241205.0",
8888
"xxhash-wasm": "^1.0.1"
8989
},

packages/wrangler/src/__tests__/deploy.test.ts

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -10310,7 +10310,7 @@ export default{
1031010310
fs.writeFileSync(
1031110311
"index.js",
1031210312
`
10313-
import path from 'node:path';
10313+
import path from 'path';
1031410314
console.log(path);
1031510315
export default {}
1031610316
`
@@ -10331,7 +10331,7 @@ export default{
1033110331
}
1033210332
`);
1033310333
expect(fs.readFileSync("dist/index.js", { encoding: "utf-8" })).toContain(
10334-
`import path from "node:path";`
10334+
`import path from "path";`
1033510335
);
1033610336
});
1033710337

packages/wrangler/src/deployment-bundle/bundle.ts

Lines changed: 1 addition & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,6 @@ import * as esbuild from "esbuild";
55
import {
66
getBuildConditionsFromEnv,
77
getBuildPlatformFromEnv,
8-
getUnenvResolvePathsFromEnv,
98
} from "../environment-variables/misc-variables";
109
import { UserError } from "../errors";
1110
import { getFlag } from "../experimental-flags";
@@ -391,8 +390,6 @@ export async function bundleWorker(
391390
},
392391
};
393392

394-
const unenvResolvePaths = getUnenvResolvePathsFromEnv()?.split(",");
395-
396393
const buildOptions: esbuild.BuildOptions & { metafile: true } = {
397394
// Don't use entryFile here as the file may have been changed when applying the middleware
398395
entryPoints: [entry.file],
@@ -438,10 +435,7 @@ export async function bundleWorker(
438435
plugins: [
439436
aliasPlugin,
440437
moduleCollector.plugin,
441-
...getNodeJSCompatPlugins({
442-
mode: nodejsCompatMode ?? null,
443-
unenvResolvePaths,
444-
}),
438+
...getNodeJSCompatPlugins(nodejsCompatMode ?? null),
445439
cloudflareInternalPlugin,
446440
buildResultPlugin,
447441
...(plugins || []),

packages/wrangler/src/deployment-bundle/esbuild-plugins/hybrid-nodejs-compat.ts

Lines changed: 25 additions & 41 deletions
Original file line numberDiff line numberDiff line change
@@ -1,45 +1,25 @@
11
import { builtinModules } from "node:module";
22
import nodePath from "node:path";
33
import dedent from "ts-dedent";
4-
import { cloudflare, defineEnv } from "unenv";
4+
import { cloudflare, env, nodeless } from "unenv";
55
import { getBasePath } from "../../paths";
66
import type { Plugin, PluginBuild } from "esbuild";
77

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

11-
/**
12-
* ESBuild plugin to apply the unenv preset.
13-
*
14-
* @param unenvResolvePaths Root paths used to resolve absolute paths.
15-
* @returns ESBuild plugin
16-
*/
17-
export function nodejsHybridPlugin(unenvResolvePaths?: string[]): Plugin {
18-
// Get the resolved environment.
19-
const { env } = defineEnv({
20-
nodeCompat: true,
21-
presets: [cloudflare],
22-
resolve: {
23-
paths: unenvResolvePaths,
24-
},
25-
});
26-
const { alias, inject, external } = env;
27-
// Get the unresolved alias.
28-
const unresolvedAlias = defineEnv({
29-
nodeCompat: true,
30-
presets: [cloudflare],
31-
resolve: false,
32-
}).env.alias;
11+
export const nodejsHybridPlugin: () => Plugin = () => {
12+
const { alias, inject, external } = env(nodeless, cloudflare);
3313
return {
3414
name: "hybrid-nodejs_compat",
3515
setup(build) {
3616
errorOnServiceWorkerFormat(build);
3717
handleRequireCallsToNodeJSBuiltins(build);
38-
handleUnenvAliasedPackages(build, unresolvedAlias, alias, external);
18+
handleUnenvAliasedPackages(build, alias, external);
3919
handleNodeJSGlobals(build, inject);
4020
},
4121
};
42-
}
22+
};
4323

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

@@ -107,41 +87,45 @@ function handleRequireCallsToNodeJSBuiltins(build: PluginBuild) {
10787
);
10888
}
10989

110-
/**
111-
* Handles aliased NPM packages.
112-
*
113-
* @param build ESBuild PluginBuild.
114-
* @param unresolvedAlias Unresolved aliases from the presets.
115-
* @param alias Aliases resolved to absolute paths.
116-
* @param external external modules.
117-
*/
11890
function handleUnenvAliasedPackages(
11991
build: PluginBuild,
120-
unresolvedAlias: Record<string, string>,
12192
alias: Record<string, string>,
12293
external: string[]
12394
) {
124-
const UNENV_ALIAS_RE = new RegExp(`^(${Object.keys(alias).join("|")})$`);
95+
// esbuild expects alias paths to be absolute
96+
const aliasAbsolute: Record<string, string> = {};
97+
for (const [module, unresolvedAlias] of Object.entries(alias)) {
98+
try {
99+
aliasAbsolute[module] = require
100+
.resolve(unresolvedAlias)
101+
.replace(/\.cjs$/, ".mjs");
102+
} catch (e) {
103+
// this is an alias for package that is not installed in the current app => ignore
104+
}
105+
}
106+
107+
const UNENV_ALIAS_RE = new RegExp(
108+
`^(${Object.keys(aliasAbsolute).join("|")})$`
109+
);
125110

126111
build.onResolve({ filter: UNENV_ALIAS_RE }, (args) => {
127-
const unresolved = unresolvedAlias[args.path];
112+
const unresolvedAlias = alias[args.path];
128113
// Convert `require()` calls for NPM packages to a virtual ES Module that can be imported avoiding the require calls.
129114
// Note: Does not apply to Node.js packages that are handled in `handleRequireCallsToNodeJSBuiltins`
130115
if (
131116
args.kind === "require-call" &&
132-
(unresolved.startsWith("unenv/runtime/npm/") ||
133-
unresolved.startsWith("unenv/runtime/mock/"))
117+
(unresolvedAlias.startsWith("unenv/runtime/npm/") ||
118+
unresolvedAlias.startsWith("unenv/runtime/mock/"))
134119
) {
135120
return {
136121
path: args.path,
137122
namespace: REQUIRED_UNENV_ALIAS_NAMESPACE,
138123
};
139124
}
140-
141125
// Resolve the alias to its absolute path and potentially mark it as external
142126
return {
143-
path: alias[args.path],
144-
external: external.includes(unresolved),
127+
path: aliasAbsolute[args.path],
128+
external: external.includes(unresolvedAlias),
145129
};
146130
});
147131

packages/wrangler/src/deployment-bundle/esbuild-plugins/nodejs-plugins.ts

Lines changed: 2 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -10,13 +10,7 @@ import type { NodeJSCompatMode } from "miniflare";
1010
/**
1111
* Returns the list of ESBuild plugins to use for a given compat mode.
1212
*/
13-
export function getNodeJSCompatPlugins({
14-
mode,
15-
unenvResolvePaths,
16-
}: {
17-
mode: NodeJSCompatMode;
18-
unenvResolvePaths?: string[];
19-
}): Plugin[] {
13+
export function getNodeJSCompatPlugins(mode: NodeJSCompatMode): Plugin[] {
2014
switch (mode) {
2115
case "als":
2216
return [asyncLocalStoragePlugin, nodejsCompatPlugin(mode)];
@@ -30,7 +24,7 @@ export function getNodeJSCompatPlugins({
3024
case "v1":
3125
return [nodejsCompatPlugin(mode)];
3226
case "v2":
33-
return [nodejsHybridPlugin(unenvResolvePaths)];
27+
return [nodejsHybridPlugin()];
3428
case null:
3529
return [nodejsCompatPlugin(mode)];
3630
}

packages/wrangler/src/environment-variables/factory.ts

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -25,7 +25,6 @@ type VariableNames =
2525
| "WRANGLER_CI_MATCH_TAG"
2626
| "WRANGLER_BUILD_CONDITIONS"
2727
| "WRANGLER_BUILD_PLATFORM"
28-
| "WRANGLER_UNENV_RESOLVE_PATHS"
2928
| "WRANGLER_REGISTRY_PATH";
3029

3130
type DeprecatedNames =

packages/wrangler/src/environment-variables/misc-variables.ts

Lines changed: 0 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -126,19 +126,6 @@ export const getBuildPlatformFromEnv = getEnvironmentVariableFactory({
126126
variableName: "WRANGLER_BUILD_PLATFORM",
127127
});
128128

129-
/**
130-
* `WRANGLER_UNENV_RESOLVE_PATHS` lists the paths used to resolve unenv.
131-
*
132-
* Note: multiple comma separated paths can be specified.
133-
*
134-
* By default wrangler uses the unenv preset version installed from the package.json.
135-
*
136-
* Setting root paths allow to use a different version of the preset.
137-
*/
138-
export const getUnenvResolvePathsFromEnv = getEnvironmentVariableFactory({
139-
variableName: "WRANGLER_UNENV_RESOLVE_PATHS",
140-
});
141-
142129
/**
143130
* `WRANGLER_REGISTRY_PATH` specifies the file based dev registry folder
144131
*/

pnpm-lock.yaml

Lines changed: 4 additions & 15 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

0 commit comments

Comments
 (0)