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

feat: second-cut of using unenv to create a hybrid node.js compatibility setting (via inject) #5878

Merged
merged 29 commits into from
Jun 6, 2024
Merged
Show file tree
Hide file tree
Changes from 26 commits
Commits
Show all changes
29 commits
Select commit Hold shift + click to select a range
61cbf9d
feat: first-cut of using unenv to create a hybrid node.js compatibili…
petebacondarwin Mar 11, 2024
617a949
fix up pg cloudflare socket detection
petebacondarwin Mar 12, 2024
6e425c2
fix up unenv integration, use inject instead of various hacks
IgorMinar May 21, 2024
a4f1e21
make the side-effects of injected globals tree-shakeable
IgorMinar May 21, 2024
3618331
mark unenv as external so that it doesn't get bundled into wrangler
IgorMinar May 22, 2024
2437267
don't configure esbuild with a define to rewrite global to globalThis…
IgorMinar May 22, 2024
8c640b8
correctly provide the global polyfill if inject is defined as a string
IgorMinar May 22, 2024
c45a91b
fix lint issues
IgorMinar May 22, 2024
8b62219
add turbo.json file
IgorMinar May 22, 2024
b47cf6c
update the activation mechanism to be just a runtime flag 'nodejs_com…
IgorMinar Jun 4, 2024
dfbf85f
update to unenv-nightly@1.10.0-1717495514.cea841e
IgorMinar Jun 4, 2024
d064991
switch to experimenta:nodejs_compat_v2 activation flag
IgorMinar Jun 4, 2024
7f09dba
fix: ignore aliases for packages that are not installed in the curren…
IgorMinar Jun 5, 2024
5d00d69
prettify
IgorMinar Jun 5, 2024
961d1a2
fixup: update unenv to 1.10.0-1717606461.a117952
IgorMinar Jun 5, 2024
1e9b4a6
post-review fixups
petebacondarwin Jun 5, 2024
461e1ae
Add changeset
petebacondarwin Jun 5, 2024
ae685f9
Update the fixture and add a test
petebacondarwin Jun 5, 2024
55f3f14
Separate out the require call conversion from the alias import handli…
petebacondarwin Jun 5, 2024
3d84d9d
Make sure we don't match packages that don't exist
petebacondarwin Jun 5, 2024
3c854aa
Fix formatting on the new fixture test
petebacondarwin Jun 5, 2024
564311d
Fix deploy test snapshot for change to warning
petebacondarwin Jun 5, 2024
7a4c8f4
Fix fixture test to be resilient to timezone changes
petebacondarwin Jun 5, 2024
d3022e0
update the fixture to include an built-in alias
IgorMinar Jun 6, 2024
6de7e89
Match externals against the alias rather than the original path
petebacondarwin Jun 6, 2024
12b2b8e
Refactor node-compat validation and handling into a helper function
petebacondarwin Jun 6, 2024
b8493ef
Remove extra node-compat validation call, which was messing things up…
petebacondarwin Jun 6, 2024
0ec2e86
Use a global (Buffer) in the fixture and test it works
petebacondarwin Jun 6, 2024
8639e8b
Use named parameters for the validateNodeCompat() function
petebacondarwin Jun 6, 2024
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
11 changes: 11 additions & 0 deletions .changeset/calm-ducks-happen.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
---
"wrangler": minor
---

feat: add experimental support for hybrid Node.js compatibility

_This feature is experimental and not yet available for general consumption._

Use a combination of workerd Node.js builtins (behind the `experimental:nodejs_compat_v2` flag) and
Unenv polyfills (configured to only add those missing from the runtime) to provide a new more effective
Node.js compatibility approach.
19 changes: 19 additions & 0 deletions fixtures/nodejs-hybrid-app/package.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,19 @@
{
"name": "nodejs-hybrid-app",
"private": true,
"scripts": {
"dev": "wrangler dev",
"build": "wrangler deploy --dry-run --outdir=./dist",
"test:ci": "vitest run",
"test:watch": "vitest"
},
"devDependencies": {
"@cloudflare/workers-tsconfig": "workspace:*",
"@cloudflare/workers-types": "^4.20240222.0",
"@types/pg": "^8.11.2",
"pg": "8.11.3",
"pg-cloudflare": "^1.1.1",
"undici": "^5.28.3",
"wrangler": "workspace:*"
}
}
44 changes: 44 additions & 0 deletions fixtures/nodejs-hybrid-app/src/index.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,44 @@
// node:assert/strict is currently an unenv alias to node:assert
// this is not very common, but happens and we need to support it
import assert from "node:assert/strict";
import { Client } from "pg";

