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

Add body getter and support multipart/form-data enctype #3

Open
wants to merge 4 commits into
base: master
Choose a base branch
from

Conversation

elquimista
Copy link

This PR supports req.body getter. E.g. I was using jquery-ujs for an anchor tag with data-method="delete". Since jquery-ujs is originally designed for rails, it used form body to send hidden input field value with name of _method. And this obviously doesn't get passed query getter.

@coveralls
Copy link

coveralls commented Dec 28, 2016

Coverage Status

Coverage remained the same at 100.0% when pulling 731ac2b on clthck:master into b1ca349 on koa-modules:master.

@coveralls
Copy link

Coverage Status

Coverage remained the same at 100.0% when pulling daec4cd on clthck:master into b1ca349 on koa-modules:master.

1 similar comment
@coveralls
Copy link

Coverage Status

Coverage remained the same at 100.0% when pulling daec4cd on clthck:master into b1ca349 on koa-modules:master.

@elquimista elquimista changed the title Feat: add body getter Add body getter and support multipart/form-data enctype Dec 29, 2016
@fundon
Copy link
Member

fundon commented Jan 4, 2017

Is it better by an option?

@elquimista
Copy link
Author

I think this is improved version for sure. Original one can only handle situations where _method param comes in query string, not request body or multipart/formdata

@fundon
Copy link
Member

fundon commented Jan 4, 2017

I think we can use an option for enabling that.

@elquimista
Copy link
Author

@fundon what's your plan for this PR?

@fundon
Copy link
Member

fundon commented Mar 11, 2017

@clthck About an option?

@elquimista
Copy link
Author

I don't think providing this as an option is not appropriate. It's not something user wants to opt in/out - it should be handled by default. There's no difference in overhead or whatever if we make this feature optable.

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

Successfully merging this pull request may close these issues.

3 participants