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

Missing source-map-support dependency, fails with Yarn Berry PnP #35

Closed
noahnu opened this issue Nov 24, 2021 · 31 comments
Closed

Missing source-map-support dependency, fails with Yarn Berry PnP #35

noahnu opened this issue Nov 24, 2021 · 31 comments

Comments

@noahnu
Copy link

noahnu commented Nov 24, 2021

When upgrading from ts-node 10.2.0 to 10.4.0, I'm seeing a failure running a script that executes require('ts-node').register({ transpileOnly: true }).

This is the error I see:

@cspotcode/source-map-support tried to access source-map-support, but it isn't declared in its dependencies; this makes the require call ambiguous and unsound.

Required package: source-map-support

Required by: @cspotcode/source-map-support@npm:0.7.0 (via /Users/noah/.yarn/berry/cache/@cspotcode-source-map-support-npm-0.7.0-456c3ea2ce-8.zip/node_modules/@cspotcode/source-map-support/)

Using Yarn PnP 3.1.0. In yarn pnp, any package required needs to be declared in its package.json. I've tried to understand where source-map-support is required and whether it should be listed, but I had some trouble with the source code. If we want to check for the existence of the package, throwing it in a try...catch would be ideal.

@francisu
Copy link

francisu commented Feb 6, 2022

Getting a similar error when using Jest with this and Yarn 3.1.1:

Cannot find module 'source-map-support' from '/Users/francis/d/IdeaProjects/product/.yarn/cache/@cspotcode-source-map-support-npm-0.7.0-456c3ea2ce-9faddda775.zip/node_modules/@cspotcode/source-map-support'

  at resolveSync (../../.yarn/cache/resolve-patch-bad885c6ea-c79ecaea36.zip/node_modules/resolve/lib/sync.js:111:15)

@francisu
Copy link

francisu commented Feb 6, 2022

This Yarn 2+ patch makes the problem go away for me, but is likely not the right thing to do.
cspotcode-source-map-support.0.7.0.txt

@noahnu
Copy link
Author

noahnu commented Feb 6, 2022

@cspotcode can you offer some guidance on this issue?

@cspotcode
Copy link
Owner

cspotcode commented Feb 6, 2022

@noahnu is there more detail you can offer about the error message you're getting? For example, if there is a detailed stack trace, are you able to share that? I see the error message in the issue description but I'm not sure if you've truncated a stack trace, or if the error omits the stack trace.

@francisu
Copy link

francisu commented Feb 6, 2022

@noahnu is there more detail you can offer about the error message you're getting? For example, if there is a detailed stack trace, are you able to share that? I see the error messag in the issue description but I'm not sure if you've truncated a stack trace, or if the error omits the stack trace.

The errors shown by my and @noahnu are complete. That's all you get when you use jest. If you can look at my patch, you can see how it's fixed by removing the resolve call. I'm happy to try out another proposed change in my Yarn2 environment.

@noahnu
Copy link
Author

noahnu commented Feb 6, 2022

I'm trying to put together a repro repository to share.

@cspotcode
Copy link
Owner

Great, thanks for confirming. I'm going to attempt a reproduction on my end, too. If I can't reproduce, I'll share the reproduction with you so that you can fork and modify it to produce the error.

@cspotcode
Copy link
Owner

I've started a branch with yarn 3 PnP and ts-node.
https://github.com/TypeStrong/ts-node-repros/tree/node-source-map-support-issue-35
https://github.com/TypeStrong/ts-node-repros/blob/e84281dde030b5fd977474bc8466421c4e9f9c5f/run.sh#L1-L14

If you are already making progress on your own reproduction, feel free to share that. Or, if it is easier, you can fork the branch I've created and commit modifications to your fork, then share a link here.

@noahnu
Copy link
Author

noahnu commented Feb 6, 2022

Hmm, might be more of a jest issue as francisu mentioned. Confirming now

@noahnu
Copy link
Author

noahnu commented Feb 6, 2022

Okay, here's the repro: https://github.com/noahnu/repro-source-map-error

At least from how I was using it, it happens when you try to run jest using ts-node. I'm using a custom script which loads jest which is why we need the ts-node rather than purely rely on babel or ts-jest. This repro just tries to call jest directly though.

So to be clear, I was wrong with my original comment about it having to do with require('ts-node').

@francisu
Copy link

francisu commented Feb 6, 2022

At least from how I was using it, it happens when you try to run jest using ts-node. I'm using a custom script which loads jest which is why we need the ts-node rather than purely rely on babel or ts-jest. This repro just tries to call jest directly though.

My experience as well; thanks for doing the repro.

@cspotcode
Copy link
Owner

What output do you get when running the reproduction? I am not seeing the error message described in your initial post. I do not see "ambiguous and unsound."

@francisu
Copy link

francisu commented Feb 6, 2022

What output do you get when running the reproduction? I am not seeing the error message described in your initial post. I do not see "ambiguous and unsound."

