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.

Sign up for GitHub

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

Jump to bottom

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 •
edited
Loading

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.

@tunnckoCore
feat: introduce Plugins API, fix tests
5c5ca3b 
Signed-off-by: Charlike Mike Reagent <opensource@tunnckocore.com>
@tunnckoCore
chore: update changelog
0aab094 
Signed-off-by: Charlike Mike Reagent <opensource@tunnckocore.com>
@tunnckoCore
chore: lgtm tweaks?
7568d08 
Signed-off-by: Charlike Mike Reagent <opensource@tunnckocore.com>
GrosSacASac
GrosSacASac previously approved these changes Jan 29, 2020
View reviewed changes
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

@tunnckoCore
start plugins tests; update exports
7fed544 
Signed-off-by: Charlike Mike Reagent <opensource@tunnckocore.com>
@tunnckoCore
chore(custom-plugins-tests): make it really passing
aa0cc85 
Signed-off-by: Charlike Mike Reagent <opensource@tunnckocore.com>
@tunnckoCore
chore: more tests; add nyc test coverage
308333d 
Signed-off-by: Charlike Mike Reagent <opensource@tunnckocore.com>
@tunnckoCore
Copy link
Member Author

tunnckoCore commented Jan 29, 2020 •
edited
Loading

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 )

@tunnckoCore
chore: move to GitHub Actions CI
f9d8cd4 
Signed-off-by: Charlike Mike Reagent <opensource@tunnckocore.com>
@allcontributors allcontributors bot mentioned this pull request Jan 29, 2020
Closed
@node-formidable node-formidable deleted a comment from allcontributors bot Jan 29, 2020
@allcontributors allcontributors bot mentioned this pull request Jan 29, 2020
Closed
@node-formidable node-formidable deleted a comment from allcontributors bot Jan 29, 2020
@allcontributors allcontributors bot mentioned this pull request Jan 29, 2020
Closed
GrosSacASac
GrosSacASac previously approved these changes Jan 29, 2020
View reviewed changes
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

@allcontributors allcontributors bot mentioned this pull request Jan 29, 2020
Merged
@node-formidable node-formidable deleted a comment from lgtm-com bot Jan 29, 2020
@node-formidable node-formidable deleted a comment from allcontributors bot Jan 29, 2020
This was referenced Jan 29, 2020
Closed
Closed
Closed
Open
Closed
Closed
@tunnckoCore @masterkain
fix: respect form hash option on incoming octect/stream requests
a84a0f1 
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

@tunnckoCore
Merge branch 'master' into plugins-api
8120b6b
@tunnckoCore
Merge branch 'master' into plugins-api
2c399be
@tunnckoCore
chore: sync & tweak
d6cd110 
Signed-off-by: Charlike Mike Reagent <opensource@tunnckocore.com>
@tunnckoCore
fix: check strictly for multipart/form-data in multipart parser
2c35e72 
Signed-off-by: Charlike Mike Reagent <opensource@tunnckocore.com>
@tunnckoCore
Merge branch 'master' into plugins-api
6d47def
@tunnckoCore
chore: publish dev.2
3ebbbd0 
Signed-off-by: Charlike Mike Reagent <opensource@tunnckocore.com>
@tunnckoCore
chore: merge
e02c69c 
Signed-off-by: Charlike Mike Reagent <opensource@tunnckocore.com>
@tunnckoCore
fix: tests updates
3e3a27a 
Signed-off-by: Charlike Mike Reagent <opensource@tunnckocore.com>
@tunnckoCore
Merge branch 'master' into plugins-api
019937e
@tunnckoCore
fix: it seems we support multipart/related too
bc34459 
Signed-off-by: Charlike Mike Reagent <opensource@tunnckocore.com>
@tunnckoCore
Copy link
Member Author

tunnckoCore commented Jan 31, 2020 •
edited
Loading

@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 tunnckoCore mentioned this pull request Jan 31, 2020
Closed
@tunnckoCore
Copy link
Member Author

tunnckoCore commented Jan 31, 2020 •
edited
Loading

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
View reviewed changes
@tunnckoCore
chore: download badges
282971d 
Signed-off-by: Charlike Mike Reagent <opensource@tunnckocore.com>
This was referenced Feb 6, 2020
Closed
Closed
@tunnckoCore tunnckoCore mentioned this pull request Feb 10, 2020
Closed
@tunnckoCore
Merge branch 'master' into plugins-api
db1d69a
@tunnckoCore
Update package.json
81c79fd
@tunnckoCore
chore: sync
e531213 
Signed-off-by: Charlike Mike Reagent <opensource@tunnckocore.com>
@tunnckoCore
chore: update npm funding link, tag to canary [skip ci]
5b813a5 
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
Reviewers

@GrosSacASac GrosSacASac GrosSacASac left review comments

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
2 participants
@tunnckoCore @GrosSacASac