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

Source maps are mis-aligned #10330

Closed
moltar opened this issue Jul 28, 2020 · 11 comments
Closed

Source maps are mis-aligned #10330

moltar opened this issue Jul 28, 2020 · 11 comments

Comments

@moltar
Copy link

moltar commented Jul 28, 2020

🐛 Bug Report

Jest errors that show line numbers and code snippets aren't showing the right thing.

This seems to happen by merely including another package that already loads source-map-support module prior, for example pino-caller. You can see the code for pino-caller is very simple, and in fact no code paths get triggered, other than just the loading of the module itself.

When I comment out the require line in pino-caller, the source maps align fine.

I've looked at other source maps issues here, and none of them seem to be relevant to this.

To Reproduce

Steps to reproduce the behavior:

require('pino-caller')

describe('Name of the group', () => {
  it('should work', () => {
    // comment

    throw new Error('foo')
  })
})

Results in:


      4 |   it('should work', () => {
      5 |     // comment
    > 6 |
        | ^
      7 |     throw new Error('foo')
      8 |   })
      9 | })

      at Object.<anonymous> (test/foo.test.ts:6:15)

Expected behavior

To point to the right line of code.

Link to repl or repo (highly encouraged)

envinfo

  System:
    OS: macOS 10.15.4
    CPU: (8) x64 Intel(R) Core(TM) i7-6920HQ CPU @ 2.90GHz
  Binaries:
    Node: 12.18.2 - ~/.nvm/versions/node/v12.18.2/bin/node
    Yarn: 1.22.4 - /usr/local/bin/yarn
    npm: 6.14.6 - ~/.nvm/versions/node/v12.18.2/bin/npm
  npmPackages:
    jest: 26.1.0 => 26.1.0
@dmaretskyi
Copy link

dmaretskyi commented Aug 14, 2020

Repo with minimal code to reproduce: https://github.com/Marik-D/jest-source-map-issue

require('source-map-support').install() //commenting out this line makes the issue dissapear






test('test', () => {
  expect(false).toEqual(true)
})

Looks like just using jest with source-map-support is enough to reproduce.

Also, console logs and node's assert are unaffected by this issue.

@andinoJamesV
Copy link

So I dug into the code and where Istanbul is working it should not be.

Also I doubt It will be fixed any time soon coverage is built into V8 directly for example implementation see
https://www.npmjs.com/package/c8

@silasdavis
Copy link

silasdavis commented Feb 17, 2021

I have noticed an issue with the same symptoms that may or may not be related when using ts-jest with source-map-support. I have isolated it to that combination (though sounds like root cause is some combination with Jest).

A detailed ticket and minimal repro is here: kulshekhar/ts-jest#2372

Let me know if I should move that ticket to this or another issue against jest.

@silasdavis
Copy link

It looks like this is a Jest issue (?). Let me inline my reproduction with ts-jest here:

## 🐛 Bug Report

When testing files that use (https://github.com/evanw/node-source-map-support) with:

import 'source-map-support/register';

The line numbers in stack traces displayed via tests run with ts-jest are wrong. In particular they point to the source line in the compiled .js files not the .ts files despite referencing the .ts files.

I want to use source-map-support in my production files to have nice stack traces.

To Reproduce

Test a file with ts-jest that has import 'source-map-support/register'; and throw an error to get a stack trace. See the line numbers are misaligned. Remove the import and see the line numbers are correct.

Expected behavior

Line numbers should be correct from ts-jest regardless of whether source-map-support is imported.

Link to repo

Minimal repro here:

https://github.com/silasdavis/ts-jest-repro

Just run yarn test (see the README.md for more details).

Annotated output:

/usr/bin/node /usr/lib/node_modules/yarn/bin/yarn.js test
yarn run v1.22.10
$ tsc --build && jest
 FAIL  src/source-map.test.ts
  ● tests › foo

    bah

       8 |   return {a: 1}
       9 | }
    > 10 |
         | ^
      11 | export function blah() {
      12 |   throw new Error('bah')
      13 | }

!!! line below is incorrect (should be line 12) !!!
      at Object.blah (src/source-map.ts:10:11)
      at src/source-map.test.ts:8:26
      at Object.<anonymous> (src/source-map.test.ts:6:46)

 FAIL  src/no-source-map.test.ts
  ● tests › foo

    bah

      12 |
      13 | export function blah() {
    > 14 |   throw new Error('bah')
         |         ^
      15 | }
      16 |

!!! line below is correct !!!
      at Object.blah (src/no-source-map.ts:14:9)
      at src/no-source-map.test.ts:5:5
      at Object.<anonymous> (src/no-source-map.test.ts:4:15)

Test Suites: 2 failed, 2 total
Tests:       2 failed, 2 total
Snapshots:   0 total
Time:        0.877 s, estimated 2 s
Ran all test suites.
error Command failed with exit code 1.
info Visit https://yarnpkg.com/en/docs/cli/run for documentation about this command.

Process finished with exit code 1

Debug log:

Find ts-jest log here: https://github.com/silasdavis/ts-jest-repro/blob/main/ts-jest.log

envinfo

System:
    OS: Linux cora 5.10.12-arch1-1 #1 SMP PREEMPT Sun, 31 Jan 2021 00:41:06 +0000 x86_64 GNU/Linux
    Node version: v14.15.5

Npm packages:
    jest: 26.6.3
    ts-jest: 26.5.1
    typescript: 4.1.5
    babel(optional): N/A

@silasdavis
Copy link

silasdavis commented Feb 17, 2021

Okay, just for any fellow travellers it would appear than since 12.12.0 (nodejs/node#29564) source-map-support is not required since there is native node support. So unless you transitively depend on it you can safely remove it and still get nice stack traces. That seems to resolve issues with jest/ts-jest.

@andinoJamesV
Copy link

andinoJamesV commented Feb 17, 2021 via email

@andinoJamesV
Copy link

andinoJamesV commented Feb 17, 2021 via email

@SimenB
Copy link
Member

SimenB commented Feb 19, 2021

Jest installs source-map-support automatically in the test environment, so you shouldn't need to install it on your own. As long as your transformer is set up correctly source maps will work

@SimenB
Copy link
Member

SimenB commented Feb 19, 2021

No library should ever set up source-map-support either. You can consider mocking it?

I don't think this is a bug in Jest however, so I'll close it. source-map-support is something that should be set up on an app basis (which jest does automatically for you per test)

@SimenB SimenB closed this as completed Feb 19, 2021
@silasdavis
Copy link

silasdavis commented Feb 19, 2021

The issue here is that production files (under test) set up source-map-support by importing it, which causes the breakage in Jest.

You can set up source-map-support dynamically by passing an argument to node or by using a dynamic import/require (and so avoid this issue by not registering when running with Jest). For people importing the support in production code it does seem like a bug (somewhere at least) that this breaks line numbers in jest; we ought not to have to change our production code to appease our testing framework.

That said given the workarounds and the fact source map support does not seem to be needed in node 12.12 and above this doesn't seem like a priority.

@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 10, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

5 participants