Skip to content

Commit e75c581

Browse files
petebacondarwinpetebacondarwin
authoredAug 21, 2024··
Nodejs compat v2 on pages (#6511)
* refactor: use the computed `compatibilityFlags` variable * refactor: do not mutate the compatibility flags array in `validateNodeCompat()` This makes it possible to call `validateNodeCompat()` mulitple times without breaking anything. * refactor: move the Pages assets esbuild plugin to its own function * refactor: remove duplication from hybrid nodejs compat plugin * fix: ensure that `process` global works correctly in node.js compat v2 * test: add fixture and tests for nodejs_compat_v2 on Pages * PR review updates * Add TODO for removing experimental: prefix
·
wrangler@4.5.0@cloudflare/chrome-devtools-patches@0.1.1
1 parent 090a596 commit e75c581

File tree

24 files changed

+435
-185
lines changed

24 files changed

+435
-185
lines changed
 

‎.changeset/good-kids-melt.md

+7
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,7 @@
1+
---
2+
"wrangler": patch
3+
---
4+
5+
fix: allow Pages projects to use `experimental:nodejs_compat_v2" flag
6+
7+
Fixews #6288

‎.vscode/settings.json

+2
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,7 @@
2424
"mockpm",
2525
"mrbbot",
2626
"mtls",
27+
"nodeless",
2728
"outdir",
2829
"outfile",
2930
"pgrep",
@@ -38,6 +39,7 @@
3839
"tsbuildinfo",
3940
"turborepo",
4041
"undici",
42+
"unenv",
4143
"unrevoke",
4244
"Untriaged",
4345
"versionless",

‎fixtures/nodejs-hybrid-app/src/index.ts

+119-19
Original file line numberDiff line numberDiff line change
@@ -38,24 +38,124 @@ export default {
3838
env: Env,
3939
ctx: ExecutionContext
4040
): Promise<Response> {
41-
const client = new Client({
42-
user: env.DB_USERNAME,
43-
password: env.DB_PASSWORD,
44-
host: env.DB_HOSTNAME,
45-
port: Number(env.DB_PORT),
46-
database: env.DB_NAME,
47-
});
48-
await client.connect();
49-
const result = await client.query(`SELECT * FROM rnc_database`);
50-
assert(true);
51-
52-
// Return the first row as JSON
53-
const resp = new Response(JSON.stringify(result.rows[0]), {
54-
headers: { "Content-Type": "application/json" },
55-
});
56-
57-
// Clean up the client
58-
ctx.waitUntil(client.end());
59-
return resp;
41+
const url = new URL(request.url);
42+
if (url.pathname === "/test-process") {
43+
const originalProcess = process;
44+
try {
45+
assert(process !== undefined, "process is missing");
46+
assert(
47+
globalThis.process !== undefined,
48+
"globalThis.process is missing"
49+
);
50+
assert(global.process !== undefined, "global.process is missing");
51+
assert(
52+
process === global.process,
53+
"process is not the same as global.process"
54+
);
55+
assert(
56+
global.process === globalThis.process,
57+
"global.process is not the same as globalThis.process"
58+
);
59+
assert(
60+
globalThis.process === process,
61+
"globalThis.process is not the same as process"
62+
);
63+
64+
const fakeProcess1 = {} as typeof process;
65+
process = fakeProcess1;
66+
assert(
67+
process === fakeProcess1,
68+
"process is not updated to fakeProcess"
69+
);
70+
assert(
71+
global.process === fakeProcess1,
72+
"global.process is not updated to fakeProcess"
73+
);
74+
assert(
75+
globalThis.process === fakeProcess1,
76+
"globalThis.process is not updated to fakeProcess"
77+
);
78+
79+
const fakeProcess2 = {} as typeof process;
80+
global.process = fakeProcess2;
81+
assert(
82+
process === fakeProcess2,
83+
"process is not updated to fakeProcess"
84+
);
85+
assert(
86+
global.process === fakeProcess2,
87+
"global.process is not updated to fakeProcess"
88+
);
89+
assert(
90+
globalThis.process === fakeProcess2,
91+
"globalThis.process is not updated to fakeProcess"
92+
);
93+
94+
const fakeProcess3 = {} as typeof process;
95+
globalThis.process = fakeProcess3;
96+
assert(
97+
process === fakeProcess3,
98+
"process is not updated to fakeProcess"
99+
);
100+
assert(
101+
global.process === fakeProcess3,
102+
"global.process is not updated to fakeProcess"
103+
);
104+
assert(
105+
globalThis.process === fakeProcess3,
106+
"globalThis.process is not updated to fakeProcess"
107+
);
108+
109+
const fakeProcess4 = {} as typeof process;
110+
globalThis["process"] = fakeProcess4;
111+
assert(
112+
process === fakeProcess4,
113+
"process is not updated to fakeProcess"
114+
);
115+
assert(
116+
global.process === fakeProcess4,
117+
"global.process is not updated to fakeProcess"
118+
);
119+
assert(
120+
globalThis.process === fakeProcess4,
121+
"globalThis.process is not updated to fakeProcess"
122+
);
123+
} catch (e) {
124+
if (e instanceof Error) {
125+
return new Response(`${e.stack}`, { status: 500 });
126+
} else {
127+
throw e;
128+
}
129+
} finally {
130+
process = originalProcess;
131+
}
132+
133+
return new Response("OK!");
134+
}
135+
136+
if (url.pathname === "/query") {
137+
const client = new Client({
138+
user: env.DB_USERNAME,
139+
password: env.DB_PASSWORD,
140+
host: env.DB_HOSTNAME,
141+
port: Number(env.DB_PORT),
142+
database: env.DB_NAME,
143+
});
144+
await client.connect();
145+
const result = await client.query(`SELECT * FROM rnc_database`);
146+
// Return the first row as JSON
147+
const resp = new Response(JSON.stringify(result.rows[0]), {
148+
headers: { "Content-Type": "application/json" },
149+
});
150+
151+
// Clean up the client
152+
ctx.waitUntil(client.end());
153+
return resp;
154+
} else {
155+
return new Response(
156+
'<a href="query">Postgres query</a> | <a href="test-process">Test process global</a>',
157+
{ headers: { "Content-Type": "text/html; charset=utf-8" } }
158+
);
159+
}
60160
},
61161
};

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

+6-2
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
import { resolve } from "node:path";
2-
// import { fetch } from "undici";
2+
import { fetch } from "undici";
33
import { describe, it } from "vitest";
44
import { runWranglerDev } from "../../shared/src/run-wrangler-long-lived";
55

@@ -12,9 +12,13 @@ describe("nodejs compat", () => {
1212
["--port=0", "--inspector-port=0"]
1313
);
1414
try {
15+
const response = await fetch(`http://${ip}:${port}/test-process`);
16+
const body = await response.text();
17+
expect(body).toMatchInlineSnapshot(`"OK!"`);
18+
1519
// Disabling actually querying the database since we are getting this error:
1620
// > too many connections for role 'reader'
17-
// const response = await fetch(`http://${ip}:${port}`);
21+
// const response = await fetch(`http://${ip}:${port}/query`);
1822
// const body = await response.text();
1923
// console.log(body);
2024
// const result = JSON.parse(body) as { id: string };

‎fixtures/pages-nodejs-v2-compat/apps/functions/README.md

+5
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
# Page Functions fixture
2+
3+
This directory is just here to avoid picking up any other \_worker.js files in the other app.
4+
The actual Pages Functions handlers are in the `functions` directory at the root of the fixture.
5+
They have to be there because Pages likes `wrangler pages dev` to be run from the root of the fixture and it always looks for a `functions` directory in that root.

‎fixtures/pages-nodejs-v2-compat/apps/workerjs-directory/README.md

+1
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
# \_worker.js directory fixture

‎fixtures/pages-nodejs-v2-compat/apps/workerjs-directory/_worker.js/index.js

+12
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,12 @@
1+
process.env.foo = "bar";
2+
3+
// Check that we don't get a build time error when assigning to globalThis.process.
4+
globalThis.process = process;
5+
6+
export default {
7+
fetch() {
8+
return new Response(
9+
`_worker.js directory, process: ${Object.keys(process).sort()}`
10+
);
11+
},
12+
};

‎fixtures/pages-nodejs-v2-compat/apps/workerjs-file/README.md

+1
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
# \_worker.js file fixture

‎fixtures/pages-nodejs-v2-compat/apps/workerjs-file/_worker.js

+12
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,12 @@
1+
process.env.foo = "bar";
2+
3+
// Check that we don't get a build time error when assigning to globalThis.process.
4+
globalThis.process = process;
5+
6+
export default {
7+
fetch() {
8+
return new Response(
9+
`_worker.js file, process: ${Object.keys(process).sort()}`
10+
);
11+
},
12+
};

‎fixtures/pages-nodejs-v2-compat/functions/[[path]].ts

+10
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,10 @@
1+
process.env.foo = "bar";
2+
3+
// Check that we don't get a build time error when assigning to globalThis.process.
4+
globalThis.process = process;
5+
6+
export const onRequest = () => {
7+
return new Response(
8+
`Pages functions, process: ${Object.keys(process).sort()}`
9+
);
10+
};

‎fixtures/pages-nodejs-v2-compat/package.json

+25
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,25 @@
1+
{
2+
"name": "pages-nodejs-v2-compat",
3+
"private": true,
4+
"sideEffects": false,
5+
"scripts": {
6+
"check:type": "tsc",
7+
"dev:functions-app": "npx wrangler pages dev ./apps/functions --port 8792",
8+
"dev:workerjs-directory": "npx wrangler pages dev ./apps/workerjs-directory --port 8792",
9+
"dev:workerjs-file": "npx wrangler pages dev ./apps/workerjs-file --port 8792",
10+
"test:ci": "vitest run",
11+
"test:watch": "vitest",
12+
"type:tests": "tsc -p ./tests/tsconfig.json"
13+
},
14+
"devDependencies": {
15+
"@cloudflare/workers-tsconfig": "workspace:^",
16+
"undici": "^5.28.4",
17+
"wrangler": "workspace:*"
18+
},
19+
"engines": {
20+
"node": ">=16.13"
21+
},
22+
"volta": {
23+
"extends": "../../package.json"
24+
}
25+
}

‎fixtures/pages-nodejs-v2-compat/tests/index.test.ts

+54
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,54 @@
1+
import { resolve } from "node:path";
2+
import { fetch } from "undici";
3+
import { describe, it } from "vitest";
4+
import { runWranglerPagesDev } from "../../shared/src/run-wrangler-long-lived";
5+
6+
describe("Pages with Node.js compat v2", () => {
7+
describe("with _worker.js file", () => {
8+
it("should polyfill `process`", async ({ expect, onTestFinished }) => {
9+
const { ip, port, stop } = await runWranglerPagesDev(
10+
resolve(__dirname, ".."),
11+
"./apps/workerjs-file",
12+
["--port=0", "--inspector-port=0"]
13+
);
14+
onTestFinished(stop);
15+
const response = await fetch(`http://${ip}:${port}/`);
16+
const body = await response.text();
17+
expect(body).toMatchInlineSnapshot(
18+
`"_worker.js file, process: _debugEnd,_debugProcess,_eventsCount,_fatalException,_getActiveHandles,_getActiveRequests,_kill,_preload_modules,_rawDebug,_startProfilerIdleNotifier,_stopProfilerIdleNotifier,_tickCallback,abort,addListener,allowedNodeEnvironmentFlags,arch,argv,argv0,assert,availableMemory,binding,chdir,config,constrainedMemory,cpuUsage,cwd,debugPort,dlopen,emit,emitWarning,env,eventNames,execArgv,execPath,exit,exitCode,features,getActiveResourcesInfo,getBuiltinModule,getMaxListeners,getegid,geteuid,getgid,getgroups,getuid,hasUncaughtExceptionCaptureCallback,hrtime,kill,listenerCount,listeners,loadEnvFile,memoryUsage,nextTick,off,on,once,pid,platform,ppid,prependListener,prependOnceListener,rawListeners,release,removeAllListeners,removeListener,report,resourceUsage,setMaxListeners,setSourceMapsEnabled,setUncaughtExceptionCaptureCallback,setegid,seteuid,setgid,setgroups,setuid,sourceMapsEnabled,stderr,stdin,stdout,title,umask,uptime,version,versions"`
19+
);
20+
});
21+
});
22+
23+
describe("with _worker.js directory", () => {
24+
it("should polyfill `process`", async ({ expect, onTestFinished }) => {
25+
const { ip, port, stop } = await runWranglerPagesDev(
26+
resolve(__dirname, ".."),
27+
"./apps/workerjs-directory",
28+
["--port=0", "--inspector-port=0"]
29+
);
30+
onTestFinished(stop);
31+
const response = await fetch(`http://${ip}:${port}/`);
32+
const body = await response.text();
33+
expect(body).toMatchInlineSnapshot(
34+
`"_worker.js directory, process: _debugEnd,_debugProcess,_eventsCount,_fatalException,_getActiveHandles,_getActiveRequests,_kill,_preload_modules,_rawDebug,_startProfilerIdleNotifier,_stopProfilerIdleNotifier,_tickCallback,abort,addListener,allowedNodeEnvironmentFlags,arch,argv,argv0,assert,availableMemory,binding,chdir,config,constrainedMemory,cpuUsage,cwd,debugPort,dlopen,emit,emitWarning,env,eventNames,execArgv,execPath,exit,exitCode,features,getActiveResourcesInfo,getBuiltinModule,getMaxListeners,getegid,geteuid,getgid,getgroups,getuid,hasUncaughtExceptionCaptureCallback,hrtime,kill,listenerCount,listeners,loadEnvFile,memoryUsage,nextTick,off,on,once,pid,platform,ppid,prependListener,prependOnceListener,rawListeners,release,removeAllListeners,removeListener,report,resourceUsage,setMaxListeners,setSourceMapsEnabled,setUncaughtExceptionCaptureCallback,setegid,seteuid,setgid,setgroups,setuid,sourceMapsEnabled,stderr,stdin,stdout,title,umask,uptime,version,versions"`
35+
);
36+
});
37+
});
38+
39+
describe("with Pages functions", () => {
40+
it("should polyfill `process`", async ({ expect, onTestFinished }) => {
41+
const { ip, port, stop } = await runWranglerPagesDev(
42+
resolve(__dirname, ".."),
43+
"./apps/functions",
44+
["--port=0", "--inspector-port=0"]
45+
);
46+
onTestFinished(stop);
47+
const response = await fetch(`http://${ip}:${port}/`);
48+
const body = await response.text();
49+
expect(body).toMatchInlineSnapshot(
50+
`"Pages functions, process: _debugEnd,_debugProcess,_eventsCount,_fatalException,_getActiveHandles,_getActiveRequests,_kill,_preload_modules,_rawDebug,_startProfilerIdleNotifier,_stopProfilerIdleNotifier,_tickCallback,abort,addListener,allowedNodeEnvironmentFlags,arch,argv,argv0,assert,availableMemory,binding,chdir,config,constrainedMemory,cpuUsage,cwd,debugPort,dlopen,emit,emitWarning,env,eventNames,execArgv,execPath,exit,exitCode,features,getActiveResourcesInfo,getBuiltinModule,getMaxListeners,getegid,geteuid,getgid,getgroups,getuid,hasUncaughtExceptionCaptureCallback,hrtime,kill,listenerCount,listeners,loadEnvFile,memoryUsage,nextTick,off,on,once,pid,platform,ppid,prependListener,prependOnceListener,rawListeners,release,removeAllListeners,removeListener,report,resourceUsage,setMaxListeners,setSourceMapsEnabled,setUncaughtExceptionCaptureCallback,setegid,seteuid,setgid,setgroups,setuid,sourceMapsEnabled,stderr,stdin,stdout,title,umask,uptime,version,versions"`
51+
);
52+
});
53+
});
54+
});

‎fixtures/pages-nodejs-v2-compat/tests/tsconfig.json

+7
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,7 @@
1+
{
2+
"extends": "@cloudflare/workers-tsconfig/tsconfig.json",
3+
"compilerOptions": {
4+
"types": ["node"]
5+
},
6+
"include": ["**/*.ts", "../../../node-types.d.ts"]
7+
}

‎fixtures/pages-nodejs-v2-compat/tsconfig.json

+20
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,20 @@
1+
{
2+
"compilerOptions": {
3+
"target": "ES2020",
4+
"esModuleInterop": true,
5+
"module": "CommonJS",
6+
"lib": ["ES2020"],
7+
"types": ["node", "@cloudflare/workers-types"],
8+
"moduleResolution": "node",
9+
"noEmit": true,
10+
"skipLibCheck": true,
11+
"checkJs": true
12+
},
13+
"include": [
14+
"apps/workerjs-directory/_worker.js/index.js",
15+
"apps/workerjs-file/_worker.js",
16+
"functions/[[path]].ts",
17+
"tests",
18+
"../../node-types.d.ts"
19+
]
20+
}

‎fixtures/pages-nodejs-v2-compat/vitest.config.mts

+9
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,9 @@
1+
import { defineProject, mergeConfig } from "vitest/config";
2+
import configShared from "../../vitest.shared";
3+
4+
export default mergeConfig(
5+
configShared,
6+
defineProject({
7+
test: {},
8+
})
9+
);

‎fixtures/pages-nodejs-v2-compat/wrangler.toml

+3
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,3 @@
1+
name = "pages-nodejs-compat"
2+
compatibility_date = "2024-08-20"
3+
compatibility_flags = ["experimental:nodejs_compat_v2"]

‎packages/wrangler/src/deploy/deploy.ts

+3-3
Original file line numberDiff line numberDiff line change
@@ -395,13 +395,13 @@ See https://developers.cloudflare.com/workers/platform/compatibility-dates for m
395395

396396
const minify = props.minify ?? config.minify;
397397

398+
const compatibilityFlags =
399+
props.compatibilityFlags ?? config.compatibility_flags;
398400
const nodejsCompatMode = validateNodeCompat({
399401
legacyNodeCompat: props.nodeCompat ?? config.node_compat ?? false,
400-
compatibilityFlags: props.compatibilityFlags ?? config.compatibility_flags,
402+
compatibilityFlags,
401403
noBundle: props.noBundle ?? config.no_bundle ?? false,
402404
});
403-
const compatibilityFlags =
404-
props.compatibilityFlags ?? config.compatibility_flags;
405405

406406
// Warn if user tries minify with no-bundle
407407
if (props.noBundle && minify) {

‎packages/wrangler/src/deployment-bundle/create-worker-upload-form.ts

+4-1
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@ import { readFileSync } from "node:fs";
33
import path from "node:path";
44
import { File, FormData } from "undici";
55
import { handleUnsafeCapnp } from "./capnp";
6+
import { stripExperimentalPrefixes } from "./node-compat";
67
import type {
78
CfDurableObjectMigrations,
89
CfModuleType,
@@ -545,7 +546,9 @@ export function createWorkerUploadForm(worker: CfWorkerInit): FormData {
545546
: { body_part: main.name }),
546547
bindings: metadataBindings,
547548
...(compatibility_date && { compatibility_date }),
548-
...(compatibility_flags && { compatibility_flags }),
549+
...(compatibility_flags && {
550+
compatibility_flags: stripExperimentalPrefixes(compatibility_flags),
551+
}),
549552
...(migrations && { migrations }),
550553
capnp_schema: capnpSchemaOutputFile,
551554
...(keep_bindings && { keep_bindings }),

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

+27-74
Original file line numberDiff line numberDiff line change
@@ -91,15 +91,16 @@ function handleNodeJSGlobals(
9191
inject: Record<string, string | string[]>
9292
) {
9393
const UNENV_GLOBALS_RE = /_virtual_unenv_global_polyfill-([^.]+)\.js$/;
94+
const prefix = nodePath.resolve(
95+
getBasePath(),
96+
"_virtual_unenv_global_polyfill-"
97+
);
9498

9599
build.initialOptions.inject = [
96100
...(build.initialOptions.inject ?? []),
97101
//convert unenv's inject keys to absolute specifiers of custom virtual modules that will be provided via a custom onLoad
98-
...Object.keys(inject).map((globalName) =>
99-
nodePath.resolve(
100-
getBasePath(),
101-
`_virtual_unenv_global_polyfill-${encodeToLowerCase(globalName)}.js`
102-
)
102+
...Object.keys(inject).map(
103+
(globalName) => `${prefix}${encodeToLowerCase(globalName)}.js`
103104
),
104105
];
105106

@@ -108,81 +109,33 @@ function handleNodeJSGlobals(
108109
build.onLoad({ filter: UNENV_GLOBALS_RE }, ({ path }) => {
109110
// eslint-disable-next-line @typescript-eslint/no-non-null-assertion
110111
const globalName = decodeFromLowerCase(path.match(UNENV_GLOBALS_RE)![1]);
111-
const globalMapping = inject[globalName];
112-
113-
if (typeof globalMapping === "string") {
114-
const globalPolyfillSpecifier = globalMapping;
115-
116-
return {
117-
contents: `
118-
import globalVar from "${globalPolyfillSpecifier}";
119-
120-
${
121-
/*
122-
// ESBuild's inject doesn't actually touch globalThis, so let's do it ourselves
123-
// by creating an exportable so that we can preserve the globalThis assignment if
124-
// the ${globalName} was found in the app, or tree-shake it, if it wasn't
125-
// see https://esbuild.github.io/api/#inject
126-
*/ ""
127-
}
128-
const exportable =
129-
${
130-
/*
131-
// mark this as a PURE call so it can be ignored and tree-shaken by ESBuild,
132-
// when we don't detect 'process', 'global.process', or 'globalThis.process'
133-
// in the app code
134-
// see https://esbuild.github.io/api/#tree-shaking-and-side-effects
135-
*/ ""
136-
}
137-
/* @__PURE__ */ (() => {
138-
return globalThis.${globalName} = globalVar;
139-
})();
140-
141-
export {
142-
exportable as '${globalName}',
143-
exportable as 'globalThis.${globalName}',
144-
}
145-
`,
146-
};
147-
}
148-
149-
const [moduleName, exportName] = inject[globalName];
112+
const { importStatement, exportName } = getGlobalInject(inject[globalName]);
150113

151114
return {
152-
contents: `
153-
import { ${exportName} } from "${moduleName}";
154-
155-
${
156-
/*
157-
// ESBuild's inject doesn't actually touch globalThis, so let's do it ourselves
158-
// by creating an exportable so that we can preserve the globalThis assignment if
159-
// the ${globalName} was found in the app, or tree-shake it, if it wasn't
160-
// see https://esbuild.github.io/api/#inject
161-
*/ ""
162-
}
163-
const exportable =
164-
${
165-
/*
166-
// mark this as a PURE call so it can be ignored and tree-shaken by ESBuild,
167-
// when we don't detect 'process', 'global.process', or 'globalThis.process'
168-
// in the app code
169-
// see https://esbuild.github.io/api/#tree-shaking-and-side-effects
170-
*/ ""
171-
}
172-
/* @__PURE__ */ (() => {
173-
return globalThis.${globalName} = ${exportName};
174-
})();
175-
176-
export {
177-
exportable as '${globalName}',
178-
exportable as 'global.${globalName}',
179-
exportable as 'globalThis.${globalName}'
180-
}
181-
`,
115+
contents: `${importStatement}\nglobalThis.${globalName} = ${exportName};`,
182116
};
183117
});
184118
}
185119

120+
/**
121+
* Get the import statement and export name to be used for the given global inject setting.
122+
*/
123+
function getGlobalInject(globalInject: string | string[]) {
124+
if (typeof globalInject === "string") {
125+
// the mapping is a simple string, indicating a default export, so the string is just the module specifier.
126+
return {
127+
importStatement: `import globalVar from "${globalInject}";`,
128+
exportName: "globalVar",
129+
};
130+
}
131+
// the mapping is a 2 item tuple, indicating a named export, made up of a module specifier and an export name.
132+
const [moduleSpecifier, exportName] = globalInject;
133+
return {
134+
importStatement: `import { ${exportName} } from "${moduleSpecifier}";`,
135+
exportName,
136+
};
137+
}
138+
186139
/**
187140
* Encodes a case sensitive string to lowercase string by prefixing all uppercase letters
188141
* with $ and turning them into lowercase letters.

‎packages/wrangler/src/deployment-bundle/node-compat.ts

+13-7
Original file line numberDiff line numberDiff line change
@@ -51,13 +51,6 @@ export function validateNodeCompat({
5151
const nodejsCompatV2NotExperimental =
5252
compatibilityFlags.includes("nodejs_compat_v2");
5353

54-
if (nodejsCompatV2) {
55-
// strip the "experimental:" prefix because workerd doesn't understand it yet.
56-
compatibilityFlags[
57-
compatibilityFlags.indexOf("experimental:nodejs_compat_v2")
58-
] = "nodejs_compat_v2";
59-
}
60-
6154
if (nodejsCompat && nodejsCompatV2) {
6255
throw new UserError(
6356
"The `nodejs_compat` and `nodejs_compat_v2` compatibility flags cannot be used in together. Please select just one."
@@ -124,3 +117,16 @@ export function getNodeCompatMode({
124117
nodejsCompatV2,
125118
};
126119
}
120+
121+
/**
122+
* The nodejs_compat_v2 flag currently requires an `experimental:` prefix within Wrangler,
123+
* but this needs to be stripped before sending to workerd, since that doesn't know about that.
124+
*
125+
* TODO: Remove this function when we graduate nodejs_v2 to non-experimental.
126+
* See https://jira.cfdata.org/browse/DEVDASH-218
127+
*/
128+
export function stripExperimentalPrefixes(
129+
compatFlags: string[] | undefined
130+
): string[] | undefined {
131+
return compatFlags?.map((flag) => flag.replace(/^experimental:/, ""));
132+
}

‎packages/wrangler/src/dev/miniflare.ts

+4-1
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,7 @@ import {
1818
} from "../ai/fetcher";
1919
import { readConfig } from "../config";
2020
import { ModuleTypeToRuleType } from "../deployment-bundle/module-collection";
21+
import { stripExperimentalPrefixes } from "../deployment-bundle/node-compat";
2122
import { withSourceURLs } from "../deployment-bundle/source-url";
2223
import { UserError } from "../errors";
2324
import { logger } from "../logger";
@@ -891,7 +892,9 @@ export async function buildMiniflareOptions(
891892
{
892893
name: getName(config),
893894
compatibilityDate: config.compatibilityDate,
894-
compatibilityFlags: config.compatibilityFlags,
895+
compatibilityFlags: stripExperimentalPrefixes(
896+
config.compatibilityFlags
897+
),
895898

896899
...sourceOptions,
897900
...bindingOptions,

‎packages/wrangler/src/dev/remote.tsx

+2-1
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@ import { useErrorHandler } from "react-error-boundary";
77
import { helpIfErrorIsSizeOrScriptStartup } from "../deploy/deploy";
88
import { printBundleSize } from "../deployment-bundle/bundle-reporter";
99
import { getBundleType } from "../deployment-bundle/bundle-type";
10+
import { stripExperimentalPrefixes } from "../deployment-bundle/node-compat";
1011
import { withSourceURLs } from "../deployment-bundle/source-url";
1112
import { getInferredHost } from "../dev";
1213
import { UserError } from "../errors";
@@ -668,7 +669,7 @@ export async function createRemoteWorkerInit(props: {
668669
},
669670
migrations: undefined, // no migrations in dev
670671
compatibility_date: props.compatibilityDate,
671-
compatibility_flags: props.compatibilityFlags,
672+
compatibility_flags: stripExperimentalPrefixes(props.compatibilityFlags),
672673
keepVars: true,
673674
keepSecrets: true,
674675
logpush: false,

‎packages/wrangler/src/pages/functions/buildWorker.ts

+77-77
Original file line numberDiff line numberDiff line change
@@ -78,83 +78,7 @@ export function buildWorkerFromFunctions({
7878
alias: {},
7979
doBindings: [], // Pages functions don't support internal Durable Objects
8080
external,
81-
plugins: [
82-
buildNotifierPlugin(onEnd),
83-
{
84-
name: "Assets",
85-
setup(pluginBuild) {
86-
const identifiers = new Map<string, string>();
87-
88-
pluginBuild.onResolve({ filter: /^assets:/ }, async (args) => {
89-
const directory = resolve(
90-
args.resolveDir,
91-
args.path.slice("assets:".length)
92-
);
93-
94-
const exists = await access(directory)
95-
.then(() => true)
96-
.catch(() => false);
97-
98-
const isDirectory =
99-
exists && (await lstat(directory)).isDirectory();
100-
101-
if (!isDirectory) {
102-
return {
103-
errors: [
104-
{
105-
text: `'${directory}' does not exist or is not a directory.`,
106-
},
107-
],
108-
};
109-
}
110-
111-
// TODO: Consider hashing the contents rather than using a unique identifier every time?
112-
identifiers.set(directory, nanoid());
113-
if (!buildOutputDirectory) {
114-
console.warn(
115-
"You're attempting to import static assets as part of your Pages Functions, but have not specified a directory in which to put them. You must use 'wrangler pages dev <directory>' rather than 'wrangler pages dev -- <command>' to import static assets in Functions."
116-
);
117-
}
118-
return { path: directory, namespace: "assets" };
119-
});
120-
121-
pluginBuild.onLoad(
122-
{ filter: /.*/, namespace: "assets" },
123-
async (args) => {
124-
const identifier = identifiers.get(args.path);
125-
126-
if (buildOutputDirectory) {
127-
const staticAssetsOutputDirectory = join(
128-
buildOutputDirectory,
129-
"cdn-cgi",
130-
"pages-plugins",
131-
identifier as string
132-
);
133-
await rm(staticAssetsOutputDirectory, {
134-
force: true,
135-
recursive: true,
136-
});
137-
await cp(args.path, staticAssetsOutputDirectory, {
138-
force: true,
139-
recursive: true,
140-
});
141-
142-
return {
143-
// TODO: Watch args.path for changes and re-copy when updated
144-
contents: `export const onRequest = ({ request, env, functionPath }) => {
145-
const url = new URL(request.url)
146-
const relativePathname = \`/\${url.pathname.replace(functionPath, "") || ""}\`.replace(/^\\/\\//, '/');
147-
url.pathname = '/cdn-cgi/pages-plugins/${identifier}' + relativePathname
148-
request = new Request(url.toString(), request)
149-
return env.ASSETS.fetch(request)
150-
}`,
151-
};
152-
}
153-
}
154-
);
155-
},
156-
},
157-
],
81+
plugins: [buildNotifierPlugin(onEnd), assetsPlugin(buildOutputDirectory)],
15882
isOutfile: !outdir,
15983
serveLegacyAssetsFromWorker: false,
16084
checkFetch: local,
@@ -420,3 +344,79 @@ function blockWorkerJsImports(nodejsCompatMode: NodeJSCompatMode): Plugin {
420344
},
421345
};
422346
}
347+
348+
function assetsPlugin(buildOutputDirectory: string | undefined): Plugin {
349+
return {
350+
name: "Assets",
351+
setup(pluginBuild) {
352+
const identifiers = new Map<string, string>();
353+
354+
pluginBuild.onResolve({ filter: /^assets:/ }, async (args) => {
355+
const directory = resolve(
356+
args.resolveDir,
357+
args.path.slice("assets:".length)
358+
);
359+
360+
const exists = await access(directory)
361+
.then(() => true)
362+
.catch(() => false);
363+
364+
const isDirectory = exists && (await lstat(directory)).isDirectory();
365+
366+
if (!isDirectory) {
367+
return {
368+
errors: [
369+
{
370+
text: `'${directory}' does not exist or is not a directory.`,
371+
},
372+
],
373+
};
374+
}
375+
376+
// TODO: Consider hashing the contents rather than using a unique identifier every time?
377+
identifiers.set(directory, nanoid());
378+
if (!buildOutputDirectory) {
379+
console.warn(
380+
"You're attempting to import static assets as part of your Pages Functions, but have not specified a directory in which to put them. You must use 'wrangler pages dev <directory>' rather than 'wrangler pages dev -- <command>' to import static assets in Functions."
381+
);
382+
}
383+
return { path: directory, namespace: "assets" };
384+
});
385+
386+
pluginBuild.onLoad(
387+
{ filter: /.*/, namespace: "assets" },
388+
async (args) => {
389+
const identifier = identifiers.get(args.path);
390+
391+
if (buildOutputDirectory) {
392+
const staticAssetsOutputDirectory = join(
393+
buildOutputDirectory,
394+
"cdn-cgi",
395+
"pages-plugins",
396+
identifier as string
397+
);
398+
await rm(staticAssetsOutputDirectory, {
399+
force: true,
400+
recursive: true,
401+
});
402+
await cp(args.path, staticAssetsOutputDirectory, {
403+
force: true,
404+
recursive: true,
405+
});
406+
407+
return {
408+
// TODO: Watch args.path for changes and re-copy when updated
409+
contents: `export const onRequest = ({ request, env, functionPath }) => {
410+
const url = new URL(request.url);
411+
const relativePathname = \`/\${url.pathname.replace(functionPath, "") || ""}\`.replace(/^\\/\\//, '/');
412+
url.pathname = '/cdn-cgi/pages-plugins/${identifier}' + relativePathname;
413+
request = new Request(url.toString(), request);
414+
return env.ASSETS.fetch(request);
415+
}`,
416+
};
417+
}
418+
}
419+
);
420+
},
421+
};
422+
}

‎pnpm-lock.yaml

+12
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

0 commit comments

Comments
 (0)
Please sign in to comment.