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

Crash when using import-in-the-middle with module.exports = {...require('$dependency')} #157

Open
juan-fernandez opened this issue Oct 3, 2024 · 10 comments

Comments

@juan-fernandez
Copy link

juan-fernandez commented Oct 3, 2024

Expected Behavior

Using a myloader.mjs file like this

import * as module from 'module'

module.register('import-in-the-middle/hook.mjs', import.meta.url)

and using NODE_OPTIONS='--import ./myloader.mjs' $my_command does not crash.

Actual Behavior

Running

NODE_OPTIONS='--import ./myloader.mjs' npm run test:vitest

results in

(node:81156) Error: 'import-in-the-middle' failed to wrap 'file:///Users/juan.fernandezdealba/Desktop/dev/starter-kit-juan-fork/node_modules/@prisma/client/default.js'

Steps to Reproduce the Problem

  1. Clone my starter-kit fork:
git clone [email protected]:juan-fernandez/starter-kit.git
cd starter-kit
  1. Install dependencies
npm i
  1. Run vitest test command with --import flag:
NODE_OPTIONS='--import ./myloader.mjs' npm run test:vitest

See this error:

(node:81156) Error: 'import-in-the-middle' failed to wrap 'file:///Users/juan.fernandezdealba/Desktop/dev/starter-kit-juan-fork/node_modules/@prisma/client/default.js'

Specifications

Running npx envinfo

  System:
    OS: macOS 14.7
    CPU: (10) arm64 Apple M1 Max
  Binaries:
    Node: 20.17.0 - ~/.volta/tools/image/node/20.17.0/bin/node
    npm: 10.8.2 - ~/.volta/tools/image/node/20.17.0/bin/npm

Investigation

If you look at node_modules/@prisma/client/default.js the file looks like this:

module.exports = {
  ...require('.prisma/client/default'),
}

Funnily enough, if you change the file contents to this:

const res = require('.prisma/client/default')
module.exports = {
  ...res,
}

the crash does not happen anymore.

Context

DataDog/dd-trace-js#4713

Extra repro case

https://github.com/juan-fernandez/iitm-issue-repro is lighter. It does not throw the same exact error but seems to be caused with a very similar setup, so it's likely to be related

@timfish
Copy link
Contributor

timfish commented Oct 3, 2024

This doesn't look like a crash. It's a logged error telling you a module could not be wrapped.

@juan-fernandez
Copy link
Author

juan-fernandez commented Oct 3, 2024

Thanks for the quick response!

no test is run though, meaning vitest crashes somewhere:

➜  starter-kit git:(main) NODE_OPTIONS='--import ./myloader.mjs' npm run test:vitest


> [email protected] test:vitest
> dotenv -e .env.test vitest run

The CJS build of Vite's Node API is deprecated. See https://vitejs.dev/guide/troubleshooting.html#vite-cjs-node-api-deprecated for more details.

 RUN  v2.0.5 /Users/juan.fernandezdealba/Desktop/dev/starter-kit-juan-fork

(node:5132) Error: 'import-in-the-middle' failed to wrap 'file:///Users/juan.fernandezdealba/Desktop/dev/starter-kit-juan-fork/node_modules/@prisma/client/default.js'
(Use `node --trace-warnings ...` to show where the warning was created)
 ❯ src/server/modules/auth/email/__tests__/email.router.test.ts (0)
(node:5133) Error: 'import-in-the-middle' failed to wrap 'file:///Users/juan.fernandezdealba/Desktop/dev/starter-kit-juan-fork/node_modules/@prisma/client/default.js'
 ❯ src/server/modules/post/__tests__/post.router.test.ts (0)
 ❯ src/server/modules/auth/email/__tests__/email.router.test.ts (0)

⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯ Failed Suites 2 ⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯

 FAIL  src/server/modules/post/__tests__/post.router.test.ts [ src/server/modules/post/__tests__/post.router.test.ts ]
 FAIL  src/server/modules/auth/email/__tests__/email.router.test.ts [ src/server/modules/auth/email/__tests__/email.router.test.ts ]
TypeError: Expected string, array buffer, or typed array to be returned for the "source" from the "load" hook but got undefined.
⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯[1/2]⎯

 Test Files  2 failed (2)
      Tests  no tests
   Start at  14:29:15
   Duration  702ms (transform 20ms, setup 0ms, collect 0ms, tests 0ms, environment 0ms, prepare 329ms)


vs without the loader:

➜  starter-kit git:(main) npm run test:vitest 


> [email protected] test:vitest
> dotenv -e .env.test vitest run

The CJS build of Vite's Node API is deprecated. See https://vitejs.dev/guide/troubleshooting.html#vite-cjs-node-api-deprecated for more details.

 RUN  v2.0.5 /Users/juan.fernandezdealba/Desktop/dev/starter-kit

... 
 ❯ src/server/modules/auth/email/__tests__/email.router.test.ts (9)
   ❯ auth.email (9)
 ❯ src/server/modules/auth/email/__tests__/email.router.test.ts (9)
   ❯ auth.email (9)
     ✓ login (4)
     ❯ verifyOtp (5)
 ❯ src/server/modules/auth/email/__tests__/email.router.test.ts (9)
   ❯ auth.email (9)
     ✓ login (4)
     ❯ verifyOtp (5)
       ⠧ should successfully set session on valid OTP
 ✓ src/server/modules/post/__tests__/post.router.test.ts (2)
 ✓ src/server/modules/auth/email/__tests__/email.router.test.ts (9)

 Test Files  2 passed (2)
      Tests  11 passed (11)
   Start at  14:30:17
   Duration  1.36s (transform 97ms, setup 88ms, collect 1.56s, tests 354ms, environment 0ms, prepare 81ms)

@juan-fernandez
Copy link
Author

hey @timfish do you need more information? I can also try to debug it myself if you can give me some pointers 😄

@timfish
Copy link
Contributor

timfish commented Oct 8, 2024

I've not had a chance to look yet but my guess is that this is something to do with the parser.. although the parser should only be used for ESM.

@juan-fernandez
Copy link
Author

@timfish

although the parser should only be used for ESM.

where's this logic in the code? I'm debugging the issue but AFAICT the parsing happens for every file

@jsumners-nr
Copy link
Contributor

Please make the minimal reproduction a single script.

@juan-fernandez
Copy link
Author

@jsumners-nr

The smallest repro case I could create is in

https://github.com/juan-fernandez/iitm-issue-repro

The error is different 🤔, but I think they might be related somehow, and it does look like a bug. Let me know if you have questions 😄

@jsumners-nr
Copy link
Contributor

I am unable to assist. I don't know what require('#dependency') is.

@juan-fernandez
Copy link
Author

that's defined in https://github.com/juan-fernandez/iitm-issue-repro/blob/main/package.json#L19-L23

it's a package internal import mimicking the original issue: https://nodejs.org/api/packages.html#imports

@karrui

This comment was marked as off-topic.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants