graphql-hive / graphql-hive/envelop
Improve types for `useErrorHandler` plugin
- Dominant language
- No language data
- Stars
- 827
- Forks
- 132
- PR merge metrics
- No merged PRs in 30d
Description
**Is your feature request related to a problem? Please describe.**
The `context` property passed as the payload to `useErrorHandler` callbacks is typed as `Readonly`: https://github.com/n1ru4l/envelop/blob/b7d0a94d9dd5bca6339fdbd9c6652877e500a114/packages/core/src/plugins/use-error-handler.ts#L11
I'm trying to use this plugin and this type doesn't seem to be very useful. Given that the `useErrorHandler` function takes a generic type for the context, I would expect the `ErrorHandler` type to also be generic instead of just using `DefaultContext`.
Additionally, depending on the phase, it seems like the type is actually different. The context value is passed to the error handler callback in the parse and validate phases, but in the execution phase it's passing a `TypedExecutionArgs`: https://github.com/n1ru4l/envelop/blob/b7d0a94d9dd5bca6339fdbd9c6652877e500a114/packages/core/src/plugins/use-error-handler.ts#L24. So even though the type of `context` is `Readonly`, I need to look in `context.contextValue` (untyped) to get the same `context` value as the parse and validate phases.
And in the "context" phase, the `context` value passed to the error handler callback is a `OnPluginInitEventPayload`. I don't really understand why this is being passed to error handlers - it would make more sense to me if the `context` value was pulled from this callback instead: https://github.com/n1ru4l/envelop/blob/b7d0a94d9dd5bca6339fdbd9c6652877e500a114/packages/core/src/plugins/use-error-handler.ts#L48
But I'm not sure if it's intentional that the "onPluginInit" payload is being used instead.
**Describe the solution you'd like**
Make the `ErrorHandler` type a generic type with a tagged union:
```ts
export type ErrorHandler = DefaultContext> = ({
errors,
context,
phase,
}: {
errors: readonly Error[] | readonly SerializableGraphQLErrorLike[];
} & (
| {
context: Readonly;
phase: 'parse' | 'validate';
}
| {
context: OnPluginInitEventPayload;
phase: 'context';
}
| {
context: TypedExecutionArgs;
phase: 'execution';
}
)) => void;
```
**Describe alternatives you've considered**
In my `useErrorHandler` plugin, explicitly cast the type of `context` based on the phase.
**Additional context**
Contributor guide
No contributing guide indexed for this repository
Assessment
This issue has not been assessed yet.