istanbul-instrumenter-loader icon indicating copy to clipboard operation
istanbul-instrumenter-loader copied to clipboard

Sourcemaps not tracing back to the source

Open filipesilva opened this issue 7 years ago • 11 comments

The sourcemaps that istanbul-instrumenter-loader returns are not using the input sourcemap to provide sourcemaps back to the source.

This can be reproduced in the tests for this repo.

  • modify the 'sourcemap files on by default' test in test/index.test.js to print the sources content:
  // ...
  const sourceMap = stats.compilation.assets['main.js.map'].source();
  console.log(JSON.parse(sourceMap).sourcesContent)
  // ...
  • npm run test
  • you should see the below printed:
[ '"use strict";\n\nvar _createClass = function () { function defineProperties(target, props) { for (var i = 0; i < props.length; i++) { var descriptor = props[i]; descriptor.enumerable = descriptor.enumerable || false; descriptor.configurable = true; if ("value" in descriptor) descriptor.writable = true; Object.defineProperty(target, descriptor.key, descriptor); } } return function (Constructor, protoProps, staticProps) { if (protoProps) defineProperties(Constructor.prototype, protoProps); if (staticProps) defineProperties(Constructor, staticProps); return Constructor; }; }();\n\nfunction _classCallCheck(instance, Constructor) { if (!(instance instanceof Constructor)) { throw new TypeError("Cannot call a class as a function"); } }\n\nmodule.exports = function () {\n  function Foo() {\n    _classCallCheck(this, Foo);\n  }\n\n_createClass(Foo, [{\n    key: "bar",\n    value: function bar() {\n      return !!this;\n    }\n  }]);\n\n  return Foo;\n}();\n\n\n// WEBPACK FOOTER //\n// /fixtures/basic.js' ]
  • this is how it looks formatted:
"use strict";

var _createClass = function () { function defineProperties(target, props) { for (var i = 0; i < props.length; i++) { var descriptor = props[i]; descriptor.enumerable = descriptor.enumerable || false; descriptor.configurable = true; if ("value" in descriptor) descriptor.writable = true; Object.defineProperty(target, descriptor.key, descriptor); } } return function (Constructor, protoProps, staticProps) { if (protoProps) defineProperties(Constructor.prototype, protoProps); if (staticProps) defineProperties(Constructor, staticProps); return Constructor; }; }();

function _classCallCheck(instance, Constructor) { if (!(instance instanceof Constructor)) { throw new TypeError("Cannot call a class as a function"); } }

module.exports = function () {
    function Foo() {
      _classCallCheck(this, Foo);
  }

_createClass(Foo, [{
      key: "bar",
    value: function bar() {
        return !!this;
    }
  }]);

  return Foo;
}();


// WEBPACK FOOTER //
// /fixtures/basic.js
  • which is quite different from the original source (tests/fixtures/basic.js):
module.exports = class Foo {
  bar() {
    return !!this;
  }
};

I tried tracing this around for a bit and couldn't find where it goes wrong, but it seems that the instrumenter is receiving the input sourcemap but instrumenter.lastSourceMap() isn't a sourcemap back to the source.

filipesilva avatar Sep 06 '17 15:09 filipesilva

Getting impacted by this issue currently. Any timeline on a possible fix?

calebeno avatar Dec 08 '17 15:12 calebeno

This is preventing me from getting proper coverage output in https://github.com/unional/domture/issues/9

felixfbecker avatar Jan 05 '18 02:01 felixfbecker

Any updates on this issue?

Thanks.

Predhin avatar Jan 10 '18 18:01 Predhin

@d3viant0ne Hi Joshua, Being a fan of your thoughtful work, kindly let me encourage you to re-evaluate this issue. From my point of view it is less an inconvenience and more a reduction of this good middleware‘s use to the lowest possible grade - at least in Angular-CLI-instrumented TS projects. Is it possible for you to give this a push? Thanks greatly for reconsidering!

Edit: I greatly overexaggerated. It is more like turning a sniper rifle into a shotgun.

csentis avatar Feb 06 '18 23:02 csentis

Any updates? This blocks coverage in SonarQube for Angular with the normal setup, since SonarQube needs the generated LCOV file's line numbers to match the TS source files.

escalonn avatar Aug 02 '18 23:08 escalonn

Here's an attempt at fixing it: https://github.com/adamburgess/istanbul-instrumenter-loader/commit/70355d4a578f330b7e24b207b4573ddd9f2a9388#diff-1fdf421c05c1140f6d71444ea2b27638 (lots of changes in this diff because I upgraded webpack/dependencies which were quite old, but the actual changes linked are very small) it's on npm if you want a quick install, @adamburgess/istanbul-instrumenter-loader

istanbul-instrumenter-lib doesn't use the sourcemap as an input, it just adds it to the output file. but you can use merge-source-map (which internally uses source-map) to merge the output source map with the input one, which seems to work.

In my projects, stack traces now have correct line numbers.

adamburgess avatar Oct 07 '18 16:10 adamburgess

Are you planning to open a PR for that?

felixfbecker avatar Oct 07 '18 16:10 felixfbecker

it's been almost a year since a functional change landed in this repo, so no, I'll just leave it as a separate repo. anyone else can take the changes and do what they want with them, of course.

adamburgess avatar Oct 07 '18 16:10 adamburgess

Some people would still appreciate a PR for visibility. Some community members might be able to give feedback on it. If there are enough PRs that noone is merging we should join efforts on one fork. Or just get the repo more people with write access.

felixfbecker avatar Oct 07 '18 23:10 felixfbecker

Added a PR based on the work by @adamburgess at #73 (comment). @michael-ciniawsky @d3viant0ne WDYT of this fix?

filipesilva avatar Oct 08 '18 14:10 filipesilva

I created a new npm package named coverage-istanbul-loader. It includes the fix mentioned above as well as other fixes and updated dependencies.

JamesMessinger avatar Feb 03 '19 00:02 JamesMessinger