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

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

Closed
vladinator1000 opened this issue Feb 29, 2024 · 9 comments · Fixed by #5070
Closed
Labels
bug Something that isn't working

Comments

@vladinator1000
Copy link

vladinator1000 commented Feb 29, 2024

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: prisma/prisma#23193 (comment)

@vladinator1000 vladinator1000 added the bug Something that isn't working label Feb 29, 2024
@github-project-automation github-project-automation bot moved this to Untriaged in workers-sdk Feb 29, 2024
@vladinator1000 vladinator1000 changed the title 🐛 BUG: vitest-pool-workers compatibility issue "TypeError: deprecate is not a function" when using the jose library 🐛 BUG: vitest-pool-workers node compatibility with the jose and prisma libraries Feb 29, 2024
@mrbbot
Copy link
Contributor

mrbbot commented Feb 29, 2024

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
Copy link
Author

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

@mrbbot
Copy link
Contributor

mrbbot commented Mar 5, 2024

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 (#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
Copy link
Author

vladinator1000 commented Mar 6, 2024

@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
Copy link
Contributor

mrbbot commented Mar 11, 2024

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
Copy link
Contributor

mrbbot commented Mar 12, 2024

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

@mrbbot mrbbot reopened this Mar 12, 2024
@github-project-automation github-project-automation bot moved this from Done to Untriaged in workers-sdk Mar 12, 2024
@vladinator1000
Copy link
Author

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

@vladinator1000
Copy link
Author

vladinator1000 commented Apr 18, 2024

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
Copy link
Author

We can close this now that prisma/prisma#23911 is done 🥳

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something that isn't working
Projects
Archived in project
2 participants