Skip to content
This repository has been archived by the owner on Jan 19, 2021. It is now read-only.

Sourcemaps not tracing back to the source #73

Open
filipesilva opened this issue Sep 6, 2017 · 11 comments · May be fixed by #87
Open

Sourcemaps not tracing back to the source #73

filipesilva opened this issue Sep 6, 2017 · 11 comments · May be fixed by #87

Comments

@filipesilva
Copy link
Contributor

filipesilva commented Sep 6, 2017

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.

@calebeno
Copy link

calebeno commented Dec 8, 2017

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

@felixfbecker
Copy link

This is preventing me from getting proper coverage output in unional/domture#9

@Predhin
Copy link

Predhin commented Jan 10, 2018

Any updates on this issue?

Thanks.

@csentis
Copy link

csentis commented Feb 6, 2018

@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.

@escalonn
Copy link

escalonn commented Aug 2, 2018

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.

@adamburgess
Copy link

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.

@felixfbecker
Copy link

Are you planning to open a PR for that?

@adamburgess
Copy link

adamburgess commented Oct 7, 2018

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.

@felixfbecker
Copy link

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.

filipesilva added a commit to filipesilva/istanbul-instrumenter-loader that referenced this issue Oct 8, 2018
Based on the work by @adamburgess in webpack-contrib#73 (comment).

Added Webpack 3 tests and a fix for the sourcemaps file names

Fix webpack-contrib#73
filipesilva added a commit to filipesilva/istanbul-instrumenter-loader that referenced this issue Oct 8, 2018
Based on the work by @adamburgess in webpack-contrib#73 (comment).

Added Webpack 3 tests and a fix for the sourcemaps file names

Fix webpack-contrib#73
@filipesilva filipesilva linked a pull request Oct 8, 2018 that will close this issue
2 tasks
filipesilva added a commit to filipesilva/istanbul-instrumenter-loader that referenced this issue Oct 8, 2018
Based on the work by @adamburgess in webpack-contrib#73 (comment).

Added Webpack 3 tests and a fix for the sourcemaps file names

Fix webpack-contrib#73
@filipesilva
Copy link
Contributor Author

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

@JamesMessinger
Copy link

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

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

Successfully merging a pull request may close this issue.

9 participants