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: allow for custom response parser with parseResponse #16

Merged
merged 12 commits into from
Oct 22, 2021

Conversation

chrstnfrrs
Copy link
Contributor

Closes #13

Description

This PR allows for automatic response parsing to be disabled. This can be accomplished through passing { parse: false } into fetch options.

This PR also allows for custom parsing functions to be used. For example, JSON.parse can be used by passing { parse: JSON.parse } into fetch options.

Parsing still defaults to destr.

@Atinux Atinux changed the title Issue 13: Allow for custom parse feat: Allow for custom parse Sep 24, 2021
@Atinux
Copy link
Member

Atinux commented Sep 24, 2021

Thank you for the PR @chrstnfrrs

Do you mind adding test cases in https://github.com/unjs/ohmyfetch/blob/main/test/index.test.ts?

@codecov-commenter
Copy link

codecov-commenter commented Sep 24, 2021

Codecov Report

Merging #16 (3982d0d) into main (bb975a1) will increase coverage by 0.13%.
The diff coverage is 100.00%.

Impacted file tree graph

@@            Coverage Diff             @@
##             main      #16      +/-   ##
==========================================
+ Coverage   98.30%   98.43%   +0.13%     
==========================================
  Files           4        4              
  Lines          59       64       +5     
  Branches       13       18       +5     
==========================================
+ Hits           58       63       +5     
  Misses          1        1              
Impacted Files Coverage Δ
src/fetch.ts 97.56% <100.00%> (+0.33%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update bb975a1...3982d0d. Read the comment docs.

@chrstnfrrs
Copy link
Contributor Author

@Atinux I added two tests. I'm not super satisfied with the current test for the { parse: false } option due to the assertion not being on the piece of code under test. I can try to improve this over the weekend, but feel free to merge as-is.

@Atinux Atinux requested a review from pi0 September 27, 2021 09:37
Copy link
Member

@Atinux Atinux left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Will wait for Pooya to see if he spots any issue in the code

@pi0 pi0 changed the title feat: Allow for custom parse feat: allow for custom response parser with parseResponse Oct 22, 2021
Copy link
Member

@pi0 pi0 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for contributing 💚 I have made a few adjustments to API to simplify

@pi0 pi0 merged commit 463ced6 into unjs:main Oct 22, 2021
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.

Make parse optional
4 participants