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]: jest fails when @cspot/source-map-support and yarn 2+ pnp are involved. #12366

Closed
francisu opened this issue Feb 11, 2022 · 17 comments · Fixed by #12706
Closed

[Bug]: jest fails when @cspot/source-map-support and yarn 2+ pnp are involved. #12366

francisu opened this issue Feb 11, 2022 · 17 comments · Fixed by #12706
Labels

Comments

@francisu
Copy link

Version

27.5.1

Steps to reproduce

Use this repo: https://github.com/noahnu/repro-source-map-error (h/t @noahnu for making this)

  1. yarn install
  2. yarn ts-node --transpile-only $(yarn bin jest)

Expected behavior

Test should pass.

Actual behavior

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

    Cannot find module 'source-map-support' from '/Users/francis/temp/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.132 s
Ran all test suites.

Additional context

The original issue is: cspotcode/node-source-map-support#35, but it seems to be a Jest problem.

Here is the a patch that fixes it (thanks to @cspotcode )

Replace this code:

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

with:

  const resolved = require.resolve('source-map-support');
  runtime.requireInternalModule(resolved, resolved).install(sourcemapOptions);

This might be related to:

#8930
#11453

Environment

System:
    OS: macOS 12.1
    CPU: (12) x64 Intel(R) Core(TM) i7-8850H CPU @ 2.60GHz
  Binaries:
    Node: 14.17.3 - ~/.nvm/versions/node/v14.17.3/bin/node
    Yarn: 3.1.1 - /usr/local/bin/yarn
    npm: 8.1.0 - ~/.nvm/versions/node/v14.17.3/bin/npm
@cspotcode
Copy link
Contributor

Hi, replying to say that I'm following along and will be happy to answer any questions. Feel free to mention me directly.

@cspotcode
Copy link
Contributor

Adding a bit more context:

In the highlighted snippet of jest code above:

runtime.requireInternalModule(require.resolve('source-map-support'), 'source-map-support')

...it appears to be emulating what would happen if "source-map-support" were, internally, to require itself. That is, a file within source-map-support attempted to do require('source-map-support'). I'm curious what necessitates this bit of code. I understand jest hooks into module resolution to support various testing features. However, in this case, jest is loading an internal dependency, so it seems unnecessary to obey any resolver hooks. Why is require('source-map-support') insufficient?

"@cspotcode/source-map-support" redirects require() of "source-map-support" to "@cspotcode/source-map-support" (cspotcode/node-source-map-support#23), as a drop-in replacement with additional bugfixes and features. (cspotcode/node-source-map-support#24) This is necessary because "@cspotcode/source-map-support" correctly handles multiple copies of the library, which avoids broken stack traces, but "source-map-support" does not. (cspotcode/node-source-map-support#28, evanw/node-source-map-support#209)

Given the above, I'm wondering what is the best path forward. Think my proposed patch solves the problem, but I don't understand why jest's current logic is necessary.

@SimenB
Copy link
Member

SimenB commented Feb 11, 2022

However, in this case, jest is loading an internal dependency, so it seems unnecessary to obey any resolver hooks. Why is require('source-map-support') insufficient?

internal in this case means "load inside the vm sandbox, but since internal, skip all user config for resolution and transformation". The code linked here runs outside the sandbox

@cspotcode
Copy link
Contributor

cspotcode commented Feb 11, 2022

I halfway understand what you're saying. As a more concrete follow-up question to hopefully aid my understanding:

Is my proposed fix acceptable? If it's not, what rules is it breaking?

@SimenB
Copy link
Member

SimenB commented Feb 11, 2022

Yeah, your fix seems correct, we should just delete line 217. Not sure why it was ever added...

@SimenB
Copy link
Member

SimenB commented Feb 11, 2022

PR welcome :grin\

@francisu
Copy link
Author

PR welcome :grin\

OK, I can do one tomorrow.

@SimenB
Copy link
Member

SimenB commented Feb 11, 2022

ideally with some integration test that fails without the change. Possibly just the reproduction used in the OP

@francisu
Copy link
Author

ideally with some integration test that fails without the change. Possibly just the reproduction used in the OP

Naturally

@cspotcode
Copy link
Contributor

Awesome, thank you both!

@SimenB
Copy link
Member

SimenB commented Feb 15, 2022

@francisu ping 🙂

@francisu
Copy link
Author

@francisu ping 🙂

@SimenB Give me another week or so on this. My schedule is pretty jammed. It's on my list.

@SimenB
Copy link
Member

SimenB commented Feb 17, 2022

Sounds good 👍

@Methuselah96
Copy link
Contributor

Interestingly enough I had this error in CI (Ubuntu), but it worked fine locally (Windows). Added this to my .yarnrc.yml while waiting for this to get fixed:

packageExtensions:
  '@cspotcode/source-map-support@*':
    dependencies:
      source-map-support: '*'

@github-actions
Copy link

github-actions bot commented Apr 3, 2022

This issue is stale because it has been open 30 days with no activity. Remove stale label or comment or this will be closed in 30 days.

@github-actions github-actions bot added the Stale label Apr 3, 2022
@SimenB
Copy link
Member

SimenB commented Apr 4, 2022

@francisu ping 😀

@github-actions
Copy link

This issue has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.
Please note this issue tracker is not a help forum. We recommend using StackOverflow or our discord channel for questions.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators May 22, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants