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

docs(core) Use response.json where possible #200

Merged
merged 1 commit into from
Apr 15, 2020
Merged

docs(core) Use response.json where possible #200

merged 1 commit into from
Apr 15, 2020

Conversation

FokkeZB
Copy link
Contributor

@FokkeZB FokkeZB commented Apr 13, 2020

I originally included some of these in #193 but figured it was better done in a separate PR.

AFAIK unless you set raw:true you can use response.json instead of z.JSON.parse(response.content) so I guess we want to promote that more simple shortcut as much as we can.

@FokkeZB FokkeZB requested review from eliangcs and xavdid April 13, 2020 09:47
@FokkeZB FokkeZB self-assigned this Apr 13, 2020
Copy link
Contributor Author

@FokkeZB FokkeZB left a comment

Choose a reason for hiding this comment

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

self-review

Comment on lines -2033 to -2036
const autoParseJson = (response, z, bundle) => {
response.json = z.JSON.parse(response.content);
return response;
};
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This example middleware was just overriding the already set response.json so IMO did not have much value as example and fortunately we have another afterResponse example.

Copy link
Member

@eliangcs eliangcs left a comment

Choose a reason for hiding this comment

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

Looks good!

@FokkeZB FokkeZB merged commit a12f24d into master Apr 15, 2020
@FokkeZB FokkeZB deleted the docs/json branch April 15, 2020 07:46
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.

2 participants