Skip to content

docs(core): ctx.http() returns the raw fetch Response, not a Response<T> wrapper - #15

Open
PerryLink wants to merge 1 commit into
cordiverse:mainfrom
PerryLink:docs/ctx-http-response
Open

docs(core): ctx.http() returns the raw fetch Response, not a Response<T> wrapper#15
PerryLink wants to merge 1 commit into
cordiverse:mainfrom
PerryLink:docs/ctx-http-response

Conversation

@PerryLink

Copy link
Copy Markdown

packages/core/readme.md documents a Response<T> wrapper with a data field that ctx.http() has not returned since 736e7d9 ("refa: ctx.http() does not validate status"). That commit switched the callable to the raw fetch Response but did not touch this readme, so the first snippet a new user copies destructures a property that is never set.

Read at 401ff63.

What did not match

readme source
readme.md:22 destructures { status, data } from ctx.http(...) the callable resolves to the Response from this.undici.fetch(...) (src/index.ts:407-420), tagged with a config symbol - there is no data
readme.md:33 types it Promise<Response<T>> (url, config?): Promise<Response> (src/index.ts:165), the global fetch Response
readme.md:173-180 declares interface Response<T> { status; statusText; headers; data } nothing produces that shape. Http.Response<T> (src/index.ts:135-141) has no reference anywhere in the repo outside its own declaration
readme.md:196 calls response.data "The decoded response body" decoding happens in _decode() (src/index.ts:426-447), called by get/delete/post/put/patch (:193-205) and head (:449-452), which resolve to the decoded value itself
readme.md:83 says the ws package polyfills WebSocket the implementation uses this.undici.WebSocket (src/index.ts:471), and no manifest in the repo declares ws

Changes (one file, docs only)

  • Usage - read status and the body off the Response, with the verb helpers shown separately. The old example also declared const data twice, which does not compile.
  • http(url, config?) - Promise<Response<T>> -> Promise<Response>, plus a sentence on what that means.
  • ### Response - drop the interface Response<T> block, keep the three fetch property entries (their MDN links were already correct), and add a short Decoding table for the verb helpers, which is where the wrapper's data semantics actually live today.
  • http.ws() - state where the WebSocket implementation comes from.

One question this leaves open. Http.Response<T> is still exported from src/index.ts:135-141 and still has no producer. Removing it would be a type-level break for anyone importing it, so this PR does not touch it - the readme just stops documenting it. If the wrapper is meant to come back instead, src/index.ts:165 is the line to change and this change is the wrong direction.

Two things I noticed but left out, to keep this to one subject - happy to send either as a separate PR:

  • the ResponseTypes block in this readme is missing headers, which is the entry http.head() relies on (src/index.ts:100).
  • the Request1 / Request2 overloads here omit the responseType: Decoder<T> overload (src/index.ts:105, :111).

Closes #14

The Usage example and the Response section both documented a `Response<T>`
wrapper with a `data` field. The callable has not returned that shape since
736e7d9 ("refa: ctx.http() does not validate status"), which switched it to
return the raw fetch `Response` but did not touch this readme.

- Usage: the example destructured `data` from a call that has no such property,
  and declared `const data` twice, which does not compile. It now reads the
  status and the body off the `Response` directly.
- `http(url, config?)`: `Promise<Response<T>>` -> `Promise<Response>`.
- `### Response`: drop the `interface Response<T>` block, document that the body
  is unread, and add how the verb helpers decode it.
- `http.ws()`: the note said the `ws` package polyfills `WebSocket`; the
  implementation uses `this.undici.WebSocket`, and no manifest here declares `ws`.

Refs cordiverse#14
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.

README documents a Response wrapper that ctx.http() no longer returns

1 participant