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

feat: introduce Plugins API, fix tests #545

Merged
merged 28 commits into from Feb 12, 2020
Merged

feat: introduce Plugins API, fix tests #545

merged 28 commits into from Feb 12, 2020

Conversation

tunnckoCore
Copy link
Member

@tunnckoCore tunnckoCore commented Jan 29, 2020

Why?

Better separation. Easier future testing. More extensible so there can be developed third-party plugins (for example more advanced QueryString parser). We now can unit test each separate plugin/parser.

Plugins API

add form.use(plugin: Plugin) method

A method that allows you to extend the Formidable library. By default we include 4 plugins,
which esentially are adapters to plug the different built-in parsers.

The plugins added by this method are always enabled.

See src/plugins/ for more detailed look on default plugins.

The plugin param has such signature:

function(formidable: Formidable, options: Options): void;

The architecture is simple. The plugin is a function that is passed with
the Formidable instance (the form across the README examples) and the options.

Note: the plugin function's this context is also the same instance.

const formidable = require('formidable');
const form = formidable({ keepExtensions: true });
form.use((self, options) => {
  // self === this === form
  console.log('woohoo, custom plugin');
  // do your stuff; check `src/plugins` for inspiration
});
form.parse(req, (error, fields, files) => {
  console.log('done!');
});

Important to note, is that inside plugin this.options, self.options and options
MAY or MAY NOT be the same. General best practice is to always use the this, so you can
later test your plugin independently and more easily.

If you want to disable some parsing capabilities of Formidable, you can disable the plugin
which corresponds to the parser. For example, if you want to disable multipart parsing
(so the src/parsers/Multipart.js which is used in src/plugins/multipart.js), then you can remove it from
the options.enabledPlugins, like so

const { Formidable } = require('formidable');
const form = new Formidable({
  hash: 'sha1',
  enabledPlugins: ['octetstream', 'querystring', 'json'],
});

Be aware that the order MAY be important too. The names corresponds 1:1
to files in src/plugins/ folder.

Pull requests for new built-in plugins MAY be accepted - for example,
more advanced querystring parser. Add your plugin as a new file
in src/plugins/ folder (lowercased) and follow how the other plugins are made.

Signed-off-by: Charlike Mike Reagent <opensource@tunnckocore.com>
Signed-off-by: Charlike Mike Reagent <opensource@tunnckocore.com>
Signed-off-by: Charlike Mike Reagent <opensource@tunnckocore.com>
GrosSacASac
GrosSacASac previously approved these changes Jan 29, 2020
Copy link
Contributor

@GrosSacASac GrosSacASac left a comment

Choose a reason for hiding this comment

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

Nice

Signed-off-by: Charlike Mike Reagent <opensource@tunnckocore.com>
Signed-off-by: Charlike Mike Reagent <opensource@tunnckocore.com>
Signed-off-by: Charlike Mike Reagent <opensource@tunnckocore.com>
@tunnckoCore
Copy link
Member Author

tunnckoCore commented Jan 29, 2020

Not bad.

-----------------|---------|----------|---------|---------|---------------------------------------------------
File             | % Stmts | % Branch | % Funcs | % Lines | Uncovered Line #s                                 
-----------------|---------|----------|---------|---------|---------------------------------------------------
All files        |    87.6 |    78.38 |   85.87 |   87.66 |                                                   
 src             |   83.74 |    70.71 |   87.23 |   83.54 |                                                   
  File.js        |   89.47 |       75 |    87.5 |   89.47 | 50,56,65,66                                       
  Formidable.js  |   82.09 |    70.11 |   86.84 |   81.91 | ...76,304,309,337,338,352,353,365,375,376,379,407 
  index.js       |     100 |      100 |     100 |     100 |                                                   
 src/parsers     |   89.34 |    82.41 |   81.82 |   89.67 |                                                   
  Dummy.js       |    62.5 |        0 |      50 |    62.5 | 15-17                                             
  JSON.js        |    87.5 |        0 |     100 |    87.5 | 27,28                                             
  Multipart.js   |   91.75 |    84.62 |   83.33 |   92.19 | ...57,188,197,216,222,262,282,285,309,326,333-335 
  OctetStream.js |     100 |        0 |     100 |     100 | 6                                                 
  Querystring.js |   68.75 |      100 |   66.67 |   68.75 | 26,30,31,36,37                                    
  index.js       |     100 |      100 |     100 |     100 |                                                   
 src/plugins     |   91.37 |    84.62 |   86.96 |   91.37 |                                                   
  index.js       |     100 |      100 |     100 |     100 |                                                   
  json.js        |     100 |      100 |     100 |     100 |                                                   
  multipart.js   |    98.7 |     88.1 |     100 |    98.7 | 24                                                
  octetstream.js |   93.94 |    66.67 |     100 |   93.94 | 54,73                                             
  querystring.js |      25 |       50 |      25 |      25 | 16,24,26,28,29,32-34,37                           
