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

fix: Update koa's integration guide for body parsing #3670

Merged
merged 2 commits into from
Jan 16, 2025

Conversation

aryascripts
Copy link
Contributor

@aryascripts aryascripts commented Jan 16, 2025

Why

I saw that these docs were updated recently, and this is also something we were struggling with in one of our applications. I noticed that we do need to use ctx.request, but the documentation was a bit confusing - it mentioned:

Make sure it is not ctx.request not ctx.req

(two negatives)
koa-body (most popular) by default, it does:

  • patchNode: false = not place parsed body in ctx.req (Node.js IncomingMessage type)
  • patchKoa: true = does place parsed body in ctx.request (Koa's Request, abstraction of Node's request)

From the investigation, it's known that we must use the ctx.request object that does have the parsed body.

Without koa-body, either one works.

What

  • Clarifying the docs about usage with koa body parsers

Copy link

changeset-bot bot commented Jan 16, 2025

⚠️ No Changeset found

Latest commit: 1f0d60f

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

This PR includes no changesets

When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

Click here to learn what changesets are, and how to add one.

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

@ardatan
Copy link
Collaborator

ardatan commented Jan 16, 2025

Thanks @aryascripts !

@ardatan ardatan merged commit 537cf57 into dotansimha:main Jan 16, 2025
5 checks passed
@aryascripts
Copy link
Contributor Author

@ardatan NP! I wasn't sure if the documentation would be released with no changeset, but let me know if anything is needed for that :)

@aryascripts aryascripts deleted the yoga-koa-integration-docs branch January 16, 2025 15:47
Copy link
Contributor

✅ Benchmark Results

     ✓ no_errors{mode:graphql}
     ✓ expected_result{mode:graphql}
     ✓ no_errors{mode:graphql-jit}
     ✓ expected_result{mode:graphql-jit}
     ✓ no_errors{mode:graphql-response-cache}
     ✓ expected_result{mode:graphql-response-cache}
     ✓ no_errors{mode:graphql-no-parse-validate-cache}
     ✓ expected_result{mode:graphql-no-parse-validate-cache}
     ✓ no_errors{mode:uws}
     ✓ expected_result{mode:uws}

     checks.......................................: 100.00% ✓ 518184      ✗ 0     
     data_received................................: 2.1 GB  14 MB/s
     data_sent....................................: 104 MB  694 kB/s
     http_req_blocked.............................: avg=1.6µs    min=1.05µs   med=1.38µs   max=3.38ms   p(90)=2.05µs   p(95)=2.24µs  
     http_req_connecting..........................: avg=2ns      min=0s       med=0s       max=151.78µs p(90)=0s       p(95)=0s      
     http_req_duration............................: avg=358.39µs min=216.61µs med=325.5µs  max=15ms     p(90)=465.74µs p(95)=487.8µs 
       { expected_response:true }.................: avg=358.39µs min=216.61µs med=325.5µs  max=15ms     p(90)=465.74µs p(95)=487.8µs 
     ✓ { mode:graphql-jit }.......................: avg=288.26µs min=216.61µs med=271.11µs max=15ms     p(90)=302.61µs p(95)=319.07µs
     ✓ { mode:graphql-no-parse-validate-cache }...: avg=490.69µs min=402.73µs med=466.88µs max=6.24ms   p(90)=507.46µs p(95)=554.35µs
     ✓ { mode:graphql-response-cache }............: avg=342.77µs min=262.32µs med=325.7µs  max=6.42ms   p(90)=355.25µs p(95)=366µs   
     ✓ { mode:graphql }...........................: avg=365.95µs min=279.51µs med=336.55µs max=13.98ms  p(90)=398.38µs p(95)=449.79µs
     ✓ { mode:uws }...............................: avg=340.29µs min=265.8µs  med=321.73µs max=6.03ms   p(90)=356.28µs p(95)=377.44µs
     http_req_failed..............................: 0.00%   ✓ 0           ✗ 259092
     http_req_receiving...........................: avg=33.5µs   min=16.31µs  med=33.1µs   max=3.07ms   p(90)=39.97µs  p(95)=42.62µs 
     http_req_sending.............................: avg=9.05µs   min=6.06µs   med=8.08µs   max=384.18µs p(90)=11.41µs  p(95)=12.61µs 
     http_req_tls_handshaking.....................: avg=0s       min=0s       med=0s       max=0s       p(90)=0s       p(95)=0s      
     http_req_waiting.............................: avg=315.84µs min=186.13µs med=283.77µs max=14.89ms  p(90)=422.86µs p(95)=443.48µs
     http_reqs....................................: 259092  1727.259756/s
     iteration_duration...........................: avg=573.86µs min=387.41µs med=537.12µs max=15.5ms   p(90)=684.64µs p(95)=710.83µs
     iterations...................................: 259092  1727.259756/s
     vus..........................................: 1       min=1         max=1   
     vus_max......................................: 2       min=2         max=2   

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