assert(true, "the world is broken");

/**
* Welcome to Cloudflare Workers! This is your first worker.
*
* - Run `npm run dev` in your terminal to start a development server
* - Open a browser tab at http://localhost:8787/ to see your worker in action
* - Run `npm run deploy` to publish your worker
*
* Learn more at https://developers.cloudflare.com/workers/
*/
petebacondarwin marked this conversation as resolved.
Show resolved Hide resolved

export default {
async fetch(
request: Request,
env: Env,
ctx: ExecutionContext
): Promise<Response> {
const client = new Client({
user: env.DB_USERNAME,
password: env.DB_PASSWORD,
host: env.DB_HOSTNAME,
port: Number(env.DB_PORT),
database: env.DB_NAME,
});
await client.connect();
const result = await client.query(`SELECT * FROM rnc_database`);
assert(true);

// Return the first row as JSON
const resp = new Response(JSON.stringify(result.rows[0]), {
headers: { "Content-Type": "application/json" },
});

// Clean up the client
ctx.waitUntil(client.end());
return resp;
},
};
22 changes: 22 additions & 0 deletions fixtures/nodejs-hybrid-app/tests/index.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,22 @@
import { resolve } from "node:path";
import { fetch } from "undici";
import { describe, it } from "vitest";
import { runWranglerDev } from "../../shared/src/run-wrangler-long-lived";

describe("nodejs compat", () => {
it("should work when running code requiring polyfills", async ({
expect,
}) => {
const { ip, port, stop } = await runWranglerDev(
resolve(__dirname, "../src"),
["--port=0", "--inspector-port=0"]
);
try {
const response = await fetch(`http://${ip}:${port}`);
const result = (await response.json()) as { id: string };
expect(result.id).toEqual("1");
} finally {
await stop();
}
});
});
7 changes: 7 additions & 0 deletions fixtures/nodejs-hybrid-app/tests/tsconfig.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
{
"extends": "@cloudflare/workers-tsconfig/tsconfig.json",
"compilerOptions": {
"types": ["node"]
},
"include": ["**/*.ts", "../../../node-types.d.ts"]
}
17 changes: 17 additions & 0 deletions fixtures/nodejs-hybrid-app/tsconfig.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
{
"compilerOptions": {
"module": "esnext",
"target": "esnext",
"lib": ["esnext"],
"strict": true,
"isolatedModules": true,
"noEmit": true,
"types": [
"@cloudflare/workers-types/experimental",
"./worker-configuration.d.ts"
],
"allowJs": true,
"allowSyntheticDefaultImports": true
},
"include": ["src"]
}
9 changes: 9 additions & 0 deletions fixtures/nodejs-hybrid-app/turbo.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
{
"$schema": "http://turbo.build/schema.json",
"extends": ["//"],
"pipeline": {
"build": {
"outputs": ["dist/**"]
}
}
}
9 changes: 9 additions & 0 deletions fixtures/nodejs-hybrid-app/vitest.config.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
import { defineProject, mergeConfig } from "vitest/config";
import configShared from "../../vitest.shared";

export default mergeConfig(
configShared,
defineProject({
test: {},
})
);
10 changes: 10 additions & 0 deletions fixtures/nodejs-hybrid-app/worker-configuration.d.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
// Generated by Wrangler on Tue Mar 05 2024 16:04:07 GMT+0000 (Greenwich Mean Time)
// by running `wrangler types`

interface Env {
DB_HOSTNAME: "hh-pgsql-public.ebi.ac.uk";
DB_PORT: "5432";
DB_NAME: "pfmegrnargs";
DB_USERNAME: "reader";
DB_PASSWORD: "NWDMCE5xdipIjRrp";
}
11 changes: 11 additions & 0 deletions fixtures/nodejs-hybrid-app/wrangler.toml
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
name = "nodejs-hybrid-app"
main = "src/index.ts"
compatibility_date = "2024-06-03"
compatibility_flags = ["experimental:nodejs_compat_v2"]

