Skip to content

Commit

Permalink
feat: second-cut of using unenv to create a hybrid node.js compatibil…
Browse files Browse the repository at this point in the history
…ity setting (via inject) (#5878)

* feat: first-cut of using unenv to create a hybrid node.js compatibility setting

* fix up pg cloudflare socket detection

* fix up unenv integration, use inject instead of various hacks

* make the side-effects of injected globals tree-shakeable

* mark unenv as external so that it doesn't get bundled into wrangler

* don't configure esbuild with a define to rewrite global to globalThis - use unenv instead

* correctly provide the global polyfill if inject is defined as a string

* fix lint issues

* add turbo.json file

* update the activation mechanism to be just a runtime flag 'nodejs_compat_v2' accompanied by 'experimental'

* update to unenv-nightly@1.10.0-1717495514.cea841e

* switch to experimenta:nodejs_compat_v2 activation flag

* fix: ignore aliases for packages that are not installed in the current app

for example consola/core

* prettify

* fixup: update unenv to 1.10.0-1717606461.a117952

* post-review fixups

* Add changeset

* Update the fixture and add a test

* Separate out the require call conversion from the alias import handling in the hybrid plugin

* Make sure we don't match packages that don't exist

* Fix formatting on the new fixture test

* Fix deploy test snapshot for change to warning

* Fix fixture test to be resilient to timezone changes

The previous test would pass/fail depending upon which timezone you are in when you run it.

* update the fixture to include an built-in alias

currently this fails with:

nodejs-hybrid-app:build: ✘ [ERROR] Plugin "unenv-cloudflare" returned a non-absolute path: node:assert (set a namespace if this is not a file path)
nodejs-hybrid-app:build:
nodejs-hybrid-app:build:     src/index.ts:2:19:
nodejs-hybrid-app:build:       2 │ import assert from "node:assert/strict";
nodejs-hybrid-app:build:         ╵                    ~~~~~~~~~~~~~~~~~~~~
nodejs-hybrid-app:build:

* Match externals against the alias rather than the original path

This was problematic because if you import from `node:assert/strict` there is no entry in the `external` map for that path, instead you need to map it to its alias first (e.g. `node:assert`).

* Refactor node-compat validation and handling into a helper function

* Remove extra node-compat validation call, which was messing things up since it mutates compat flags

* Use a global (Buffer) in the fixture and test it works

* Use named parameters for the validateNodeCompat() function

---------

Co-authored-by: Peter Bacon Darwin <pbacondarwin@cloudflare.com>
  • Loading branch information
IgorMinar and petebacondarwin authored Jun 6, 2024
1 parent 35b1a2f commit 1e68fe5
Show file tree
Hide file tree
Showing 34 changed files with 1,140 additions and 441 deletions.
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:*"
}
}
53 changes: 53 additions & 0 deletions fixtures/nodejs-hybrid-app/src/index.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,53 @@
// 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");

const buffer1 = Buffer.of(1);
assert(buffer1.toJSON().data[0] === 1, "Buffer is broken");

const buffer2 = global.Buffer.of(1);
assert(buffer2.toJSON().data[0] === 1, "global.Buffer is broken");

const buffer3 = globalThis.Buffer.of(1);
assert(buffer3.toJSON().data[0] === 1, "globalThis.Buffer 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/
*/

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"
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"
}
}
}
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",
"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": "[33m▲ [43;33m[[43;30mWARNING[43;33m][0m [1mEnabling 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.[0m
",
}
`);
Object {
"debug": "",
"err": "",
"info": "",
"out": "Total Upload: xx KiB / gzip: xx KiB
--dry-run: exiting now.",
"warn": "[33m▲ [43;33m[[43;30mWARNING[43;33m][0m [1mEnabling Wrangler compile-time Node.js compatibility polyfill mode for builtins and globals. This is experimental and has serious tradeoffs.[0m
",
}
`);
});

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": "[33m▲ [43;33m[[43;30mWARNING[43;33m][0m [1mEnabling 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.[0m
",
}
`);
Object {
"debug": "",
"err": "",
"info": "",
"out": "Total Upload: xx KiB / gzip: xx KiB
--dry-run: exiting now.",
"warn": "[33m▲ [43;33m[[43;30mWARNING[43;33m][0m [1mEnabling Wrangler compile-time Node.js compatibility polyfill mode for builtins and globals. This is experimental and has serious tradeoffs.[0m
",
}
`);
});
});

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(`
"[33m▲ [43;33m[[43;30mWARNING[43;33m][0m [1mEnabling 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.[0m
"[33m▲ [43;33m[[43;30mWARNING[43;33m][0m [1mEnabling Wrangler compile-time Node.js compatibility polyfill mode for builtins and globals. This is experimental and has serious tradeoffs.[0m
[33m▲ [43;33m[[43;30mWARNING[43;33m][0m [1m\`--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.[0m
[33m▲ [43;33m[[43;30mWARNING[43;33m][0m [1m\`--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.[0m
"
`);
"
`);
});

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(`
"[33m▲ [43;33m[[43;30mWARNING[43;33m][0m [1mEnabling 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.[0m
"[33m▲ [43;33m[[43;30mWARNING[43;33m][0m [1mEnabling Wrangler compile-time Node.js compatibility polyfill mode for builtins and globals. This is experimental and has serious tradeoffs.[0m
[33m▲ [43;33m[[43;30mWARNING[43;33m][0m [1m\`--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.[0m
[33m▲ [43;33m[[43;30mWARNING[43;33m][0m [1m\`--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.[0m
"
`);
"
`);
});
});

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(`
"[33m▲ [43;33m[[43;30mWARNING[43;33m][0m [1mThe package \\"node:async_hooks\\" wasn't found on the file system but is built into node.[0m
"[33m▲ [43;33m[[43;30mWARNING[43;33m][0m [1mThe package \\"node:async_hooks\\" wasn't found on the file system but is built into node.[0m
Your Worker may throw errors at runtime unless you enable the \\"nodejs_compat\\" compatibility flag.
Refer to [4mhttps://developers.cloudflare.com/workers/runtime-apis/nodejs/[0m for more details. Imported
from:
- hello.js
Your Worker may throw errors at runtime unless you enable the \\"nodejs_compat\\" compatibility flag.
Refer to [4mhttps://developers.cloudflare.com/workers/runtime-apis/nodejs/[0m for more details. Imported
from:
- hello.js
"
`);
"
`);
});

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

0 comments on commit 1e68fe5

Please sign in to comment.