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

A few ideas: options.extendTypes.custom, options.handler, split buffer thingy? #52

Closed
tunnckoCore opened this issue Jul 9, 2016 · 6 comments
Milestone

Comments

@tunnckoCore
Copy link
Member

tunnckoCore commented Jul 9, 2016

Get an ideas.

split buffer thingy

Currently (in v2.0):

  • if you wanna buffer body, you should pass options.buffer: true and trick the things through options.extendTypes.text (because that's the only way to use the this.request.buffer parser to parse buffers). you will receive it in this.request.body
  • If you wanna limit the buffer size you should pass options.textLimit or options.bufferLimit

I'm asking myself, isn't it will be better if there's separate if for options.extendTypes.buffer and try to parse it separate from text? So i came to the next idea for extendTypes.custom

options.extendTypes.custom and options.handler

So, to be able hack the things legally. Meaning if you wanna parse foo/bar-x type, for instance, you will pass that type to options.extendTypes.custom: ['foo/bar-x'], then options.handler.call(ctx, ctx, options, next) will be called.

related: #22 and #31

This will give you the freedom and ability to do whatever you want:

  1. access the this context (ctx)
  2. access the original options (options)
  3. to use all of the parsers - ctx.request.json, ctx.request.urlencoded, ctx.request.multipart, ctx.request.text and ctx.request.buffer
  4. and what your mind can think
@tunnckoCore
Copy link
Member Author

@ztrange
Copy link

ztrange commented Sep 2, 2016

It sounds cool!

On an unrelated matter, since you stopped using this.body, I think the major version (semver) should be bumped, since it is a breaking change.

@internalfx
Copy link

@ztrange I agree.

@tunnckoCore I think these are great additions. However, they are very likely to break existing apps.

@tunnckoCore
Copy link
Member Author

@ztrange, i'm not sure enough. in one side, it is bugfix to wrong behaviour - it was total mistake, in another yea it is breaking change, but dont thinking for major bump, but for minor definitely.

about the ideas, i'll try to push them today.

@tunnckoCore
Copy link
Member Author

but yea maybe you are right, i'll add these features and bump major directly

@tunnckoCore
Copy link
Member Author

All is implemented as suggested.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants