"TypeError: Cannot create proxy with a non-object as target or handler" when importing node module with a Proxy

JoshuaKGoldberg commented 4 months ago

Describe the bug

When importing from an installed npm package such as not-a-log that uses a Proxy, Vitest crashes with:

 FAIL  index.test.js [ index.test.js ]
TypeError: Cannot create proxy with a non-object as target or handler
 ❯ Object.get node_modules/not-a-log/not-a-log.js:9:12
 ❯ index.test.js:1:1
      1| import logger from "not-a-log";
       | ^
      2|

Reproduction

https://stackblitz.com/edit/vitest-dev-vitest-keoxtv

import logger from "not-a-log";

I also set up a full reproduction here: https://github.com/JoshuaKGoldberg/repros/tree/vitest-proxy-not-a-log

System Info

System:
    OS: Linux 5.0 undefined
    CPU: (8) x64 Intel(R) Core(TM) i9-9880H CPU @ 2.30GHz
    Memory: 0 Bytes / 0 Bytes
    Shell: 1.0 - /bin/jsh
  Binaries:
    Node: 18.20.3 - /usr/local/bin/node
    Yarn: 1.22.19 - /usr/local/bin/yarn
    npm: 10.2.3 - /usr/local/bin/npm
    pnpm: 8.15.6 - /usr/local/bin/pnpm
  npmPackages:
    @vitest/ui: latest => 2.0.2 
    vite: latest => 5.3.3 
    vitest: latest => 2.0.2

Used Package Manager

npm

Validations

[X] Follow our Code of Conduct
[X] Read the Contributing Guidelines.
[X] Read the docs.
[X] Check that there isn't already an issue that reports the same bug to avoid creating a duplicate.
[X] Check that this is a concrete bug. For Q&A open a GitHub Discussion or join our Discord Chat Server.
[X] The provided reproduction is a minimal reproducible example of the bug.

hi-ogawa commented 3 months ago

Thanks for the reproduction. It looks like this is because "then" property access is send to not-a-log's proxy due to interop. https://github.com/vitest-dev/vitest/blob/5d6d8013371b46522ff55cb64015d29e617994f2/packages/vite-node/src/client.ts#L547-L553

First of all, "then" access shouldn't happen for static import, but Vitest (Vite SSR) rewrite's all imports as dynamic import internally, so that part cannot be probably fixed (cf. https://github.com/vitest-dev/vitest/issues/5122).

Technically, the same error can happen in innocent-looking async/await code outside of Vitest, so maybe not-a-log should be robust about this use case. For example, a following code causes same error on node:

// node repro.mjs

async function main() {
  // "await" implicitly checks `mod.default.then`
  await import("not-a-log").then(mod => mod.default);
}

main();

// or maybe this looks more innocent
async function main2() {
  const { default: logger } = await import("not-a-log");
  return logger;
}

// "await" implicitly checks `logger.then`
const logger = await main2();

One way to save error this on Vitest is to fix this TODO

https://github.com/vitest-dev/vitest/blob/5d6d8013371b46522ff55cb64015d29e617994f2/packages/vite-node/src/client.ts#L526-L528

or you can try disabling this "interop" entirely by test.deps.interopDefault: false, which would preserve node import semantics.

export default defineConfig({
  test: {
    deps: {
      interopDefault: false,
    }
  },
})

JoshuaKGoldberg commented 3 months ago

Thanks for the deep dive & explanation! I filed an issue upstream: https://github.com/jimmywarting/not-a-log/issues/2.

sheremet-va commented 3 months ago

I am not sure if this is a bug in Vitest. I don't think it's possible to support this case because then on an object is always called even if you return then from a module:

// mod.ts
export const then = () => {}

// index.ts
await import('./mod.ts')

hi-ogawa commented 3 months ago

Named export of then would be impossible (which is the case of actor library in https://github.com/vitest-dev/vitest/issues/5122), but this issue is slightly different since Vitest is accessing then on default export object due to cjs interop.

Since not-a-log has type: "module", cjs interop should not be necessary, but Vitest does it currently due to this heuristics:

https://github.com/vitest-dev/vitest/blob/5d6d8013371b46522ff55cb64015d29e617994f2/packages/vite-node/src/client.ts#L526-L528

actor library crashes on Vite SSR, but not-a-log works on Vite SSR.

hi-ogawa commented 1 month ago

I'm closing this for the same reason as https://github.com/vitest-dev/vitest/issues/5122 and upstream https://github.com/vitejs/vite/issues/18328

vitest-dev / vitest