[vars]
DB_HOSTNAME = "hh-pgsql-public.ebi.ac.uk"
DB_PORT = "5432"
DB_NAME = "pfmegrnargs"
DB_USERNAME = "reader"
DB_PASSWORD = "NWDMCE5xdipIjRrp"
petebacondarwin marked this conversation as resolved.
Show resolved Hide resolved
3 changes: 2 additions & 1 deletion package.json
Original file line number Diff line number Diff line change
Expand Up @@ -78,7 +78,8 @@
"ink@3.2.0": "patches/ink@3.2.0.patch",
"toucan-js@3.2.2": "patches/toucan-js@3.2.2.patch",
"@cloudflare/component-listbox@1.10.6": "patches/@cloudflare__component-listbox@1.10.6.patch",
"capnp-ts@0.7.0": "patches/capnp-ts@0.7.0.patch"
"capnp-ts@0.7.0": "patches/capnp-ts@0.7.0.patch",
"pg@8.11.3": "patches/pg@8.11.3.patch"
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

TODO: remove this once it is fixed upstream

}
}
}
1 change: 1 addition & 0 deletions packages/wrangler/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -82,6 +82,7 @@
"resolve.exports": "^2.0.2",
"selfsigned": "^2.0.1",
"source-map": "0.6.1",
"unenv": "npm:unenv-nightly@1.10.0-1717606461.a117952",
petebacondarwin marked this conversation as resolved.
Show resolved Hide resolved
"xxhash-wasm": "^1.0.1"
},
"devDependencies": {
Expand Down
3 changes: 3 additions & 0 deletions packages/wrangler/scripts/deps.ts
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,9 @@ export const EXTERNAL_DEPENDENCIES = [
// @cloudflare/workers-types is an optional peer dependency of wrangler, so users can
// get the types by installing the package (to what version they prefer) themselves
"@cloudflare/workers-types",
// unenv must be external because it contains unenv/runtime code which needs to be resolved
// and read when we are bundling the worker application
"unenv",
];

const pathToPackageJson = path.resolve(__dirname, "..", "package.json");
Expand Down
62 changes: 31 additions & 31 deletions packages/wrangler/src/__tests__/deploy.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -8530,17 +8530,17 @@ export default{
writeWorkerSource();
await runWrangler("deploy index.js --node-compat --dry-run");
expect(std).toMatchInlineSnapshot(`
Object {
"debug": "",
"err": "",
"info": "",
"out": "Total Upload: xx KiB / gzip: xx KiB
--dry-run: exiting now.",
"warn": "▲ [WARNING] Enabling Node.js compatibility mode for built-ins and globals. This is experimental and has serious tradeoffs. Please see https://github.com/ionic-team/rollup-plugin-node-polyfills/ for more details.

",
}
`);
Object {
"debug": "",
"err": "",
"info": "",
"out": "Total Upload: xx KiB / gzip: xx KiB
--dry-run: exiting now.",
"warn": "▲ [WARNING] Enabling Wrangler compile-time Node.js compatibility polyfill mode for builtins and globals. This is experimental and has serious tradeoffs.

",
}
`);
});

it("should recommend node compatibility mode when using node builtins and node-compat isn't enabled", async () => {
Expand Down Expand Up @@ -8578,17 +8578,17 @@ export default{
);
await runWrangler("deploy index.js --node-compat --dry-run"); // this would throw if node compatibility didn't exist
expect(std).toMatchInlineSnapshot(`
Object {
"debug": "",
"err": "",
"info": "",
"out": "Total Upload: xx KiB / gzip: xx KiB
--dry-run: exiting now.",
"warn": "▲ [WARNING] Enabling Node.js compatibility mode for built-ins and globals. This is experimental and has serious tradeoffs. Please see https://github.com/ionic-team/rollup-plugin-node-polyfills/ for more details.

",
}
`);
Object {
"debug": "",
"err": "",
"info": "",
"out": "Total Upload: xx KiB / gzip: xx KiB
--dry-run: exiting now.",
"warn": "▲ [WARNING] Enabling Wrangler compile-time Node.js compatibility polyfill mode for builtins and globals. This is experimental and has serious tradeoffs.

",
}
`);
});
});

