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(matchers): Add custom body type matcher #88

Merged
merged 1 commit into from
Jul 14, 2021

Conversation

cakejelly
Copy link
Contributor

PR to add the custom body type matcher that was proposed in #87

The logic is pretty simple - If a user passes a non-empty mime type to the BodyType function, then the Content-Type header on the http request must be the same for there to be a match. If there's no mime type passed then it falls back to the the existing logic of checking against the hardcoded list of supported mime types.

One thing I'm a bit unsure of - I discovered MatchType and now I feel like adding this additional method will be confusing to people, since there will be two separate methods for matching based on the Content-Type header but the behaviour is different.

@h2non Any thoughts on this?

@h2non
Copy link
Owner

h2non commented Jul 5, 2021

True. It is somehow redundant and can lead to confusion. I believe we should keep MatchType and simply pass the MIME type to match to supportedType function.

@cakejelly cakejelly force-pushed the support-for-custom-body-types branch from f8b0b6c to ce9d2d4 Compare July 6, 2021 15:48
@cakejelly
Copy link
Contributor Author

@h2non makes sense and agreed, I think it's much cleaner to just keep MatchType. Pushed a new commit with your suggestion, please take another look when you get the chance 🙃 thanks.

@h2non h2non merged commit 48ac21d into h2non:master Jul 14, 2021
@h2non
Copy link
Owner

h2non commented Jul 14, 2021

Thank you!

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.

None yet

2 participants