docs: add QUERY method docs#1447
Merged
Merged
Conversation
Cross-cutting audits and docs for the HTTP QUERY method (RFC 10008), completing the QUERY method support tracked in #1441. - cors: document that `QUERY` is not CORS-safelisted, so explicit `methods` allowlists must include `"QUERY"` (no logic change — CORS is already method-agnostic). - tests: regression coverage that a `QUERY` preflight works (wildcard and explicit allowlist) and that `handleCacheHeaders` returns `304` for `QUERY` on `If-None-Match` / `If-Modified-Since`. - docs: add an HTTP `QUERY` method section to the routing guide covering `app.query()` and the read-with-a-body use case. Co-Authored-By: Claude Opus 4.8 <[email protected]>
📝 WalkthroughWalkthroughDocumentation, example code, and tests are updated for HTTP QUERY support. The routing guide and example pages describe QUERY usage and helpers, the runnable example switches to ChangesQUERY method docs, example, and tests
Estimated code review effort: 2 (Simple) | ~10 minutes Possibly related issues
Suggested reviewers: Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
pi0
changed the title
QUERY method docs
- examples/query.mjs imported `setAcceptQuery`, but the util shipped as `appendAcceptQuery` (#1446) — the example failed to run. Verified the fixed example end-to-end (QUERY, 415, cacheable GET follow-up). - docs: add an "HTTP QUERY Method" example page walking through `app.query()`, `appendAcceptQuery`, `requireContentType`, and the cacheable-GET / `Content-Location` pattern; link it from the routing guide and the examples index. Co-Authored-By: Claude Opus 4.8 <[email protected]>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
PR 4 — Cross-cutting audits + docs from the HTTP
QUERYmethod proposal (#1441). Completes the checklist now that core support (#1445) and theAccept-Query/requireContentTypeutils (#1446) have landed.The audit finding is that no core logic change is needed — CORS and cache handling are already method-agnostic. This PR locks that behavior in with regression tests, documents the one real gotcha (CORS preflight), fixes a broken example, and adds an example doc page.
Audit results
handleCors/isPreflightRequestnever inspect the specific requested method;access-control-allow-methodsjust echoesmethods. Default"*"already permitsQUERY.QUERYis not CORS-safelisted, so explicitmethodsallowlists must include"QUERY".handleCacheHeadershas no method guard — keys offIf-None-Match/If-Modified-Sinceregardless of method.QUERYalready gets304.QUERYinPayloadMethods, forwarded with body).Changes
src/utils/cors.ts— clarify themethodsoption jsdoc: include"QUERY"in explicit allowlists since browsers preflight it.test/query.test.ts— matrix regression tests:QUERYpreflight (wildcard + explicit allowlist) and304conditional caching (If-None-Match/If-Modified-Since).examples/query.mjs— 🐛 bug fix: the example importedsetAcceptQuery, but the util shipped asappendAcceptQuery(feat: addrequireContentTypeandappendAcceptQueryutils #1446), so it failed to run. Corrected and verified end-to-end (QUERY→200+Content-Location,415on unsupported type, cacheableGETfollow-up).docs/1.guide/1.basics/2.routing.md— new "HTTPQUERYmethod" section coveringapp.query(), the read-with-a-body use case, body-size limits, and the CORS/cache/proxy parity notes.docs/4.examples/handle-query.md— new example page walking throughapp.query(),appendAcceptQuery,requireContentType, and the cacheable-GET/Content-Locationpattern; linked from the routing guide and the examples index.Testing
pnpm vitest run test/query.test.ts→ 38 passed (19 × web/node matrix)pnpm vitest run test/unit/cors.test.ts test/utils.test.ts test/proxy.test.ts→ all greenpnpm lint→ cleannode examples/query.mjsand exercised every path withcurlCloses the PR 4 checklist in #1441.
🤖 Generated with Claude Code
Summary by CodeRabbit
QUERYmethod, including handler registration, reading request bodies, and enforcing body-size limits.QUERYcaching behavior, proxy forwarding expectations, and CORS/preflight implications.QUERYis not CORS-safelisted and must be explicitly included in allowed methods.QUERYCORS preflight and conditional caching behavior.