Skip to content

Do not convert type-check-function into array #6

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

Closed
wants to merge 3 commits into from

Conversation

Equals182
Copy link
Contributor

Allows bodyparser.text to use your custom type-check-function and resolves #5

Allows bodyparser.text to use your custom type-check-function
@fiznool
Copy link
Owner

fiznool commented Dec 9, 2019

Great catch - thank you for submitting this PR.

Can you add a test for this behaviour? I'll get it merged in once that is done.

No Content-Type header passed in request, but since type function always returns true, its parsed as xml anyway
@Equals182
Copy link
Contributor Author

I guess Travis error on #19.1 build is not related to my pull request since its about some util.promisify compatibility with environment.

@Equals182
Copy link
Contributor Author

Equals182 commented Dec 10, 2019

By the way, do we really need additional checks for not being an array or function? It seems like bodyparser.text()'s code is already taking care of function case and the rest is on type-is.

Maybe it would be better to remove that if statement entirely and just copy type option's description from bodyparser changing json to xml?

I've tested it locally and all tests passed.
Not gonna add more commits here without discussion, but you can check out this branch if you want to: https://github.com/Equals182/body-parser-xml/tree/remove-type-checking-on-type-check-option

@fiznool
Copy link
Owner

fiznool commented Mar 25, 2020

Sorry for the late reply, this one slipped through the cracks.

I'm prepping for a v2.0 release of this lib and I'd love for this to be in place. I've taken a look at the branch you linked to and it looks great - if you could get this PR synced with the latest on master and updated with the code from that branch, I'd be happy to merge in.

Otherwise, I'll pull in the commits separately as I'd like to get this out soon.

@fiznool
Copy link
Owner

fiznool commented Mar 25, 2020

I've opened #13 to handle this.

@fiznool fiznool closed this Mar 25, 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
Development

Successfully merging this pull request may close these issues.

Support bodyparser's 'type' option as function
2 participants