🐛 BUG: vitest-pool-workers node compatibility with the jose and prisma libraries

vladinator1000 commented 6 months ago

Which Cloudflare product(s) does this pertain to?

Wrangler core

What version(s) of the tool(s) are you using?

3.30.1

What version of Node are you using?

v21.6.1

What operating system and version are you using?

Windows 11

Describe the Bug

Observed behavior

When I run the tests in my repro

npm run test

I observe this error

 src/jwt.test.ts [ src/jwt.test.ts ]
TypeError: deprecate is not a function
 ❯ node_modules/jose/dist/node/esm/runtime/rsaes.js:23:38

The jose library actually works in the production runtime of Cloudflare workers, without the nodejs_compat flag, but vitest-pool-workers paradoxically requires the flag.

Expected behavior

The tests work.

Steps to reproduce

Clone https://github.com/vladinator1000/miniflare-node-compat-bug

Run

npm i

npm run test

Please provide a link to a minimal reproduction

https://github.com/vladinator1000/miniflare-node-compat-bug

Please provide any relevant error logs

 src/jwt.test.ts [ src/jwt.test.ts ]
TypeError: deprecate is not a function
 ❯ node_modules/jose/dist/node/esm/runtime/rsaes.js:23:38

I also found a similar issue when using Prisma: https://github.com/prisma/prisma/issues/23193#issuecomment-1971658732

mrbbot commented 6 months ago

Hey! 👋 vitest-pool-workers isn't quite ready to be released, but thanks for trying it out and raising this! It looks like this issue should be fixed by some in-progress work to make module resolution in the pool behave more like wrangler. Specifically, we now use the same resolve conditions and respect the package.json browser field. Running your repro on my WIP branch seems to work fine. 🎉 I'll post an update here once there's a new pre-release to try. 🙂

vladinator1000 commented 6 months ago

Great work @mrbbot! I'm excited to try it once it's out 💓

mrbbot commented 6 months ago

Hey! 👋 The new pre-release should be ready: npm install -D https://prerelease-registry.devprod.cloudflare.dev/workers-sdk/runs/8161335149/npm-package-cloudflare-vitest-pool-workers-5070 (https://github.com/cloudflare/workers-sdk/pull/5070). The big change is the replacement of defineWorkersPoolOptions() with defineWorkersConfig(). This new function replaces vitest's default defineConfig() function. This makes sure the required Vite resolution config is set, and defaults pool to @cloudflare/workers-pool-workers. A basic configuration now looks like...

// vitest.config.ts
import { defineWorkersConfig } from "@cloudflare/vitest-pool-workers/config";

export default defineWorkersConfig({
  test: {
    poolOptions: {
      workers: {
        isolatedStorage: true,
        wrangler: {
          configPath: "./wrangler.toml"
        }
      },
    },
  },
});

https://github.com/mrbbot/vitest-pool-workers-prerelease-getting-started has been updated with the new pre-release. You can also find a bunch of examples here: https://github.com/cloudflare/workers-sdk/tree/bcoll/vitest-pool-workers-examples/fixtures/vitest-pool-workers-examples.

vladinator1000 commented 6 months ago

@mrbbot the new release fixed the TypeError: deprecate is not a function 🎉 when importing the jose library!

I'm still seeing this Prisma import failure error however:

 workerd/server/server.c++:2676: error: Fallback service failed to fetch module; payload = ; spec = /?specifier=%2Fhome%2Frunner%2Fwork%2Fprisma-vitest-miniflare%2Fprisma-vitest-miniflare%2Fnode_modules%2F%40prisma%2Fclient%2Fdefault.js&referrer=%2Fhome%2Frunner%2Fwork%2Fprisma-vitest-miniflare%2Fprisma-vitest-miniflare%2Fnode_modules%2Fvite-node%2Fdist%2Fclient.mjs
 ❯ src/staticImport.test.ts  (0 test)

⎯⎯⎯⎯⎯⎯ Failed Suites 1 ⎯⎯⎯⎯⎯⎯⎯

 FAIL  src/staticImport.test.ts [ src/staticImport.test.ts ]
Error: No such module "home/runner/work/prisma-vitest-miniflare/prisma-vitest-miniflare/node_modules/@prisma/client/default.js".
 ❯ src/prismaClient.ts:1:154
      1| import { PrismaClient } from '@prisma/client'

I reproduced it here and added a CI workflow for easy testing: https://github.com/vladinator1000/prisma-vitest-miniflare

Importing something from @prisma/client makes it fail https://github.com/vladinator1000/prisma-vitest-miniflare/blob/main/src/staticImport.test.ts

Looking at the files, they just do some re-exports

// @prisma/client
export * from '.prisma/client/default'

