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.

Sign up for GitHub

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

Jump to bottom

fix: add generic */*+json parser #344

Merged
merged 15 commits into from
Jun 9, 2019
Merged

fix: add generic */*+json parser #344

merged 15 commits into from
Jun 9, 2019

Conversation

XVincentX
Copy link
Contributor

@XVincentX XVincentX commented Jun 6, 2019 •
edited
Loading

  1. Restores default Fastify serialisation for regular JSON
  2. Uses type-is.is to detect a flavour of JSON and parse/serialise it
  3. Removes an old negotiation test that does not really make that much sense and it's already tested inside there

Closes #341

@XVincentX XVincentX changed the title Feat/contenty type parser fix: add generic */*+json parser Jun 6, 2019
@XVincentX XVincentX force-pushed the feat/contenty-type-parser branch 2 times, most recently from 5935317 to 525ba8c Compare June 6, 2019 21:41
@XVincentX XVincentX marked this pull request as ready for review June 7, 2019 11:34
@XVincentX XVincentX force-pushed the feat/contenty-type-parser branch 2 times, most recently from 917cb3a to f3b57a2 Compare June 7, 2019 15:45
@XVincentX XVincentX force-pushed the feat/contenty-type-parser branch 3 times, most recently from 304151c to bad8d3c Compare June 7, 2019 15:52
chris-miaskowski
chris-miaskowski reviewed Jun 7, 2019
View reviewed changes
Copy link
Contributor

@chris-miaskowski chris-miaskowski left a comment

Choose a reason for hiding this comment

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

Code looks really good @XVincentX. I would like to see more edge cases tested though.
application/json
application/vnd.json
application/vnd.xml;x=json

...___headers____content-type___application_vnd.api+json____body____\_hello\__\_world\____.json Show resolved Hide resolved
philsturgeon
philsturgeon reviewed Jun 9, 2019
View reviewed changes
test-harness/index.js Outdated Show resolved Hide resolved
philsturgeon
philsturgeon previously approved these changes Jun 9, 2019
View reviewed changes
Copy link
Contributor

@philsturgeon philsturgeon left a comment

Choose a reason for hiding this comment

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

Two trivial changes that you can just do or not do, maybe just let it let leak into your next PR. :D

This is great work man, huge improvement.

@XVincentX XVincentX merged commit 9e770c7 into master Jun 9, 2019
@XVincentX XVincentX deleted the feat/contenty-type-parser branch June 9, 2019 14:07
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@philsturgeon philsturgeon philsturgeon approved these changes

@chris-miaskowski chris-miaskowski Awaiting requested review from chris-miaskowski

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Prism failing with custom Content-Type header
3 participants
@XVincentX @philsturgeon @chris-miaskowski