Support context on request methods.#32
Conversation
This would typically be used by clients to implement cancellation.
|
Is there a type that would be better to use than |
ts/src/facilityCore.ts
Outdated
| /** The fetch function. */ | ||
| export interface IFetch { | ||
| (uri: string, request: IFetchRequest): Promise<IFetchResponse>; | ||
| (uri: string, request: IFetchRequest, context?: any): Promise<IFetchResponse>; |
There was a problem hiding this comment.
You could use unknown instead of any, which would force a type check/cast where it's used.
It's nice that IFetchRequest is compatible with fetch's init. Did you consider adding a signal of type any or unknown to that interface instead of separate parameter?
There was a problem hiding this comment.
You could use
unknowninstead ofany, which would force a type check/cast where it's used.
That sounds nice.
There was a problem hiding this comment.
Did you consider adding a
signalof typeanyorunknownto that interface instead of separate parameter?
I'm not intuitively excited about the idea, but I'm willing to be convinced. I like the context being ignored unless the caller explicitly does something with it. I wouldn't want people to get the idea that it "should" be an AbortSignal.
There was a problem hiding this comment.
fetch(uri, { ...request, signal: context as AbortSignal? })It seems fine.
node-fetch doesn't have signal in its RequestInit type anyway, at least in the version I happen to have in the project I'm currently working on.
2 participants
This would typically be used by clients to implement cancellation.
Fixes #17.
My npm/js/ts knowledge is so weak and obsolete that I don't know how to build and publishfacility-core(ints/src).