Skip to content

Commit e19a4cf

Browse files
authored
refactor: december (#801)
* refactor: remove unused * refactor: make options.maxFields more consitent with options.maxFiles * refactor: make pause a method and define this.req * refactor: make resume a method * refactor: remove placeholders * refactor: remove if statement that is always true * refactor: remove old code as comments (use git) * refactor: make plugins consistent (they all do not return) * refactor: do not use pluginReturn as plugins do not return anything * refactor: do not create unused variables * refactor: prefer forEach
1 parent 64f32c2 commit e19a4cf

File tree

5 files changed

+41
-87
lines changed

5 files changed

+41
-87
lines changed

README.md

+2-2
Original file line numberDiff line numberDiff line change
@@ -311,12 +311,12 @@ See it's defaults in [src/Formidable.js DEFAULT_OPTIONS](./src/Formidable.js)
311311
- `options.minFileSize` **{number}** - default `1` (1byte); the minium size of
312312
uploaded file.
313313
- `options.maxFiles` **{number}** - default `Infinity`;
314-
limit the amount of uploaded files.
314+
limit the amount of uploaded files, set Infinity for unlimited
315315
- `options.maxFileSize` **{number}** - default `200 * 1024 * 1024` (200mb);
316316
limit the size of each uploaded file.
317317
- `options.maxTotalFileSize` **{number}** - default `options.maxFileSize`;
318318
limit the size of the batch of uploaded files.
319-
- `options.maxFields` **{number}** - default `1000`; limit the number of fields, set 0 for unlimited
319+
- `options.maxFields` **{number}** - default `1000`; limit the number of fields, set Infinity for unlimited
320320
- `options.maxFieldsSize` **{number}** - default `20 * 1024 * 1024` (20mb);
321321
limit the amount of memory all fields together (except files) can allocate in
322322
bytes.

src/Formidable.js

+39-74
Original file line numberDiff line numberDiff line change
@@ -66,6 +66,7 @@ class IncomingForm extends EventEmitter {
6666
'bytesExpected',
6767
'bytesReceived',
6868
'_parser',
69+
'req',
6970
].forEach((key) => {
7071
this[key] = null;
7172
});
@@ -110,41 +111,42 @@ class IncomingForm extends EventEmitter {
110111
return this;
111112
}
112113

113-
parse(req, cb) {
114-
this.pause = () => {
115-
try {
116-
req.pause();
117-
} catch (err) {
118-
// the stream was destroyed
119-
if (!this.ended) {
120-
// before it was completed, crash & burn
121-
this._error(err);
122-
}
123-
return false;
114+
pause () {
115+
try {
116+
this.req.pause();
117+
} catch (err) {
118+
// the stream was destroyed
119+
if (!this.ended) {
120+
// before it was completed, crash & burn
121+
this._error(err);
124122
}
125-
return true;
126-
};
123+
return false;
124+
}
125+
return true;
126+
}
127127

128-
this.resume = () => {
129-
try {
130-
req.resume();
131-
} catch (err) {
132-
// the stream was destroyed
133-
if (!this.ended) {
134-
// before it was completed, crash & burn
135-
this._error(err);
136-
}
137-
return false;
128+
resume () {
129+
try {
130+
this.req.resume();
131+
} catch (err) {
132+
// the stream was destroyed
133+
if (!this.ended) {
134+
// before it was completed, crash & burn
135+
this._error(err);
138136
}
137+
return false;
138+
}
139139

140-
return true;
141-
};
140+
return true;
141+
}
142+
143+
parse(req, cb) {
144+
this.req = req;
142145

143146
// Setup callback first, so we don't miss anything from data events emitted immediately.
144147
if (cb) {
145148
const callback = once(dezalgo(cb));
146149
this.fields = {};
147-
let mockFields = '';
148150
const files = {};
149151

150152
this.on('field', (name, value) => {
@@ -245,16 +247,6 @@ class IncomingForm extends EventEmitter {
245247
return this.bytesReceived;
246248
}
247249

248-
pause() {
249-
// this does nothing, unless overwritten in IncomingForm.parse
250-
return false;
251-
}
252-
253-
resume() {
254-
// this does nothing, unless overwritten in IncomingForm.parse
255-
return false;
256-
}
257-
258250
onPart(part) {
259251
// this method can be overwritten by the user
260252
this._handlePart(part);
@@ -412,17 +404,12 @@ class IncomingForm extends EventEmitter {
412404
return;
413405
}
414406

415-
const results = [];
416-
const _dummyParser = new DummyParser(this, this.options);
417407

418-
// eslint-disable-next-line no-plusplus
419-
for (let idx = 0; idx < this._plugins.length; idx++) {
420-
const plugin = this._plugins[idx];
421-
422-
let pluginReturn = null;
408+
new DummyParser(this, this.options);
423409

410+
this._plugins.forEach((plugin, idx) => {
424411
try {
425-
pluginReturn = plugin(this, this.options) || this;
412+
plugin(this, this.options) || this;
426413
} catch (err) {
427414
// directly throw from the `form.parse` method;
428415
// there is no other better way, except a handle through options
@@ -435,42 +422,23 @@ class IncomingForm extends EventEmitter {
435422
throw error;
436423
}
437424

438-
Object.assign(this, pluginReturn);
439-
440425
// todo: use Set/Map and pass plugin name instead of the `idx` index
441-
this.emit('plugin', idx, pluginReturn);
442-
results.push(pluginReturn);
443-
}
444-
445-
this.emit('pluginsResults', results);
446-
447-
// NOTE: probably not needed, because we check options.enabledPlugins in the constructor
448-
// if (results.length === 0 /* && results.length !== this._plugins.length */) {
449-
// this._error(
450-
// new Error(
451-
// `bad content-type header, unknown content-type: ${this.headers['content-type']}`,
452-
// ),
453-
// );
454-
// }
426+
this.emit('plugin', idx);
427+
});
455428
}
456429

457430
_error(err, eventName = 'error') {
458-
// if (!err && this.error) {
459-
// this.emit('error', this.error);
460-
// return;
461-
// }
462431
if (this.error || this.ended) {
463432
return;
464433
}
465434

435+
this.req = null;
466436
this.error = err;
467437
this.emit(eventName, err);
468438

469-
if (Array.isArray(this.openedFiles)) {
470-
this.openedFiles.forEach((file) => {
471-
file.destroy();
472-
});
473-
}
439+
this.openedFiles.forEach((file) => {
440+
file.destroy();
441+
});
474442
}
475443

476444
_parseContentLength() {
@@ -585,7 +553,7 @@ class IncomingForm extends EventEmitter {
585553
}
586554

587555
_setUpMaxFields() {
588-
if (this.options.maxFields !== 0) {
556+
if (this.options.maxFields !== Infinity) {
589557
let fieldsCount = 0;
590558
this.on('field', () => {
591559
fieldsCount += 1;
@@ -621,13 +589,10 @@ class IncomingForm extends EventEmitter {
621589
}
622590

623591
_maybeEnd() {
624-
// console.log('ended', this.ended);
625-
// console.log('_flushing', this._flushing);
626-
// console.log('error', this.error);
627592
if (!this.ended || this._flushing || this.error) {
628593
return;
629594
}
630-
595+
this.req = null;
631596
this.emit('end');
632597
}
633598
}

src/plugins/octetstream.js

-2
Original file line numberDiff line numberDiff line change
@@ -14,8 +14,6 @@ export default function plugin(formidable, options) {
1414
if (/octet-stream/i.test(self.headers['content-type'])) {
1515
init.call(self, self, options);
1616
}
17-
18-
return self;
1917
}
2018

2119
// Note that it's a good practice (but it's up to you) to use the `this.options` instead

src/plugins/querystring.js

-2
Original file line numberDiff line numberDiff line change
@@ -15,8 +15,6 @@ export default function plugin(formidable, options) {
1515
if (/urlencoded/i.test(self.headers['content-type'])) {
1616
init.call(self, self, options);
1717
}
18-
19-
return self;
2018
};
2119

2220
// Note that it's a good practice (but it's up to you) to use the `this.options` instead

test/unit/custom-plugins.test.js

-7
Original file line numberDiff line numberDiff line change
@@ -60,10 +60,6 @@ test('should call 3 custom and 1 builtin plugins, when .parse() is called', asyn
6060
ctx.__pluginsCount = ctx.__pluginsCount || 0;
6161
ctx.__pluginsCount += 1;
6262
});
63-
form.on('pluginsResults', (results) => {
64-
expect(results.length).toBe(4);
65-
ctx.__pluginsResults = true;
66-
});
6763
form.on('end', () => {
6864
ctx.__ends = 1;
6965
expect(ctx.__customPlugin1).toBe(111);
@@ -117,9 +113,6 @@ test('.parse throw error when some plugin fail', async () => {
117113
ctx.__pluginsCount = ctx.__pluginsCount || 0;
118114
ctx.__pluginsCount += 1;
119115
});
120-
form.on('pluginsResults', () => {
121-
throw new Error('should not be called');
122-
});
123116

124117
form.once('error', () => {
125118
throw new Error('error event should not be fired when plugin throw');

0 commit comments

Comments
 (0)