Did you get anything bad? I did not get the ambiguous and unsound message either. Just 'cannot find module'.

@noahnu
Copy link
Author

noahnu commented Feb 6, 2022

The repro I shared was somewhat incomplete. With the current setup, this is my output:

λ yarn ts-node --transpile-only $(yarn bin jest)
 FAIL  ./index.test.ts
  ● Test suite failed to run

    Cannot find module 'source-map-support' from '/home/noah/repos/repro-source-map-error/.yarn/cache/@cspotcode-source-map-support-npm-0.7.0-456c3ea2ce-9faddda775.zip/node_modules/@cspotcode/source-map-support'

      at resolveSync (.yarn/cache/resolve-patch-bad885c6ea-c79ecaea36.zip/node_modules/resolve/lib/sync.js:111:15)

Test Suites: 1 failed, 1 total
Tests:       0 total
Snapshots:   0 total
Time:        0.058 s
Ran all test suites.

If I first add 'source-map-support' via yarn add -D source-map-support, then it passes. I believe the "ambigous and unsound" error is because in the use case which prompted this issue, I had an extra level of indirection so yarn recognized there was a source-map-support package it just didn't have access to -- I may have been using a resolver to force jest to go through yarn instead of using its built-in resolver, which is why I get the more yarn specific error. We can ignore that for the purpose of this issue and just look at support with pure jest.

@noahnu
Copy link
Author

noahnu commented Feb 6, 2022

At a higher level, what's the purpose of requiring source-map-support? Should it be listed in the package.json as a peer dependency?

@cspotcode
Copy link
Owner

I don't think we are, though. I think the opposite: jest is trying to require source-map-support, and we're redirecting to @cspotcode/source-map-support as per #23

The motivation for this is explained here: TypeStrong/ts-node#1441

It's also worth keeping in mind that jest does a lot of magic. The more magic jest has, the more likely it is to break. In this case, I see jest using this function. I don't know what it's supposed to do, but jest is using it to require source-map-support:

https://github.com/facebook/jest/blob/fcd626d202069073e6212acaf730d672323c7859/packages/jest-runtime/src/index.ts#L849-L870

@cspotcode
Copy link
Owner

Some more relevant comments here TypeStrong/ts-node#1496

If anyone in the future is reading this, please keep in mind that we are only forced to do this because source-map-support has not merged the corresponding PR to their repository.

Here is a link to that PR: evanw/node-source-map-support#209

@noahnu
Copy link
Author

noahnu commented Feb 6, 2022

Ah I see. I haven't reproduced my original error (ambiguous and unsound) outside of my work repo but I imagine the way the redirect is happening is causing the require to come through this repo's version of source-map-support which causes the ancestor chain to break -- hence the "ambiguous and unsound" error.

I wonder if listing source-map-support as an optional peer might still make sense. At some point this week when I'm at my work computer, I can try narrow the ambiguous and unsound aspect down further.

@cspotcode
Copy link
Owner

cspotcode commented Feb 6, 2022

Here's where jest is doing the weird internal require thing:

https://github.com/facebook/jest/blob/3a85065fe5604655e1337ffc1631f9999722c821/packages/jest-runner/src/runTest.ts#L213-L219

Looks like it's (a) asking node for the path to source-map-support, then it's (b) trying to require('source-map-support') relative to the path it gets. Step (b) is failing.

@francisu
Copy link

francisu commented Feb 6, 2022

