ChildProps data? Partial Forces Null Checks

[spentak]

Intended outcome:
Expected to be able to grab props like so:

export class MasterDetailPage extends React.Component<ChildProps<{}, Response>, {}> {
  render() {
  	const { getSlopes } = this.props.data;
  	console.log(getSlopes);
    return (
        <div>

Actual outcome:
Type(QueryProps<OperationVariables> & Partial<Response>) | undefined' has no property getSlopes and no string index signature

And you must then do:

if (this.props.data) { 
    console.log(this.props.data.getSlopes);
 }

How to reproduce the issue:
Use apollo-client with TypeScript and use ChildProps

Turning of strictNullChecks is obviously not the right option, and neither is changing the source code. To get this to work one only has to change this:

export declare type ChildProps<P, R> = P & {
    data?: QueryProps & Partial<R>;
    mutate?: MutationFunc<R>;
};

to this (without optional ?):

export declare type ChildProps<P, R> = P & {
    data: QueryProps & Partial<R>;
    mutate?: MutationFunc<R>;
};

But who knows of the consequences.

Version

• [email protected]

[mctep]

As I understand there are two cases for HOC usage (query or mutation, not both). So all of this props data and mutate are optional.

react-apollo/src/graphql.tsx

Lines 520 to 522 in 1cd331e

	if (this.type === DocumentType.Mutation) {
	return this.dataForChildViaMutation;
	}

I think it possible to fix issue with separate ChildProps to two interfaces like:

interface MutateChildProps<R> { mutate: MutationFunc<R> }
interface QueryChildProps<R> { data: QueryProps & Partial<R> }

And also separate declaration for graphql HOC.

I've made some like:

export interface QueryChildProps<R> {
  data: QueryProps & Partial<R>;
}

type QueryWrapper = <R = {}, P = {}, C = QueryChildProps<R>>(
  document: DocumentNode,
  operationOptions?: OperationOption<P, R>,
) => ComponentDecorator<P, C & P>;

export const graphqlQuery = graphql as QueryWrapper;

export interface MutateChildProps<R> {
  mutate: MutationFunc<R>;
}

type MutateWrapper = <R = {}, P = {}, C = MutateChildProps<R>>(
  document: DocumentNode,
  operationOptions?: OperationOption<P, R>,
) => ComponentDecorator<P, C & P>;

export const graphqlMutate = graphql as MutateWrapper;

[rosskevin]

Duplicate of #1083, please close.

EDIT: actually this seems more succinctly stated, I think we should keep this one open and close the other.

[rosskevin]

I'm doing some refactoring and getting tests fully typed. I have fixed inference and streamlined usage for fully typed use already in #1402.

So, given my recent breaking apart of ChildProps seems to help in that data and mutate are optional, but now at least our TResult is not Partial:

// export to allow usage individually for simple components
export interface DataProps<TResult, TVariables = OperationVariables> {
  data: QueryProps<TVariables> & TResult;
}

// export to allow usage individually for simple components
export interface MutateProps<TResult = any, TVariables = OperationVariables> {
  mutate: MutationFunc<TResult, TVariables>;
}

export type ChildProps<
  TProps = {},
  TResult = {},
  TVariables = OperationVariables
> = TProps &
  Partial<DataProps<TResult, TVariables>> &
  Partial<MutateProps<TResult, TVariables>>;

With this, I looked at higher kinded types, but ultimately it would be simplest to split this API. I do think though that while verbose, I can think we can temporarily solve this situation until the split with the addition of:

export type ChildDataProps<
  TProps = {},
  TResult = {},
  TVariables = OperationVariables
  > = TProps &
  DataProps<TResult, TVariables>;

export type ChildMutateProps<
  TProps = {},
  TResult = {},
  TVariables = OperationVariables
  > = TProps &
  MutateProps<TResult, TVariables>;

So now you could strongly type the query usage with:

type CDP = ChildDataProps<MyDialogProps, MyResult, MyVariables>

class MyDialog extends React.Component<CDP> {}

export default graphql<MyDialogProps, MyResult, MyVariables, CDP>(
  QUERY,
)(MyDialog)

It's more verbose than it will be before the split, but it is a step in the right direction and this will facilitate a signature split between queries and mutations for better developer experience with the typed API.

[leoasis]

@rosskevin the reason the result was Partial in ChildProps before is that you may not have data yet if you are loading the first time. So I think just adding & TResult would make that fail in those cases, making people think those fields are there even when the data is still being loaded. Maybe we can have some union type instead of partial, to signify that either all the fields are loaded, or none are.

Maybe something like this would work? data: QueryProps<TVariables> & ({} | TResult);

[rosskevin]

Thanks @leoasis - I'll make a note of that in the typings and try that out.

[rosskevin]

/related PR #1398 2.1 Query component

[pellea]

Hello, I wrote a question on Stack Overflow asking how to make 2 mutations work with the same component in Typescript. It would be very helpful if someone from here could take a look. Thanks.

[alamothe]

Got stuck on this issue by using TypeScript-React-Starter and official Apollo documentation.

Perhaps the documentation should be fixed, as most people nowadays use strict settings

[rosskevin]

Query results are now typed. For a discussion on data being undefined or optional, let's refer to #1917.