graphql-hive / graphql-hive/envelop

Improve types for `useErrorHandler` plugin

Open
#2,068 0 comments 1 reaction 0 assignees View on GitHub
kind/enhancement
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.

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.