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

Author: GrosSacASac

README.md

@@ -311,12 +311,12 @@ See it's defaults in [src/Formidable.js DEFAULT_OPTIONS](./src/Formidable.js)
 - `options.minFileSize` **{number}** - default `1` (1byte); the minium size of
   uploaded file.
 - `options.maxFiles` **{number}** - default `Infinity`;
-  limit the amount of uploaded files.
+  limit the amount of uploaded files, set Infinity for unlimited
 - `options.maxFileSize` **{number}** - default `200 * 1024 * 1024` (200mb);
   limit the size of each uploaded file.
 - `options.maxTotalFileSize` **{number}** - default `options.maxFileSize`;
   limit the size of the batch of uploaded files.
-- `options.maxFields` **{number}** - default `1000`; limit the number of fields, set 0 for unlimited
+- `options.maxFields` **{number}** - default `1000`; limit the number of fields, set Infinity for unlimited
 - `options.maxFieldsSize` **{number}** - default `20 * 1024 * 1024` (20mb);
   limit the amount of memory all fields together (except files) can allocate in
   bytes.

src/Formidable.js

@@ -66,6 +66,7 @@ class IncomingForm extends EventEmitter {
       'bytesExpected',
       'bytesReceived',
       '_parser',
+      'req',
     ].forEach((key) => {
       this[key] = null;
     });
@@ -110,41 +111,42 @@ class IncomingForm extends EventEmitter {
     return this;
   }
 
-  parse(req, cb) {
-    this.pause = () => {
-      try {
-        req.pause();
-      } catch (err) {
-        // the stream was destroyed
-        if (!this.ended) {
-          // before it was completed, crash & burn
-          this._error(err);
-        }
-        return false;
+  pause () {
+    try {
+      this.req.pause();
+    } catch (err) {
+      // the stream was destroyed
+      if (!this.ended) {
+        // before it was completed, crash & burn
+        this._error(err);
       }
-      return true;
-    };
+      return false;
+    }
+    return true;
+  }
 
-    this.resume = () => {
-      try {
-        req.resume();
-      } catch (err) {
-        // the stream was destroyed
-        if (!this.ended) {
-          // before it was completed, crash & burn
-          this._error(err);
-        }
-        return false;
+  resume () {
+    try {
+      this.req.resume();
+    } catch (err) {
+      // the stream was destroyed
+      if (!this.ended) {
+        // before it was completed, crash & burn
+        this._error(err);
       }
+      return false;
+    }
 
-      return true;
-    };
+    return true;
+  }
+
+  parse(req, cb) {
+    this.req = req;
 
     // Setup callback first, so we don't miss anything from data events emitted immediately.
     if (cb) {
       const callback = once(dezalgo(cb));
       this.fields = {};
-      let mockFields = '';
       const files = {};
 
       this.on('field', (name, value) => {
@@ -245,16 +247,6 @@ class IncomingForm extends EventEmitter {
     return this.bytesReceived;
   }
 
-  pause() {
-    // this does nothing, unless overwritten in IncomingForm.parse
-    return false;
-  }
-
-  resume() {
-    // this does nothing, unless overwritten in IncomingForm.parse
-    return false;
-  }
-
   onPart(part) {
     // this method can be overwritten by the user
     this._handlePart(part);
@@ -412,17 +404,12 @@ class IncomingForm extends EventEmitter {
       return;
     }
 
-    const results = [];
-    const _dummyParser = new DummyParser(this, this.options);
 
-    // eslint-disable-next-line no-plusplus
-    for (let idx = 0; idx < this._plugins.length; idx++) {
-      const plugin = this._plugins[idx];
-
-      let pluginReturn = null;
+    new DummyParser(this, this.options);
 
+    this._plugins.forEach((plugin, idx) => {  
       try {
-        pluginReturn = plugin(this, this.options) || this;
+        plugin(this, this.options) || this;
       } catch (err) {
         // directly throw from the `form.parse` method;
         // there is no other better way, except a handle through options
@@ -435,42 +422,23 @@ class IncomingForm extends EventEmitter {
         throw error;
       }
 
-      Object.assign(this, pluginReturn);
-
       // todo: use Set/Map and pass plugin name instead of the `idx` index
-      this.emit('plugin', idx, pluginReturn);
-      results.push(pluginReturn);
-    }
-
-    this.emit('pluginsResults', results);
-
-    // NOTE: probably not needed, because we check options.enabledPlugins in the constructor
-    // if (results.length === 0 /* && results.length !== this._plugins.length */) {
-    //   this._error(
-    //     new Error(
-    //       `bad content-type header, unknown content-type: ${this.headers['content-type']}`,
-    //     ),
-    //   );
-    // }
+      this.emit('plugin', idx);
+    });
   }
 
   _error(err, eventName = 'error') {
-    // if (!err && this.error) {
-    //   this.emit('error', this.error);
-    //   return;
-    // }
     if (this.error || this.ended) {
       return;
     }
 
+    this.req = null;
     this.error = err;
     this.emit(eventName, err);
 
-    if (Array.isArray(this.openedFiles)) {
-      this.openedFiles.forEach((file) => {
-        file.destroy();
-      });
-    }
+    this.openedFiles.forEach((file) => {
+      file.destroy();
+    });
   }
 
   _parseContentLength() {
@@ -585,7 +553,7 @@ class IncomingForm extends EventEmitter {
   }
 
   _setUpMaxFields() {
-    if (this.options.maxFields !== 0) {
+    if (this.options.maxFields !== Infinity) {
       let fieldsCount = 0;
       this.on('field', () => {
         fieldsCount += 1;
@@ -621,13 +589,10 @@ class IncomingForm extends EventEmitter {
   }
 
   _maybeEnd() {
-    // console.log('ended', this.ended);
-    // console.log('_flushing', this._flushing);
-    // console.log('error', this.error);
     if (!this.ended || this._flushing || this.error) {
       return;
     }
-
+    this.req = null;
     this.emit('end');
   }
 }

src/plugins/octetstream.js

@@ -14,8 +14,6 @@ export default function plugin(formidable, options) {
   if (/octet-stream/i.test(self.headers['content-type'])) {
     init.call(self, self, options);
   }
-
-  return self;
 }
 
 // Note that it's a good practice (but it's up to you) to use the `this.options` instead

src/plugins/querystring.js

@@ -15,8 +15,6 @@ export default function plugin(formidable, options) {
   if (/urlencoded/i.test(self.headers['content-type'])) {
     init.call(self, self, options);
   }
-
-  return self;
 };
 
 // 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

@@ -60,10 +60,6 @@ test('should call 3 custom and 1 builtin plugins, when .parse() is called', asyn
       ctx.__pluginsCount = ctx.__pluginsCount || 0;
       ctx.__pluginsCount += 1;
     });
-    form.on('pluginsResults', (results) => {
-      expect(results.length).toBe(4);
-      ctx.__pluginsResults = true;
-    });
     form.on('end', () => {
       ctx.__ends = 1;
       expect(ctx.__customPlugin1).toBe(111);
@@ -117,9 +113,6 @@ test('.parse throw error when some plugin fail', async () => {
         ctx.__pluginsCount = ctx.__pluginsCount || 0;
         ctx.__pluginsCount += 1;
       });
-      form.on('pluginsResults', () => {
-        throw new Error('should not be called');
-      });
 
       form.once('error', () => {
         throw new Error('error event should not be fired when plugin throw');