Skip to content

Conversation

@abdulrehmank
Copy link

remainingLines[remainingLinesIndex] sometimes give undefined and the build gets failed

remainingLines[remainingLinesIndex] sometimes give undefined and the build gets failed
@tromey
Copy link
Contributor

tromey commented Jun 8, 2017

Thanks for the patch. This bug has been reported a few times, and there's another PR open for it (since obsoleted by other changes) -- but what is missing is a test case. I'd like to have one before merging this. I'm willing to write the test myself but I don't know how to reproduce the bug. Could you either write a test or explain how I can do so?

@fictitious
Copy link
Contributor

fictitious commented Jun 30, 2017

From my limited understanding, it happens when source code passed to fromStringWithSourceMap does not correspond to the sourcemap in SourceMapConsumer.

If you start by adding this test to test-source-node.js (which passes)

exports['test .fromStringWithSourceMap() empty code'] = function (assert) {
  var emptyCode = '';
  var node = SourceNode.fromStringWithSourceMap(
      emptyCode,
      new SourceMapConsumer(util.emptyMap));

  var result = node.toStringWithSourceMap({
    file: 'min.js'
  });
  var map = result.map;
  var code = result.code;

  assert.equal(code, emptyCode);
  assert.ok(map instanceof SourceMapGenerator, 'map instanceof SourceMapGenerator');
  map = map.toJSON();
  assert.equal(map.version, util.emptyMap.version);
  assert.equal(map.file, util.emptyMap.file);
  assert.equal(map.mappings, util.emptyMap.mappings);
};

then change it and make consumer to use testMap:

  var node = SourceNode.fromStringWithSourceMap(
      emptyCode,
      new SourceMapConsumer(util.testMap));

you get

FAILED ./test-source-node: test .fromStringWithSourceMap() empty code!
TypeError: Cannot read property 'substr' of undefined
    at Function.<anonymous> (...source-map/lib/source-node.js:121:26)

This is unlikely a valid use case, it probably just exposes some other bug somewhere in the code that uses sourcemap, and that code should be fixed regardless of what is done here.

@tromey
Copy link
Contributor

tromey commented Sep 27, 2017

Obsoleting in favor of #288 - same thing, slightly different spelling.

@tromey tromey closed this Sep 27, 2017
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

Successfully merging this pull request may close these issues.

3 participants