Expand Down Expand Up @@ -8663,7 +8663,7 @@ export default{
"deploy index.js --dry-run --outdir=dist --compatibility-flag=nodejs_compat --node-compat"
)
).rejects.toThrowErrorMatchingInlineSnapshot(
`[AssertionError: The \`nodejs_compat\` compatibility flag cannot be used in conjunction with the legacy \`--node-compat\` flag. If you want to use the Workers runtime Node.js compatibility features, please remove the \`--node-compat\` argument from your CLI command or \`node_compat = true\` from your config file.]`
`[Error: The \`nodejs_compat\` compatibility flag cannot be used in conjunction with the legacy \`--node-compat\` flag. If you want to use the Workers \`nodejs_compat\` compatibility flag, please remove the \`--node-compat\` argument from your CLI command or \`node_compat = true\` from your config file.]`
);
});
});
Expand Down Expand Up @@ -9098,13 +9098,13 @@ export default{
"deploy index.js --no-bundle --node-compat --dry-run --outdir dist"
);
expect(std.warn).toMatchInlineSnapshot(`
"▲ [WARNING] Enabling Node.js compatibility mode for built-ins and globals. This is experimental and has serious tradeoffs. Please see https://github.com/ionic-team/rollup-plugin-node-polyfills/ for more details.
"▲ [WARNING] Enabling Wrangler compile-time Node.js compatibility polyfill mode for builtins and globals. This is experimental and has serious tradeoffs.


▲ [WARNING] \`--node-compat\` and \`--no-bundle\` can't be used together. If you want to polyfill Node.js built-ins and disable Wrangler's bundling, please polyfill as part of your own bundling process.
▲ [WARNING] \`--node-compat\` and \`--no-bundle\` can't be used together. If you want to polyfill Node.js built-ins and disable Wrangler's bundling, please polyfill as part of your own bundling process.

"
`);
"
`);
});

it("should warn that no-bundle and node-compat can't be used together", async () => {
Expand All @@ -9118,13 +9118,13 @@ export default{
fs.writeFileSync("index.js", scriptContent);
await runWrangler("deploy index.js --dry-run --outdir dist");
expect(std.warn).toMatchInlineSnapshot(`
"▲ [WARNING] Enabling Node.js compatibility mode for built-ins and globals. This is experimental and has serious tradeoffs. Please see https://github.com/ionic-team/rollup-plugin-node-polyfills/ for more details.
"▲ [WARNING] Enabling Wrangler compile-time Node.js compatibility polyfill mode for builtins and globals. This is experimental and has serious tradeoffs.


▲ [WARNING] \`--node-compat\` and \`--no-bundle\` can't be used together. If you want to polyfill Node.js built-ins and disable Wrangler's bundling, please polyfill as part of your own bundling process.
▲ [WARNING] \`--node-compat\` and \`--no-bundle\` can't be used together. If you want to polyfill Node.js built-ins and disable Wrangler's bundling, please polyfill as part of your own bundling process.

"
`);
"
`);
});
});

Expand Down
14 changes: 7 additions & 7 deletions packages/wrangler/src/__tests__/pages/functions-build.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -432,15 +432,15 @@ export default {
await runWrangler(`pages functions build --outfile=public/_worker.bundle`)
);
expect(std.warn).toMatchInlineSnapshot(`
"▲ [WARNING] The package \\"node:async_hooks\\" wasn't found on the file system but is built into node.
"▲ [WARNING] The package \\"node:async_hooks\\" wasn't found on the file system but is built into node.

Your Worker may throw errors at runtime unless you enable the \\"nodejs_compat\\" compatibility flag.
Refer to https://developers.cloudflare.com/workers/runtime-apis/nodejs/ for more details. Imported
from:
- hello.js
Your Worker may throw errors at runtime unless you enable the \\"nodejs_compat\\" compatibility flag.
Refer to https://developers.cloudflare.com/workers/runtime-apis/nodejs/ for more details. Imported
from:
- hello.js

"
`);
"
`);
});

it("should compile a _worker.js/ directory", async () => {
Expand Down
Loading
Loading