Cloudflare support: ws requires "crypto"

[bbigras]

Correct me if I'm wrong, but ws requires crypto, which doesn't work with Cloudflare, right?

Note: This module does not work in the browser. The client in the docs is a reference to a back end with the role of a client in the WebSocket communication. Browser clients must use the native WebSocket object. To make the same code work seamlessly on Node.js and the browser, you can use one of the many wrappers available on npm, like isomorphic-ws.

[bbigras]

cc @marbemac maybe

[jawj]

Yes. On Cloudflare you don't need ws and it won't work. In Node you need it and it will. Is Node perhaps used in the development environment here? (This is a reason it would be nice if we could arrange to import it only if necessary within the library).

[bbigras]

I'm using qwik with https://qwik.builder.io/integrations/deployments/cloudflare-pages/#cloudflare-pages-adaptor .

To use qwik without Cloudflare, I can just use the express (node) adaptor https://qwik.builder.io/integrations/deployments/node/ .

Do you think that if neondatabase/serverless would use isomorphic-ws instead of ws, that it would just work with nodejs and cloudflare?

[bbigras]

Note that even when I'm using the Cloudflare adaptor, when I'm developing locally, I might be using just nodejs.

[bbigras]

Any ideas?

[jawj]

The latest version of the driver (0.2.6) automatically tries to import ws if it doesn't manage to create a WebSocket by other means. That should mean that it still just works on Cloudflare, with no other dependencies; and that it now just works on Node, as long as ws is installed.

Are you finding that it's not working on one or both of those platforms?

[bbigras]

It works when I run my test app without cloudflare, but if I try to build for Cloudflare, I get:

[commonjs--resolver] Cannot bundle Node.js built-in "crypto" imported from "node_modules/ws/lib/sender.js". Consider disabling ssr.noExternal or remove the built-in dependency.
file: /home/bbigras/dev/test-qwik-neon/node_modules/ws/lib/sender.js
error during build:
RollupError: Cannot bundle Node.js built-in "crypto" imported from "node_modules/ws/lib/sender.js". Consider disabling ssr.noExternal or remove the built-in dependency.
    at error (file:///home/bbigras/dev/test-qwik-neon/node_modules/rollup/dist/es/shared/rollup.js:2091:30)
    at Object.error (file:///home/bbigras/dev/test-qwik-neon/node_modules/rollup/dist/es/shared/rollup.js:23700:20)
    at Object.resolveId (file:///home/bbigras/dev/test-qwik-neon/node_modules/vite/dist/node/chunks/dep-3007b26d.js:21500:34)
    at Object.handler (file:///home/bbigras/dev/test-qwik-neon/node_modules/vite/dist/node/chunks/dep-3007b26d.js:44852:19)
    at file:///home/bbigras/dev/test-qwik-neon/node_modules/rollup/dist/es/shared/rollup.js:23899:40
    at async PluginDriver.hookFirstAndGetPlugin (file:///home/bbigras/dev/test-qwik-neon/node_modules/rollup/dist/es/shared/rollup.js:23799:28)
    at async resolveId (file:///home/bbigras/dev/test-qwik-neon/node_modules/rollup/dist/es/shared/rollup.js:22743:26)
    at async ModuleLoader.resolveId (file:///home/bbigras/dev/test-qwik-neon/node_modules/rollup/dist/es/shared/rollup.js:23007:15)
    at async file:///home/bbigras/dev/test-qwik-neon/node_modules/vite/dist/node/chunks/dep-3007b26d.js:8169:16
    at async Promise.all (index 2)

Note that I have "ws" as dependency of @miniflare/http-server and @miniflare/web-sockets.

If I do rm -rf node_modules/ws and build again:

[vite]: Rollup failed to resolve import "ws" from "/home/bbigras/dev/test-qwik-neon/node_modules/@neondatabase/serverless/index.js".
This is most likely unintended because it can break your application at runtime.
If you do want to externalize this module explicitly add it to
`build.rollupOptions.external`
error during build:
Error: [vite]: Rollup failed to resolve import "ws" from "/home/bbigras/dev/test-qwik-neon/node_modules/@neondatabase/serverless/index.js".
This is most likely unintended because it can break your application at runtime.
If you do want to externalize this module explicitly add it to
`build.rollupOptions.external`
    at onRollupWarning (file:///home/bbigras/dev/test-qwik-neon/node_modules/vite/dist/node/chunks/dep-3007b26d.js:44772:19)
    at onwarn (file:///home/bbigras/dev/test-qwik-neon/node_modules/vite/dist/node/chunks/dep-3007b26d.js:44542:13)
    at Object.onwarn (file:///home/bbigras/dev/test-qwik-neon/node_modules/rollup/dist/es/shared/rollup.js:24651:13)
    at ModuleLoader.handleInvalidResolvedId (file:///home/bbigras/dev/test-qwik-neon/node_modules/rollup/dist/es/shared/rollup.js:23335:26)
    at ModuleLoader.resolveDynamicImport (file:///home/bbigras/dev/test-qwik-neon/node_modules/rollup/dist/es/shared/rollup.js:23391:58)
    at async file:///home/bbigras/dev/test-qwik-neon/node_modules/rollup/dist/es/shared/rollup.js:23280:32

my-qwik-basic-starter@ /home/bbigras/dev/test-qwik-neon
├── @builder.io/qwik-city@0.1.1
├── @builder.io/qwik@0.18.0
├── @neondatabase/serverless@0.2.6
├── @types/eslint@8.4.10
├── @types/node@18.14.6
├── @typescript-eslint/eslint-plugin@5.45.0
├── @typescript-eslint/parser@5.45.0
├── eslint-plugin-qwik@0.15.2
├── eslint@8.28.0
├── net@1.0.2
├── node-fetch@3.3.0
├── prettier@2.8.0
├── stream@0.0.2
├── tls@0.0.1
├── typescript@4.9.5
├── vite-plugin-top-level-await@1.2.4
├── vite-plugin-wasm@3.2.1
├── vite-tsconfig-paths@4.0.5
├── vite@4.1.1
└── wrangler@2.12.1

I can provide a repo to test.

[jawj]

OK — a repo to test would be helpful. But it should be fine to do what's suggested in the error: just 'externalise' ws by adding it to build.rollupOptions.external.

We don't use ws if it's not installed, and sometimes I wish bundling software would just mind its own business ...

[jawj]

In the latest release of the driver I've changed the ws import to be dynamic, i.e. import(String.fromCharCode(119) + String.fromCharCode(115)) instead of import("ws"), because this prevents complaints from Next.js/Vercel.

It would be interesting to know how this affects the issue you reported here. If it's a still a problem, then a repo to test would be helpful.

[bbigras]

I think it worked even before 5de999c02c4889ecf89f410c499850fac579e779 . I didn't test on real Cloudflare, but it seemed to work on my side.

I'll test both commits.

[jawj]

@bbigras Recent versions of the driver have stopped trying to import ws again, because it caused too much trouble with bundlers. Is the current driver version (0.4.9) working for you, or are there still problems here?