graphql-hive / graphql-hive/envelop

Improve context and typings

Open
#1,566 4 comments 3 reactions 1 assignee Claimed by @enisdenjo View on GitHub
Dominant language
No language data
Stars
827
Forks
132
PR merge metrics
No merged PRs in 30d

Description

Typings for the context are completely unreliable. Envelop tries to get a "union" of required contexts for each plugin and use that union as the context supplied in `getEnveloped`. Ultimately, due to complexity, the context is not typed anywhere where it should be...

I see some flaws with the current approach:
1. Accurately inferring the union of plugins contexts
2. Plugins can have contexts that shouldn't be supplied by users (plugin scope context)
3. Plugging in a shared generic for the user context is not possible (you cannot just `type Ctx = { hey: 'there' }; envelop({ plugins: [...] })`)
4. A plugin can extend the context, but what it extends is nowhere known outside of the plugin itself

IMHO, envelop's unnecessarily overcomplicating it...

A simple, robust and reliable approach would be to simply split the context in 2:
- User context (actual context passed to getEnveloped, accessible to all plugins)
- Plugin context (scoped context that doesn't hoist and is accessible only to the plugin, is reset on every getEnveloped lifecycle)

About the point 4. in the flaws list: with the new approach, you'd simply define the parts plugins extend inside the user context (as optional of course, since not available on init). This is not only simpler and easier to understand, but is also more secure TS-wise because a plugin needs to define which fields the user context has to have and then TS wouldn't compile until the user adjusts the context; of course, adjusting the user context will make the context extensions available **everywhere** in envelop.

Furthermore, having a properly typed context will never raise a need for mitigations like: https://github.com/dotansimha/graphql-yoga/pull/1911.

### Implementation

_Pseudo code ahead_

```ts
// envelop.ts

// NOTE: intentionally `setContext` and not `extendContext` in options.
// it is more reliable because extendContext({ partOf: 'ctx' }) will
// raise errors in plugins because the full context is not known from the generic. while
// setContext({ ...context, partOf: 'ctx' }) wont raise errors since the full context is
// guaranteed to always have been supplied

export interface EnvelopPlugin {
onEnveloped?(opts: {
context: Readonly;
setContext: (ext: Ctx) => void;
pluginContext: Readonly;
setPluginContext: (ext: PluginCtx) => void;
// ...other options...
}): void;
onExecute?(opts: {
context: Readonly;
setContext: (ext: Ctx) => void;
pluginContext: Readonly;
setPluginContext: (ext: PluginCtx) => void;
// ...other options...
}): void;
// ...other hooks...
}

export interface EnvelopOptions {
plugins: EnvelopPlugin[];
// ...other options...
}

export function envelop(opts: EnvelopOptions) {
return function getEnveloped(ctx: Ctx) {
return {
// ...graphql...
};
};
}
```

```ts
// useAuth.ts

import { authenticate } from './my-auth';

// what is required and what extendable in the user context
export interface AuthUserContext {
request: Request;
userId: string | null;
}

export function useAuth<
// using extends will force the main user context to have necessary fields
Ctx extends AuthUserContext
>(): EnvelopPlugin {
return {
onExecute({ context, setContext }) {
const userId: string = authenticate(
context.request // will exists reliably always
);
if (userId) {
setContext({
...context,
userId, // will be typed since the plugin expects the field
});
}
},
};
}
```

```ts
// useTracing.ts

// what is extendable in the user context
export interface TracingUserContext {
executeDuration: number | null;
}

// never leaks, is only for the plugin
export interface TracingPluginContext {
start: number;
}

export function useTracing<
// using extends will force the main user context to have necessary fields
Ctx extends TracingUserContext
>(): EnvelopPlugin {
return {
onEnveloped({ pluginContext, setPluginContext }) {
setPluginContext({ ...pluginContext, start: Date.now() });
},
onExecute({ context, setContext, pluginContext }) {
const executeDuration = Date.now() - pluginContext.start;
setContext({ ...context, executeDuration });
},
};
}
```

### ✅ Succ usage

_Pseudo code ahead_

```ts
import { envelop } from './envelop';
import { AuthUserContext, useAuth } from './useAuth';
import { TracingUserContext, useTracing } from './useTracing';

type UserContext = AuthUserContext &
TracingUserContext & {
whateverelse: 'hey';
};

const getEnveloped = envelop({
plugins: [useAuth(), useTracing()],
});

export function handleRequest(request: Request) {
getEnveloped({
request, // required
userId: null, // not available yet
executeDuration: null, // not available yet
whateverelse: 'hey',
});
}
```

### 🛑 Fail usage

_Pseudo code ahead_

```ts
import { envelop } from './envelop';
import { AuthUserContext, useAuth } from './useAuth';
import { useTracing } from './useTracing';

type UserContext = AuthUserContext & {
whateverelse: 'hey';
};

const getEnveloped = envelop({
plugins: [
useAuth(),
useTracing(), // ts error - user context is incomplete
],
});
```

```ts
import { envelop } from './envelop';
import { AuthUserContext, useAuth } from './useAuth';
import { TracingUserContext, useTracing } from './useTracing';

type UserContext = AuthUserContext &
TracingUserContext & {
whateverelse: 'hey';
};

const getEnveloped = envelop({
plugins: [useAuth(), useTracing()],
});

getEnveloped(
// ts error - request is required
{
userId: null, // not available yet
executeDuration: null, // not available
whateverelse: 'hey',
}
);
```

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.