docs(core): ctx.http() returns the raw fetch Response, not a Response<T> wrapper - #15
Open
PerryLink wants to merge 1 commit into
Open
docs(core): ctx.http() returns the raw fetch Response, not a Response<T> wrapper#15PerryLink wants to merge 1 commit into
PerryLink wants to merge 1 commit into
Conversation
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
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
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.
packages/core/readme.mddocuments aResponse<T>wrapper with adatafield thatctx.http()has not returned since736e7d9("refa: ctx.http() does not validate status"). That commit switched the callable to the raw fetchResponsebut 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.md:22destructures{ status, data }fromctx.http(...)Responsefromthis.undici.fetch(...)(src/index.ts:407-420), tagged with a config symbol - there is nodatareadme.md:33types itPromise<Response<T>>(url, config?): Promise<Response>(src/index.ts:165), the global fetchResponsereadme.md:173-180declaresinterface Response<T> { status; statusText; headers; data }Http.Response<T>(src/index.ts:135-141) has no reference anywhere in the repo outside its own declarationreadme.md:196callsresponse.data"The decoded response body"_decode()(src/index.ts:426-447), called byget/delete/post/put/patch(:193-205) andhead(:449-452), which resolve to the decoded value itselfreadme.md:83says thewspackage polyfillsWebSocketthis.undici.WebSocket(src/index.ts:471), and no manifest in the repo declareswsChanges (one file, docs only)
statusand the body off theResponse, with the verb helpers shown separately. The old example also declaredconst datatwice, which does not compile.http(url, config?)-Promise<Response<T>>->Promise<Response>, plus a sentence on what that means.### Response- drop theinterface Response<T>block, keep the three fetch property entries (their MDN links were already correct), and add a shortDecodingtable for the verb helpers, which is where the wrapper'sdatasemantics actually live today.http.ws()- state where the WebSocket implementation comes from.One question this leaves open.
Http.Response<T>is still exported fromsrc/index.ts:135-141and 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:165is 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:
ResponseTypesblock in this readme is missingheaders, which is the entryhttp.head()relies on (src/index.ts:100).Request1/Request2overloads here omit theresponseType: Decoder<T>overload (src/index.ts:105,:111).Closes #14