refactor: remove overly strict context types #108
Conversation
Other minor type changes: add types for overrides, fix broken payload type BREAKING CHANGE: The TypeScript types ServiceRequest and ServiceContext have been removed as service-client is not concerned with the shape of the context passed to it. Use types relevant to the context that you provide instead.
| timeout?: number; | ||
| }; | ||
|
|
||
| export interface ServiceRequest { |
There was a problem hiding this comment.
This library has no need for any specificity in this type, used for the context option, which is passed through to plugins without being accessed by this library
|
|
||
| export interface ServiceRequest { | ||
| headers: Headers; | ||
| logger: Logger; |
There was a problem hiding this comment.
This would require use of pino, which is not a requirement to use service-client
| isAuthenticated: boolean; | ||
| credentials: { | ||
| apiToken: string; | ||
| principalToken: string; |
There was a problem hiding this comment.
This is a proprietary auth credential structure
| } | ||
|
|
||
| export type ServiceContext = { | ||
| dataSources: {[serviceClient: string]: ClientInstance}; |
There was a problem hiding this comment.
This would require use of graphql-component which is not a requirement to use this library
| }; | ||
| } | ||
|
|
||
| export type ServiceContext = { |
| }; | ||
| export type ServiceConfig = { | ||
| protocol?: string; | ||
| hostname?: string; |
There was a problem hiding this comment.
Removed comments and added types for options that were previously missed like hostname
|
Thanks for your PR. This will be scheduled for a larger release cycle, tentatively targeting early Q3, due to the breaking nature of this change. |
2 participants
Description
The TypeScript types for the context option added in #61 are overly strict. The context option is merely passed through to plugins. This library makes no use of it directly, so the typings do not need to be strict.
Additionally, the overrides structure is currently set to any non-nullable value, so some types for that are added here.
Additionally, previously missed options for the client config have been added to that type.
Motivation and Context
Consumers of this library who use context and TypeScript are forced to either unnecessarily conform to the types defined here (which depend on hapi, pino, and a proprietary auth structure) or disable TypeScript by other means.
How Has This Been Tested?
Used the new definitions in a private project that uses service-client and TypeScript compilation was still successful.
Types of changes
Checklist: