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

Support 'application/graphql-response+json' #1616

Merged
merged 4 commits into from
Aug 18, 2022
Merged

Conversation

ardatan
Copy link
Collaborator

@ardatan ardatan commented Aug 17, 2022

Also now the order is updated per spec.

@changeset-bot
Copy link

changeset-bot bot commented Aug 17, 2022

🦋 Changeset detected

Latest commit: df1fdea

The changes in this PR will be included in the next version bump.

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

@github-actions
Copy link
Contributor

The latest changes of this PR are not available as canary, since there are no linked changesets for this PR.

@github-actions
Copy link
Contributor

github-actions bot commented Aug 17, 2022

❌ Benchmark Failed

Failed assertions detected

If the performance regression is expected, please increase the failing threshold.

     ✗ no_errors
      ↳  0% — ✓ 0 / ✗ 86060
     ✗ expected_result
      ↳  0% — ✓ 0 / ✗ 86060

     checks.....................: 0.00%   ✓ 0           ✗ 172120
     data_received..............: 23 MB   752 kB/s
     data_sent..................: 9.9 MB  330 kB/s
     http_req_blocked...........: avg=1.14µs   min=700ns    med=1µs      max=2.34ms  p(90)=1.4µs   p(95)=1.7µs   
     http_req_connecting........: avg=25ns     min=0s       med=0s       max=2.18ms  p(90)=0s      p(95)=0s      
   ✓ http_req_duration..........: avg=281.22µs min=209.49µs med=261.59µs max=16.48ms p(90)=310.5µs p(95)=350.99µs
     http_req_failed............: 100.00% ✓ 86060       ✗ 0     
     http_req_receiving.........: avg=17.29µs  min=10.5µs   med=16.1µs   max=2.83ms  p(90)=20.7µs  p(95)=22.9µs  
     http_req_sending...........: avg=5.73µs   min=4µs      med=4.7µs    max=1.85ms  p(90)=6.2µs   p(95)=8µs     
     http_req_tls_handshaking...: avg=0s       min=0s       med=0s       max=0s      p(90)=0s      p(95)=0s      
     http_req_waiting...........: avg=258.19µs min=191.99µs med=240.29µs max=16.1ms  p(90)=282.9µs p(95)=318.4µs 
     http_reqs..................: 86060   2868.551821/s
     iteration_duration.........: avg=344.07µs min=259.4µs  med=321.6µs  max=17.96ms p(90)=381.9µs p(95)=427.89µs
     iterations.................: 86060   2868.551821/s
     vus........................: 1       min=1         max=1   
     vus_max....................: 1       min=1         max=1   

@@ -12,14 +12,18 @@ export function isRegularResult(
if (!isAsyncIterable(result)) {
const acceptHeader = request.headers.get('accept')
if (acceptHeader && !acceptHeader.includes('*/*')) {
if (acceptHeader.includes('application/json')) {
acceptHeaderByResult.set(result, 'application/json')
if (acceptHeader.includes('application/graphql-response+json')) {
Copy link
Owner

Choose a reason for hiding this comment

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

should we also keep the support for application/json here?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

We still support it but we change the order here. If accept header has the newer one with application/json, we choose the newer one.

@ardatan ardatan merged commit 1d5cde9 into v3 Aug 18, 2022
@ardatan ardatan deleted the support-graphql-response-json branch August 18, 2022 12:34
This was referenced Sep 21, 2022
@saihaj saihaj mentioned this pull request Oct 17, 2022
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