@noahnu try clearing the jest cache (it's persistent): yarn jest --clearCache

@cspotcode
Copy link
Owner

cspotcode commented Feb 6, 2022

You might consider asking the jest folks what they're trying to do here: https://github.com/facebook/jest/blob/3a85065fe5604655e1337ffc1631f9999722c821/packages/jest-runner/src/runTest.ts#L213-L219

It seems like they're trying to require source-map-support through some custom reimplementation of node's module resolution algorithm. They do that to support their __mocks__ stuff. I'm not sure why that's appropriate in this case, because I don't think they're trying to mock or otherwise replace that module. This seems like it would work better:

// For tests
  const resolved = require.resolve('source-map-support');
  runtime
    .requireInternalModule<typeof import('source-map-support')>(
      resolved,
      resolved,
    )
    .install(sourcemapOptions);

@cspotcode
Copy link
Owner

Have you had any luck asking jest about the above ^ ?

@francisu
Copy link

francisu commented Feb 9, 2022

Have you had any luck asking jest about the above ^ ?

I will make an issue for them later today.

@cspotcode
Copy link
Owner

Thanks! If you share it here, I'll be sure to subscribe and follow along.

@cspotcode
Copy link
Owner

Any luck?

@francisu
Copy link

I tried your suggestion above in repro provided by @noahnu:

 % yarn ts-node --transpile-only $(yarn bin jest)
 FAIL  ./index.test.ts
  ● Test suite failed to run

    TypeError: (resolved , resolved).install is not a function

      at runTestInternal (.yarn/unplugged/jest-runner-npm-27.5.0-3919650619/node_modules/jest-runner/build/runTest.js:342:90)

Test Suites: 1 failed, 1 total
Tests:       0 total
Snapshots:   0 total
Time:        0.17 s
Ran all test suites.
(node:53783) UnhandledPromiseRejectionWarning: Error [ERR_MODULE_NOT_FOUND]: Cannot find package 'source-map-support' imported from /Users/francis/d/IdeaProjects/repro-source-map-error/.yarn/unplugged/jest-runner-npm-27.5.0-3919650619/node_modules/jest-runner/build/runTest.js
Did you mean to import source-map-support-npm-0.7.0-456c3ea2ce-9faddda775.zip/node_modules/@cspotcode/source-map-support/source-map-support.js?
    at packageResolve (internal/modules/esm/resolve.js:650:9)
    at moduleResolve (internal/modules/esm/resolve.js:691:18)
    at Loader.defaultResolve [as _resolve] (internal/modules/esm/resolve.js:805:11)
    at Loader.resolve (internal/modules/esm/loader.js:88:40)
    at Loader.getModuleJob (internal/modules/esm/loader.js:241:28)
    at Loader.import (internal/modules/esm/loader.js:176:28)
    at importModuleDynamically (internal/modules/cjs/loader.js:1028:27)
    at exports.importModuleDynamicallyCallback (internal/process/esm_loader.js:30:14)
    at runTestInternal (/Users/francis/d/IdeaProjects/repro-source-map-error/.yarn/unplugged/jest-runner-npm-27.5.0-3919650619/node_modules/jest-runner/build/runTest.js:342:11)
    at processTicksAndRejections (internal/process/task_queues.js:95:5)
(Use `node --trace-warnings ...` to show where the warning was created)
(node:53783) UnhandledPromiseRejectionWarning: Unhandled promise rejection. This error originated either by throwing inside of an async function without a catch block, or by rejecting a promise which was not handled with .catch(). To terminate the node process on unhandled promise rejection, use the CLI flag `--unhandled-rejections=strict` (see https://nodejs.org/api/cli.html#cli_unhandled_rejections_mode). (rejection id: 2)
(node:53783) [DEP0018] DeprecationWarning: Unhandled promise rejections are deprecated. In the future, promise rejections that are not handled will terminate the Node.js process with a non-zero exit code.

% yarn info @cspotcode/source-map-support -AR
└─ @cspotcode/source-map-support@npm:0.7.0
   ├─ Version: 0.7.0
   │
   └─ Dependencies
      └─ @cspotcode/source-map-consumer@npm:0.8.0 → npm:0.8.0

The require.resolve() call is returning this:

/Users/francis/d/IdeaProjects/repro-source-map-error/.yarn/cache/@cspotcode-source-map-support-npm-0.7.0-456c3ea2ce-9faddda775.zip/node_modules/@cspotcode/source-map-support/source-map-support.js

Here is the stack trace of the original exception:

onParseError(), url.js:279
URL(), url.js:355
defaultResolve(), resolve.js:767
resolve(), loader.js:88
getModuleJob(), loader.js:241
import(), loader.js:176
importModuleDynamically(), loader.js:1028
exports.importModuleDynamicallyCallback(), esm_loader.js:30
runTestInternal(), runTest.js:343

I'm not sure what's going wrong here, nor what I should ask the Jest people. If you could propose a patch that would fix the problem in the repro, I will do the leg work of making an issue with them and show how your patch fixes it.

@cspotcode
Copy link
Owner

This looks suspicious to me.

TypeError: (resolved , resolved).install is not a function

Almost as if there's typescript generics syntax in a JS file.

@francisu
Copy link

Actually, removing the '<typeof import('source-map-support')>' made it work, and I see now why. It looks like a typescript generic, but since it's in a JS file, it's a greater than and less than, and thus returning false, causing the error.

So I have a working patch and repro that I can make an issue with for the Jest team. I will do that shortly. Thanks for following up with me.

@francisu
Copy link

OK, let's see what they do.

@Kurt-von-Laven
Copy link

Kurt-von-Laven commented Apr 28, 2022

Ha ha, howdy @noahnu. Thank you all for sorting so much of this out already! I must confess that I have been putting off investigating it for months. The good news is that this issue was fixed a week ago by Jest's prolific maintainer. The bad news is that while the fix has been released in v28.0.0+, it didn't make v27, and ts-jest v27 rather sensibly pins Jest to v27. But wait, there's more good news! ts-jest 28.0.0-next.1 was released 2 days ago, and bumps Jest to v28. It works for me at Yarn 3.2.0 (but not at 3.1.0), so I think this issue can be closed if it works for others as well. Fair warning that Jest v28 has a lot of breaking changes, but I found their migration guide very helpful in my case.

@cspotcode
Copy link
Owner

Closing since this was a jest issue, so no action required here.

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