// .prisma/client/default
export * from './index'

I double-checked that I'm running prisma generate before the test. The files get imported without issue in vitest-environment-miniflare.

mrbbot commented 6 months ago

Hey! 👋 Did some more digging into why Prisma doesn't work, and uncovered a whole bunch of issues. 😅 Thanks for testing this early!

Issue 1: `@prisma/client`'s `package.json` `exports` are interpreted incorrectly

@prisma/client's package.json looks something like this...

{
  "name": "@prisma/client",
  "version": "5.11.0-dev.5",
  "main": "default.js",
  "browser": "index-browser.js",
  "types": "default.d.ts",
  "exports": {
    ".": {
      "require": {
        "types": "./default.d.ts",
        "node": "./default.js",
        "edge-light": "./wasm.js",
        "workerd": "./wasm.js",
        "worker": "./wasm.js",
        "browser": "./index-browser.js"
      },
      "import": {
        "types": "./default.d.ts",
        "node": "./default.js",
        "edge-light": "./wasm.js",
        "workerd": "./wasm.js",
        "worker": "./wasm.js",
        "browser": "./index-browser.js"
      },
      "default": "./default.js"
    },
    "./edge": { ... },
    "./extension": { ...  },
    "./index-browser": { ... },
    "./index": { ... },
    "./wasm": { ... },
    "./runtime/library": { ... },
    "./runtime/binary": { ... },
    "./generator-build": { ... },
    "./*": "./*"
  },
}

@cloudflare/vitest-pool-workers uses Vite's resolution algorithm for resolving module paths, using the same "conditions" of Wrangler (workerd, worker, browser). Vitest additionally always adds the node condition. Internally, Vite uses the resolve.exports package to find the correct module path given package.json exports, a specifier, and a set of conditions. The call ends up looking something like this, where pkg is the contents of @prisma/client's package.json above:

import { exports } from "resolve.exports";
const target = ".";
const opts = {
  "browser": false,
  "require": false,
  "conditions": ["development", "module", "workerd", "worker", "browser", "node" ],
};
const result = exports(pkg, target, opts); // "./default.js"

This gives ./default.js (not ./wasm.js) since resolve.exports correctly checks conditions in the order they appear in package.json, and Vitest added the node condition. Even removing the node condition gives us the same result, as resolve.exports will automatically add it back when "browser": false is set. Vite only sets "browser": true if the node condition isn't set (😅) and we're targeting a Web-like environment (which we are).

We can solve this by removing the default node condition with a custom Vite plugin. 👍

Issue 2: unable to resolve `.prisma/client/wasm` specifier from inside `@prsima/client`

Modules starting with . aren't treated as node_modules by Vite, meaning they don't go through Node-like resolution. Node will throw an ERR_INVALID_MODULE_SPECIFIER when trying to import a module starting with a . like this, so it feels like something's that's not meant to be supported.

I guess this isn't a problem with wrangler dev because esbuild's resolver is more lax. We can solve this by falling back to Node's require.resolve() in the case where the specifier starts with a . like this. 👍

Issue 3: `.wasm` files aren't supported if imported from `node_modules`

@cloudflare/vitest-pool-workers's support for module rules currently only works if files are imported from Vite-transformed files. This is to ensure we respect the correct project config when matching file paths against rules. Unfortunately, Prisma imports a .wasm file from one of the client dependencies.

Usually this isn't a problem with wrangler dev because we apply modules rules to all files. I think we could solve this by just treating .wasm files as WebAssembly modules all the time, regardless of configured rules. It seems unlikely someone would want to do something different with a .wasm file inside node_modules. 👍

Issue 4: `@prisma/adapter-pg` tries to use something on an `import *` that doesn't exist

@prisma/adapter-pg includes the lines import * as pg2 from "pg"; and if (!(client instanceof pg2.Pool)) { ... }. Unfortunately, pg2.Pool is undefined giving TypeError: Right-hand side of 'instanceof' is not an object. pg's entrypoint is a CommonJS module, meaning we rely on cjs-module-lexer like Node.js to find named exports for ES module import. pg's entrypoint contains something like...

module.exports = new PG(...)

...where instances of PG have a Pool property. Unfortunately, this can't be detected statically with cjs-module-lexer, meaning pg2 above just has a default property. Replacing the instanceof pg2.Pool with instanceof pg2.default.Pool fixes things.

I guess this isn't a problem with wrangler dev because esbuild's runtime helpers are more lax with accessing named exports at runtime. I think we'll need to fix this with a PR to @prisma/adapter-pg. 📮

Issue 5: `pg-cloudflare` exports an ES module, but `pg` loads it with a `require()`