-----------------|---------|----------|---------|---------|---------------------------------------------------
=============================== Coverage summary ===============================
Statements   : 87.6% ( 551/629 )
Branches     : 78.38% ( 203/259 )
Functions    : 85.87% ( 79/92 )
Lines        : 87.66% ( 547/624 )

Signed-off-by: Charlike Mike Reagent <opensource@tunnckocore.com>
GrosSacASac
GrosSacASac previously approved these changes Jan 29, 2020
Copy link
Contributor

@GrosSacASac GrosSacASac left a comment

Choose a reason for hiding this comment

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

Thank you for adding me as a contributor

@node-formidable node-formidable deleted a comment from allcontributors bot Jan 29, 2020
@tunnckoCore
Copy link
Member Author

@GrosSacASac, Thank you for adding me as a contributor

Sure, of course ; )


trying to sync the bot and GH actions... ;d

@all-contributors please add @tunnckoCore for idea and doc

close #407

Co-authored-by: Claudio Poli <masterkain@gmail.com>
Signed-off-by: Charlike Mike Reagent <opensource@tunnckocore.com>
@tunnckoCore
Copy link
Member Author

Happy birthday to me 🎉

Available through formidable@2.0.0-dev.20200130.1 or formidable@dev

Signed-off-by: Charlike Mike Reagent <opensource@tunnckocore.com>
Signed-off-by: Charlike Mike Reagent <opensource@tunnckocore.com>
Signed-off-by: Charlike Mike Reagent <opensource@tunnckocore.com>
Signed-off-by: Charlike Mike Reagent <opensource@tunnckocore.com>
Signed-off-by: Charlike Mike Reagent <opensource@tunnckocore.com>
Signed-off-by: Charlike Mike Reagent <opensource@tunnckocore.com>
@tunnckoCore
Copy link
Member Author

tunnckoCore commented Jan 31, 2020

@GrosSacASac , it's generally ready. There are two test suites - one is the old, second is with Jest (the test/unit). Both are passing, but we need to continue converting them. But that could be after v2.

After merging this a lot will be closed too, so v2 is really near, just need to add a few easy fixes and features like #154 and #153

I hope you are okay with the ton of stuff updates on the master and this PR.
I really like the whole thing now. 🎉 ❤️ I even have one more idea in the pocket.

@tunnckoCore
Copy link
Member Author

tunnckoCore commented Jan 31, 2020

The only thing I'm wondering is when and why the performance dropped to 1500mb/sec, it was up to 2500mb/sec for some time. And I didn't touched the multipart parser... Hm... 🤔

GrosSacASac
GrosSacASac previously approved these changes Feb 4, 2020
Signed-off-by: Charlike Mike Reagent <opensource@tunnckocore.com>
Signed-off-by: Charlike Mike Reagent <opensource@tunnckocore.com>
Signed-off-by: Charlike Mike Reagent <opensource@tunnckocore.com>
@tunnckoCore tunnckoCore merged commit 037a738 into master Feb 12, 2020
@tunnckoCore tunnckoCore deleted the plugins-api branch February 12, 2020 14:01
@pr-triage pr-triage bot added the PR: merged label Feb 12, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
2 participants