relay-tools / relay-hooks

Use Relay as React hooks
https://relay-tools.github.io/relay-hooks/docs/relay-hooks.html
MIT License
540 stars 56 forks source link

feat: add a skip option for useSubscription #148

Closed n1ru4l closed 3 years ago

n1ru4l commented 3 years ago

Sometimes it is useful to skip a subscription as the variables for setting up the subscription might not be available yet.

Closes https://github.com/relay-tools/relay-hooks/issues/131

Test will follow soon.

n1ru4l commented 3 years ago

@morrys I was also wondering how we could type the useSubscription arguments in a way whether the variables parameter is optional if skip is true.

morrys commented 3 years ago

@morrys I was also wondering how we could type the useSubscription arguments in a way whether the variables parameter is optional if skip is true.

@n1ru4l, you can try to change this https://github.com/relay-tools/relay-hooks/blob/b35e16b5132500dc75d92f02a012cc32c811c3a2/src/useSubscription.ts#L1-L12

with:

import { useEffect } from 'react';
import {
    GraphQLSubscriptionConfig,
    requestSubscription,
    OperationType,
    GraphQLTaggedNode,
} from 'relay-runtime';
import { useRelayEnvironment } from './useRelayEnvironment';

export type SubscriptionConfig = {
    skip?: boolean;
};

export type SkipSubscriptionConfig = {
    skip: true;
};

export interface SkipGraphQLSubscriptionConfig<TSubscription extends OperationType>
    extends Omit<GraphQLSubscriptionConfig<TSubscription>, 'variables' | 'subscription'> {
    subscription?: GraphQLTaggedNode;
    variables?: TSubscription['variables'];
}

export function useSubscription<TSubscriptionPayload extends OperationType = OperationType>(
    config: GraphQLSubscriptionConfig<TSubscriptionPayload>,
    opts?: SubscriptionConfig,
): void;
export function useSubscription<TSubscriptionPayload extends OperationType = OperationType>(
    config: SkipGraphQLSubscriptionConfig<TSubscriptionPayload>,
    opts: SkipSubscriptionConfig,
): void;
export function useSubscription<TSubscriptionPayload extends OperationType = OperationType>(
    config: GraphQLSubscriptionConfig<TSubscriptionPayload>,
    opts?: SubscriptionConfig,
): void {

Let me know what you think

@n1ru4l I have updated the comment

morrys commented 3 years ago

Now I think it's okay without causing any breaking changes :)

Can I ask you to move all type declarations into RelayHooksTypes.ts?

morrys commented 3 years ago

now it seems perfect 👍

now I'm not at the PC, on Monday I will accept it :)

thanks for your work