Skip to content

Commit 61473f8

Browse files
phatedphated
committed
Breaking: Reduce argument juggling (closes #15)
1 parent 86aa9c7 commit 61473f8

File tree

3 files changed

+195
-88
lines changed

3 files changed

+195
-88
lines changed

‎index.js‎

Lines changed: 22 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,10 @@ var helpers = require('./lib/helpers');
66

77
var PLUGIN_NAME = 'vinyl-sourcemap';
88

9+
function isObject(value) {
10+
return value && typeof value === 'object' && !Array.isArray(value);
11+
}
12+
913
/**
1014
* Add a sourcemap to a vinyl file (async, with callback function)
1115
* @param file
@@ -14,21 +18,23 @@ var PLUGIN_NAME = 'vinyl-sourcemap';
1418
*/
1519
function add(file, options, callback) {
1620

17-
// check if options are passed or a callback as second argument
18-
// if there are 3 arguments, the options param should be an object
21+
// Check if options or a callback are passed as second argument
1922
if (typeof options === 'function') {
2023
callback = options;
2124
options = {};
22-
} else if (!options || typeof options !== 'object') {
23-
return callback(new Error(PLUGIN_NAME + '-add: Invalid argument: options'));
2425
}
2526

26-
// Throw an error if the file argument is not a vinyl file
27+
// Default options if not an object
28+
if (!isObject(options)) {
29+
options = {};
30+
}
31+
32+
// Bail early an error if the file argument is not a Vinyl file
2733
if (!File.isVinyl(file)) {
2834
return callback(new Error(PLUGIN_NAME + '-add: Not a vinyl file'));
2935
}
3036

31-
// Return the file if already has sourcemaps
37+
// Bail early successfully if file already has sourcemap
3238
if (file.sourceMap) {
3339
return callback(null, file);
3440
}
@@ -56,23 +62,30 @@ function add(file, options, callback) {
5662
*/
5763
function write(file, options, callback) {
5864

59-
// Check arguments for optional destPath, options, or callback function
65+
// Check if options or a callback are passed as second argument
6066
if (typeof options === 'function') {
6167
callback = options;
6268
options = {};
6369
}
6470

65-
options = options || {};
71+
// Default options if not an object
72+
if (!isObject(options)) {
73+
options = {};
74+
}
6675

67-
// Throw an error if the file argument is not a vinyl file
76+
// Bail early with an error if the file argument is not a Vinyl file
6877
if (!File.isVinyl(file)) {
6978
return callback(new Error(PLUGIN_NAME + '-write: Not a vinyl file'));
7079
}
7180

81+
// Bail early with an error if file has streaming contents
82+
// TODO: needs test
7283
if (file.isStream()) {
7384
return callback(new Error(PLUGIN_NAME + '-write: Streaming not supported'));
7485
}
7586

87+
// Bail early successfully if file is null or doesn't have sourcemap
88+
// TODO: needs test (at least for null contents?)
7689
if (file.isNull() || !file.sourceMap) {
7790
return callback(null, file);
7891
}

‎test/add.js‎

Lines changed: 84 additions & 37 deletions
Original file line numberDiff line numberDiff line change
@@ -39,56 +39,103 @@ function makeFileWithInlineSourceMap() {
3939

4040
describe('add', function() {
4141

42-
it('should not accept null as argument', function(done) {
43-
sourcemaps.add(null, function(err) {
44-
expect(err instanceof Error && err.message === 'vinyl-sourcemap-add: Not a vinyl file').toExist();
45-
done();
42+
describe('ensures file argument', function() {
43+
44+
it('is not undefined', function(done) {
45+
sourcemaps.add(undefined, function(err) {
46+
expect(err instanceof Error && err.message === 'vinyl-sourcemap-add: Not a vinyl file').toExist();
47+
done();
48+
});
4649
});
47-
});
4850

49-
it('should not accept an empty object as argument', function(done) {
50-
sourcemaps.add({}, function(err) {
51-
expect(err instanceof Error && err.message === 'vinyl-sourcemap-add: Not a vinyl file').toExist();
52-
done();
51+
it('is not null', function(done) {
52+
sourcemaps.add(null, function(err) {
53+
expect(err instanceof Error && err.message === 'vinyl-sourcemap-add: Not a vinyl file').toExist();
54+
done();
55+
});
5356
});
54-
});
5557

56-
it('should not accept a stream as argument', function(done) {
57-
sourcemaps.add(new stream.Readable(), function(err) {
58-
expect(err instanceof Error && err.message === 'vinyl-sourcemap-add: Not a vinyl file').toExist();
59-
done();
58+
it('is not a plain object', function(done) {
59+
sourcemaps.add({}, function(err) {
60+
expect(err instanceof Error && err.message === 'vinyl-sourcemap-add: Not a vinyl file').toExist();
61+
done();
62+
});
6063
});
61-
});
6264

63-
it('should not accept undefined as options argument', function(done) {
64-
var file = makeFile();
65-
sourcemaps.add(file, undefined, function(err) {
66-
expect(err instanceof Error && err.message === 'vinyl-sourcemap-add: Invalid argument: options').toExist();
67-
done();
65+
// TODO: seems like a bad test
66+
it('is not a stream', function(done) {
67+
sourcemaps.add(new stream.Readable(), function(err) {
68+
expect(err instanceof Error && err.message === 'vinyl-sourcemap-add: Not a vinyl file').toExist();
69+
done();
70+
});
6871
});
69-
});
7072

71-
it('should not accept null as options argument', function(done) {
72-
var file = makeFile();
73-
sourcemaps.add(file, null, function(err) {
74-
expect(err instanceof Error && err.message === 'vinyl-sourcemap-add: Invalid argument: options').toExist();
75-
done();
73+
it('is a vinyl object', function(done) {
74+
var file = makeFile();
75+
sourcemaps.add(file, function(err) {
76+
expect(err).toNotExist();
77+
done();
78+
});
7679
});
7780
});
7881

79-
it('should not accept empty string as options argument', function(done) {
80-
var file = makeFile();
81-
sourcemaps.add(file, '', function(err) {
82-
expect(err instanceof Error && err.message === 'vinyl-sourcemap-add: Invalid argument: options').toExist();
83-
done();
82+
describe('ensures options argument', function() {
83+
84+
85+
it('is defaulted if undefined', function(done) {
86+
var file = makeFile();
87+
sourcemaps.add(file, undefined, function(err) {
88+
expect(err).toNotExist();
89+
done();
90+
});
8491
});
85-
});
8692

87-
it('should not accept boolean as options argument', function(done) {
88-
var file = makeFile();
89-
sourcemaps.add(file, true, function(err) {
90-
expect(err instanceof Error && err.message === 'vinyl-sourcemap-add: Invalid argument: options').toExist();
91-
done();
93+
it('is defaulted if null', function(done) {
94+
var file = makeFile();
95+
sourcemaps.add(file, null, function(err) {
96+
expect(err).toNotExist();
97+
done();
98+
});
99+
});
100+
101+
it('is defaulted if empty string', function(done) {
102+
var file = makeFile();
103+
sourcemaps.add(file, '', function(err) {
104+
expect(err).toNotExist();
105+
done();
106+
});
107+
});
108+
109+
it('is defaulted if non-empty string', function(done) {
110+
var file = makeFile();
111+
sourcemaps.add(file, 'invalid', function(err) {
112+
expect(err).toNotExist();
113+
done();
114+
});
115+
});
116+
117+
it('is defaulted if boolean false', function(done) {
118+
var file = makeFile();
119+
sourcemaps.add(file, false, function(err) {
120+
expect(err).toNotExist();
121+
done();
122+
});
123+
});
124+
125+
it('is defaulted if boolean true', function(done) {
126+
var file = makeFile();
127+
sourcemaps.add(file, true, function(err) {
128+
expect(err).toNotExist();
129+
done();
130+
});
131+
});
132+
133+
it('is defaulted if array', function(done) {
134+
var file = makeFile();
135+
sourcemaps.add(file, [], function(err) {
136+
expect(err).toNotExist();
137+
done();
138+
});
92139
});
93140
});
94141

‎test/write.js‎

Lines changed: 89 additions & 42 deletions
Original file line numberDiff line numberDiff line change
@@ -50,66 +50,113 @@ function base64JSON(object) {
5050

5151
describe('write', function() {
5252

53-
it('should return an error when on valid vinyl file is provided', function(done) {
54-
sourcemaps.write(undefined, function(err) {
55-
expect(err instanceof Error && err.message === 'vinyl-sourcemap-write: Not a vinyl file').toExist();
56-
done();
53+
describe('ensures file argument', function() {
54+
55+
it('is not undefined', function(done) {
56+
sourcemaps.write(undefined, function(err) {
57+
expect(err instanceof Error && err.message === 'vinyl-sourcemap-write: Not a vinyl file').toExist();
58+
done();
59+
});
5760
});
58-
});
5961

60-
it('should return an error when on valid vinyl file is provided', function(done) {
61-
sourcemaps.write(null, function(err) {
62-
expect(err instanceof Error && err.message === 'vinyl-sourcemap-write: Not a vinyl file').toExist();
63-
done();
62+
it('is not null', function(done) {
63+
sourcemaps.write(null, function(err) {
64+
expect(err instanceof Error && err.message === 'vinyl-sourcemap-write: Not a vinyl file').toExist();
65+
done();
66+
});
6467
});
65-
});
6668

67-
it('should return an error when on valid vinyl file is provided', function(done) {
68-
sourcemaps.write({}, function(err) {
69-
expect(err instanceof Error && err.message === 'vinyl-sourcemap-write: Not a vinyl file').toExist();
70-
done();
69+
it('is not a plain object', function(done) {
70+
sourcemaps.write({}, function(err) {
71+
expect(err instanceof Error && err.message === 'vinyl-sourcemap-write: Not a vinyl file').toExist();
72+
done();
73+
});
7174
});
72-
});
7375

74-
it('should return an error when on valid vinyl file is provided', function(done) {
75-
sourcemaps.write(new stream.Readable(), function(err) {
76-
expect(err instanceof Error && err.message === 'vinyl-sourcemap-write: Not a vinyl file').toExist();
77-
done();
76+
// TODO: seems like a bad test
77+
it('is not a stream', function(done) {
78+
sourcemaps.write(new stream.Readable(), function(err) {
79+
expect(err instanceof Error && err.message === 'vinyl-sourcemap-write: Not a vinyl file').toExist();
80+
done();
81+
});
7882
});
79-
});
8083

81-
it('calls back with the untouched file if sourceMap property does not exist', function(done) {
82-
var file = makeFile();
83-
delete file.sourceMap;
84-
sourcemaps.write(file, function(err, outFile) {
85-
expect(err).toNotExist();
86-
expect(file).toExist();
87-
expect(outFile).toEqual(file);
88-
done(err);
84+
it('is a vinyl object', function(done) {
85+
var file = makeFile();
86+
sourcemaps.write(file, function(err) {
87+
expect(err).toNotExist();
88+
done();
89+
});
8990
});
9091
});
9192

92-
it('should return an error when invalid arguments are provided', function(done) {
93-
var file = makeFile();
94-
sourcemaps.write(file, undefined, function(err) {
95-
expect(err).toNotExist();
96-
done();
93+
describe('ensures options argument', function() {
94+
95+
it('is defaulted if undefined', function(done) {
96+
var file = makeFile();
97+
sourcemaps.write(file, undefined, function(err) {
98+
expect(err).toNotExist();
99+
done();
100+
});
97101
});
98-
});
99102

100-
it('should return an error when invalid arguments are provided', function(done) {
101-
var file = makeFile();
102-
sourcemaps.write(file, null, function(err) {
103-
expect(err).toNotExist();
104-
done();
103+
it('is defaulted if null', function(done) {
104+
var file = makeFile();
105+
sourcemaps.write(file, null, function(err) {
106+
expect(err).toNotExist();
107+
done();
108+
});
109+
});
110+
111+
it('is defaulted if empty string', function(done) {
112+
var file = makeFile();
113+
sourcemaps.write(file, '', function(err) {
114+
expect(err).toNotExist();
115+
done();
116+
});
117+
});
118+
119+
it('is defaulted if non-empty string', function(done) {
120+
var file = makeFile();
121+
sourcemaps.write(file, 'invalid', function(err) {
122+
expect(err).toNotExist();
123+
done();
124+
});
125+
});
126+
127+
it('is defaulted if boolean false', function(done) {
128+
var file = makeFile();
129+
sourcemaps.write(file, false, function(err) {
130+
expect(err).toNotExist();
131+
done();
132+
});
133+
});
134+
135+
it('is defaulted if boolean true', function(done) {
136+
var file = makeFile();
137+
sourcemaps.write(file, true, function(err) {
138+
expect(err).toNotExist();
139+
done();
140+
});
141+
});
142+
143+
it('is defaulted if array', function(done) {
144+
var file = makeFile();
145+
sourcemaps.write(file, [], function(err) {
146+
expect(err).toNotExist();
147+
done();
148+
});
105149
});
106150
});
107151

108-
it('should return an error when invalid arguments are provided', function(done) {
152+
it('calls back with the untouched file if sourceMap property does not exist', function(done) {
109153
var file = makeFile();
110-
sourcemaps.write(file, true, function(err) {
154+
delete file.sourceMap;
155+
sourcemaps.write(file, function(err, outFile) {
111156
expect(err).toNotExist();
112-
done();
157+
expect(file).toExist();
158+
expect(outFile).toEqual(file);
159+
done(err);
113160
});
114161
});
115162

0 commit comments

Comments
 (0)