require()ing ES modules isn't allowed. In this case node_modules/pg-cloudflare/dist/index.js is an ES module, but node_modules/pg/lib/stream.js includes require('pg-cloudflare'). I guess this isn't a problem with wrangler dev because esbuild nicely converts between the two. I think we'll need to fix this with a PR to pg-cloudflare. 📮

Issue 6: `pg` relies on the `node:dns` module existing

Even though it doesn't require any methods from it when used in a Cloudflare Worker, node_modules/pg/lib/connection-parameters.js includes require("dns"). I guess this isn't a problem with wrangler dev because node_compat is required which polyfills this. We should be able to solve this by adding an empty polyfill for node:dns to @cloudflare/vitest-pool-workers like we do for a few of the other node:* modules to support Vitest. 👍

I'll make the required changes and put up PRs to fix these. 🙂 Might not all be ready in time for the launch tough... 😬

mrbbot commented 6 months ago

https://github.com/cloudflare/workers-sdk/pull/5070 includes fixes for Issues 1, 2, 3 and 6. It looks like Issue 4 may be fixed by https://github.com/prisma/prisma/pull/23399. I've opened a PR to node-postgres to fix Issue 5: https://github.com/brianc/node-postgres/pull/3168. Re-opening until those upstream issues are resolved. 👍

vladinator1000 commented 6 months ago

Woah @mrbbot, that's some great detective work! I'll keep updating my test repo when there are new developments 🙌🏼

vladinator1000 commented 5 months ago

Hi @mrbbot, I gave this another whirl now that Prisma 5.12.1 officially supports D1. I found a new error. Looks like it relates to wasm resolution. It happens after you start querying the client:

workerd/server/server.c++:2802: error: Fallback service failed to fetch module; payload = ; spec = /?specifier=%2Fhome%2Frunner%2Fwork%2Fprisma-vitest-miniflare%2Fprisma-vitest-miniflare%2Fnode_modules%2F.prisma%2Fclient%2Fwasm-worker-loader.js&referrer=%2Fhome%2Frunner%2Fwork%2Fprisma-vitest-miniflare%2Fprisma-vitest-miniflare%2Fnode_modules%2F.prisma%2Fclient%2Fwasm.js%3Fmf_vitest_no_cjs_esm_shim
 ❯ src/prismaClient.test.ts  (1 test | 1 failed) 14ms
   ❯ src/prismaClient.test.ts > prismaClient > can run basic query
     → No such module "home/runner/work/prisma-vitest-miniflare/prisma-vitest-miniflare/node_modules/.prisma/client/#wasm-engine-loader".
⎯⎯⎯⎯⎯⎯⎯ Failed Tests 1 ⎯⎯⎯⎯⎯⎯⎯
 FAIL  src/prismaClient.test.ts > prismaClient > can run basic query
Error: No such module "home/runner/work/prisma-vitest-miniflare/prisma-vitest-miniflare/node_modules/.prisma/client/#wasm-engine-loader".
 ❯ Object.getQueryEngineWasmModule home/runner/work/prisma-vitest-miniflare/prisma-vitest-miniflare/node_modules/.prisma/client/wasm.js?mf_vitest_no_cjs_esm_shim:161:5
 ❯ home/runner/work/prisma-vitest-miniflare/prisma-vitest-miniflare/node_modules/@prisma/client/runtime/wasm.js?mf_vitest_no_cjs_esm_shim:11:731
 ❯ Object.loadLibrary home/runner/work/prisma-vitest-miniflare/prisma-vitest-miniflare/node_modules/@prisma/client/runtime/wasm.js?mf_vitest_no_cjs_esm_shim:11:975
 ❯ gt.loadEngine home/runner/work/prisma-vitest-miniflare/prisma-vitest-miniflare/node_modules/@prisma/client/runtime/wasm.js?mf_vitest_no_cjs_esm_shim:11:3194
 ❯ gt.instantiateLibrary home/runner/work/prisma-vitest-miniflare/prisma-vitest-miniflare/node_modules/@prisma/client/runtime/wasm.js?mf_vitest_no_cjs_esm_shim:11:2778

Here's the reproduction repo: https://github.com/vladinator1000/prisma-vitest-miniflare/tree/27410822df6161c2d97006352fe74dbd0692e90e

Failing CI run: https://github.com/vladinator1000/prisma-vitest-miniflare/actions/runs/8733836366/job/23963290162

By the way, I was following the new vitest-pool-workers docs and they are amazing! 🎉 I absolutely love the new experience. It blows my mind that I can just import { env } from "cloudflare:test"; and everything is loaded.

vladinator1000 commented 2 months ago

We can close this now that https://github.com/prisma/prisma/issues/23911 is done 🥳

cloudflare / workers-sdk