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

Make the file property optional #95

Merged
merged 1 commit into from Feb 15, 2014
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
13 changes: 7 additions & 6 deletions README.md
Expand Up @@ -173,7 +173,7 @@ following attributes:

* `mappings`: A string of base64 VLQs which contain the actual mappings.

* `file`: The generated filename this source map is associated with.
* `file`: Optional. The generated filename this source map is associated with.

#### SourceMapConsumer.prototype.originalPositionFor(generatedPosition)

Expand Down Expand Up @@ -244,14 +244,14 @@ generated line/column in this source map.
An instance of the SourceMapGenerator represents a source map which is being
built incrementally.

#### new SourceMapGenerator(startOfSourceMap)
#### new SourceMapGenerator([startOfSourceMap])

To create a new one, you must pass an object with the following properties:
You may pass an object with the following properties:

* `file`: The filename of the generated source that this source map is
associated with.

* `sourceRoot`: An optional root for all relative URLs in this source map.
* `sourceRoot`: A root for all relative URLs in this source map.

#### SourceMapGenerator.fromSourceMap(sourceMapConsumer)

Expand Down Expand Up @@ -291,7 +291,8 @@ is the minimium of this map and the supplied map.
* `sourceMapConsumer`: The SourceMap to be applied.

* `sourceFile`: Optional. The filename of the source file.
If omitted, sourceMapConsumer.file will be used.
If omitted, sourceMapConsumer.file will be used, if it exists.
Otherwise an error will be thrown.

#### SourceMapGenerator.prototype.toString()

Expand Down Expand Up @@ -387,7 +388,7 @@ for trimming whitespace from the end of a source node, etc.
Return the string representation of this source node. Walks over the tree and
concatenates all the various snippets together to one string.

### SourceNode.prototype.toStringWithSourceMap(startOfSourceMap)
### SourceNode.prototype.toStringWithSourceMap([startOfSourceMap])

Returns the string representation of this tree of source nodes, plus a
SourceMapGenerator which contains all the mappings between the generated and
Expand Down
2 changes: 1 addition & 1 deletion lib/source-map/source-map-consumer.js
Expand Up @@ -29,7 +29,7 @@ define(function (require, exports, module) {
* - sourceRoot: Optional. The URL root from which all sources are relative.
* - sourcesContent: Optional. An array of contents of the original source files.
* - mappings: A string of base64 VLQs which contain the actual mappings.
* - file: The generated file this source map is associated with.
* - file: Optional. The generated file this source map is associated with.
*
* Here is an example source map, taken from the source map spec[0]:
*
Expand Down
20 changes: 15 additions & 5 deletions lib/source-map/source-map-generator.js
Expand Up @@ -15,14 +15,17 @@ define(function (require, exports, module) {

/**
* An instance of the SourceMapGenerator represents a source map which is
* being built incrementally. To create a new one, you must pass an object
* with the following properties:
* being built incrementally. You may pass an object with the following
* properties:
*
* - file: The filename of the generated source.
* - sourceRoot: An optional root for all URLs in this source map.
* - sourceRoot: A root for all relative URLs in this source map.
*/
function SourceMapGenerator(aArgs) {
this._file = util.getArg(aArgs, 'file');
if (!aArgs) {
aArgs = {};
}
this._file = util.getArg(aArgs, 'file', null);
this._sourceRoot = util.getArg(aArgs, 'sourceRoot', null);
this._sources = new ArraySet();
this._names = new ArraySet();
Expand Down Expand Up @@ -151,12 +154,19 @@ define(function (require, exports, module) {
*
* @param aSourceMapConsumer The source map to be applied.
* @param aSourceFile Optional. The filename of the source file.
* If omitted, SourceMapConsumer's file property will be used.
* If omitted, SourceMapConsumer's file property will be used,
* if it exists. Otherwise an error is thrown.
*/
SourceMapGenerator.prototype.applySourceMap =
function SourceMapGenerator_applySourceMap(aSourceMapConsumer, aSourceFile) {
// If aSourceFile is omitted, we will use the file property of the SourceMap
if (!aSourceFile) {
if (!aSourceMapConsumer.file) {
throw new Error(
'SourceMapGenerator.prototype.applySourceMap requires either an explicit source file, ' +
'or the source map\'s "file" property. Both were omitted.'
);
Copy link
Contributor

Choose a reason for hiding this comment

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

How about something like

throw new Error(
  'SourceMapGenerator.prototype.applySourceMap requires either an explicit source file, ' +
  'or the source map\'s "file" property. Both were omitted.'
);

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yeah, that sounds better. But really I'd like to deprecate omitting the second parameter. How can we warn users? console.warn? Or is that a separate pull request?

Copy link
Contributor

Choose a reason for hiding this comment

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

File an issue first to open discussion. My reaction at this moment in time is that nothing needs to be deprecated.

}
aSourceFile = aSourceMapConsumer.file;
}
var sourceRoot = this._sourceRoot;
Expand Down
2 changes: 1 addition & 1 deletion test/source-map/test-source-map-consumer.js
Expand Up @@ -12,7 +12,7 @@ define(function (require, exports, module) {
var SourceMapConsumer = require('../../lib/source-map/source-map-consumer').SourceMapConsumer;
var SourceMapGenerator = require('../../lib/source-map/source-map-generator').SourceMapGenerator;

exports['test that we can instantiate with a string or an objects'] = function (assert, util) {
exports['test that we can instantiate with a string or an object'] = function (assert, util) {
assert.doesNotThrow(function () {
var map = new SourceMapConsumer(util.testMap);
});
Expand Down
13 changes: 13 additions & 0 deletions test/source-map/test-source-map-generator.js
Expand Up @@ -20,6 +20,9 @@ define(function (require, exports, module) {
sourceRoot: '.'
});
assert.ok(true);

var map = new SourceMapGenerator();
assert.ok(true);
};

exports['test JSON serialization'] = function (assert, util) {
Expand Down Expand Up @@ -269,6 +272,16 @@ define(function (require, exports, module) {
util.assertEqualMaps(assert, actualMap, expectedMap);
};

exports['test applySourceMap throws when file is missing'] = function (assert, util) {
var map = new SourceMapGenerator({
file: 'test.js'
});
var map2 = new SourceMapGenerator();
assert.throws(function() {
map.applySourceMap(new SourceMapConsumer(map2.toJSON()));
})
};

exports['test sorting with duplicate generated mappings'] = function (assert, util) {
var map = new SourceMapGenerator({
file: 'test.js'
Expand Down
5 changes: 5 additions & 0 deletions test/source-map/test-source-node.js
Expand Up @@ -140,8 +140,13 @@ define(function (require, exports, module) {
var map = node.toStringWithSourceMap({
file: 'foo.js'
}).map;
var mapWithoutOptions = node.toStringWithSourceMap().map;

assert.ok(map instanceof SourceMapGenerator, 'map instanceof SourceMapGenerator');
assert.ok(mapWithoutOptions instanceof SourceMapGenerator, 'mapWithoutOptions instanceof SourceMapGenerator');
mapWithoutOptions._file = 'foo.js';
util.assertEqualMaps(assert, map.toJSON(), mapWithoutOptions.toJSON());

map = new SourceMapConsumer(map.toString());

var actual;
Expand Down