Skip to content

Empty mappings fix implemented. #251

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.

Sign up for GitHub

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

Merged
merged 4 commits into from
Sep 22, 2017
Merged

Empty mappings fix implemented. #251

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
879443e
Empty mappings fix implemented.
dmurat Nov 6, 2016
4192bf4
SourceMapGenerator.fromSourceMap() test case added for a map with sin…
dmurat Nov 19, 2016
0f1607c
.gitignore updated
dmurat Nov 19, 2016
d08bc2f
Implementing fix and test for case when we have multiple sources but …
dmurat Nov 23, 2016
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions .gitignore
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1 +1,4 @@
*.iml
*.log
.idea
node_modules/*
9 changes: 9 additions & 0 deletions lib/source-map-generator.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -72,6 +72,15 @@ SourceMapGenerator.fromSourceMap =
generator.addMapping(newMapping);
});
aSourceMapConsumer.sources.forEach(function (sourceFile) {
var sourceRelative = sourceFile;
if (sourceRoot !== null) {
sourceRelative = util.relative(sourceRoot, sourceFile);
}

if (!generator._sources.has(sourceRelative)) {
generator._sources.add(sourceRelative);
}

var content = aSourceMapConsumer.sourceContentFor(sourceFile);
if (content != null) {
generator.setSourceContent(sourceFile, content);
Expand Down
24 changes: 24 additions & 0 deletions test/test-source-map-generator.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -252,6 +252,30 @@ exports['test .fromSourceMap with sourcesContent'] = function (assert) {
util.assertEqualMaps(assert, map.toJSON(), util.testMapWithSourcesContent);
};

exports['test .fromSourceMap with single source'] = function (assert) {
var map = SourceMapGenerator.fromSourceMap(
new SourceMapConsumer(util.testMapSingleSource));
util.assertEqualMaps(assert, map.toJSON(), util.testMapSingleSource);
};

exports['test .fromSourceMap with empty mappings'] = function (assert) {
Copy link
Contributor

@ejpbruel ejpbruel Nov 16, 2016

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could you also add a test case here for the edge case I described above? (i.e. mappings are not empty, but only ever refer to one source).

var map = SourceMapGenerator.fromSourceMap(
new SourceMapConsumer(util.testMapEmptyMappings));
util.assertEqualMaps(assert, map.toJSON(), util.testMapEmptyMappings);
};

exports['test .fromSourceMap with empty mappings and relative sources'] = function (assert) {
var map = SourceMapGenerator.fromSourceMap(
new SourceMapConsumer(util.testMapEmptyMappingsRelativeSources));
util.assertEqualMaps(assert, map.toJSON(), util.testMapEmptyMappingsRelativeSources_generatedExpected);
};

Copy link
Contributor

@ejpbruel ejpbruel Nov 22, 2016

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We also need a test for the following scenario:

  • We have two sources: foo.js and bar.js
  • The mappings are NOT empty, but all the mappings refer only to foo.js.
  • The test should assert that bar.js is also included in the list of sources.

Note that this test will fail with your current code, because it will only add bar.js if the mappings are empty.

exports['test .fromSourceMap with multiple sources where mappings refers only to single source'] = function (assert) {
var map = SourceMapGenerator.fromSourceMap(
new SourceMapConsumer(util.testMapMultiSourcesMappingRefersSingleSourceOnly));
util.assertEqualMaps(assert, map.toJSON(), util.testMapMultiSourcesMappingRefersSingleSourceOnly);
};

exports['test applySourceMap'] = function (assert) {
var node = new SourceNode(null, null, null, [
new SourceNode(2, 0, 'fileX', 'lineX2\n'),
Expand Down
52 changes: 52 additions & 0 deletions test/util.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -51,6 +51,58 @@ exports.testMapEmptySourceRoot = {
sourceRoot: '',
mappings: 'CAAC,IAAI,IAAM,SAAUA,GAClB,OAAOC,IAAID;CCDb,IAAI,IAAM,SAAUE,GAClB,OAAOA'
};
exports.testMapSingleSource = {
version: 3,
file: 'min.js',
names: ['bar', 'baz'],
sources: ['one.js'],
sourceRoot: '',
mappings: 'CAAC,IAAI,IAAM,SAAUA,GAClB,OAAOC,IAAID'
};
exports.testMapEmptyMappings = {
version: 3,
file: 'min.js',
names: [],
sources: ['one.js', 'two.js'],
sourcesContent: [
' ONE.foo = 1;',
' TWO.inc = 2;'
],
sourceRoot: '',
mappings: ''
};
exports.testMapEmptyMappingsRelativeSources = {
version: 3,
file: 'min.js',
names: [],
sources: ['./one.js', './two.js'],
sourcesContent: [
' ONE.foo = 1;',
' TWO.inc = 2;'
],
sourceRoot: '/the/root',
mappings: ''
};
exports.testMapEmptyMappingsRelativeSources_generatedExpected = {
version: 3,
file: 'min.js',
names: [],
sources: ['one.js', 'two.js'],
sourcesContent: [
' ONE.foo = 1;',
' TWO.inc = 2;'
],
sourceRoot: '/the/root',
mappings: ''
};
exports.testMapMultiSourcesMappingRefersSingleSourceOnly = {
version: 3,
file: 'min.js',
names: ['bar', 'baz'],
sources: ['one.js', 'withoutMappings.js'],
sourceRoot: '',
mappings: 'CAAC,IAAI,IAAM,SAAUA,GAClB,OAAOC,IAAID'
};
// This mapping is identical to above, but uses the indexed format instead.
exports.indexedTestMap = {
version: 3,
Expand Down