Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

TS type of "previousQueryResult" on updateQuery #3391

Closed
nilshartmann opened this issue May 1, 2018 · 8 comments
Closed

TS type of "previousQueryResult" on updateQuery #3391

nilshartmann opened this issue May 1, 2018 · 8 comments
Labels
📚 good-first-issue Issues that are suitable for first-time contributors. ✔ confirmed 🙏 help-wanted 🚧 in-triage Issue currently being triaged ⛑ TypeScript

Comments

@nilshartmann
Copy link

Hello,

when using the updateQuery function on subscribeToMore (that gets passed as prop to the render prop from Query), the first argument previousQueryResult is in TypeScript defined as Object:

Wouldn't it make sense that previousQueryResult is of the same type as the Query result type?

Example:

class PersonQuery extends Query<PersonQueryResult> {};

class MyComponent extends React.Component {
  render() {
    return <PersonQuery query="...">
      {({ loading, error, data, subscribeToMore }) => {
        . . .
        subscribeToMore({
          updateQuery: (previousQueryResult, options) {
            // previousQueryResult should be of Type PersonQueryResult?
            // but is: Object 
          }
        })}
      }
    </PersonQuery>
  }
}

Version

  • apollo-client@2.2.8
@ghost ghost added the question label May 1, 2018
@clayne11
Copy link
Contributor

clayne11 commented May 4, 2018

The TS types overall could definitely use some work.

@hwillson
Copy link
Member

The TS types overall could definitely use some work.

We ❤️ pull requests! 😉

@nilshartmann
Copy link
Author

@hwillson cool! Will need some time to get into, but really like to help!

@skovy
Copy link
Contributor

skovy commented Jun 11, 2018

@nilshartmann @hwillson here is a PR for a temporary "band-aid" to improve these types immediately for react-apollo: apollographql/react-apollo#2093 (assuming those were the types you were looking at @nilshartmann? I think the file you linked to might be used elsewhere?).

I also created an issue to clean up these types in the react-apollo repository: apollographql/react-apollo#2092

These types still aren't released in apollo-client, but were greatly improved recently: #3140

@nilshartmann
Copy link
Author

@skovy looks good, thank you so much!

I am not sure how I discovered the file in my original comment (somehow by blindly following VSCode navigations 😉)

Regarding #3140 (I think...): when using mutate on apollo client instance (passed to a render function), it's possible to specify the type for the mutationRESULT but not for their variables:

client.mutate<SetCurrentBeerIdMutationResult, NO VARIABLES TYPE POSSIBLE HERE>({
  mutation: MY_MUTATION,
  variables: { } // variables have any type here
})

This seems not be addressed in 3140?

@hwillson hwillson added ✔ confirmed 📚 good-first-issue Issues that are suitable for first-time contributors. and removed question labels Jun 14, 2018
@elie222
Copy link

elie222 commented Jun 20, 2018

I think I just ran into a similar issue when trying to read the result of a mutation:

Type 'void | FetchResult<LogInData, Record<string, any>>' is not assignable to type 'LogInData'.
  Type 'void' is not assignable to type 'LogInData'.

Code looks something like this, and I'm struggling to get it working.

import * as React from 'react'
import { Mutation } from 'react-apollo'
import gql from 'graphql-tag'
import LogInForm from './LogInForm'

const LOG_IN = gql`
  mutation logIn($email: String!, $password: String!) {
    logIn(email: $email, password: $password) {
      _id
      email
      password
      jwt
    }
  }
`

interface LogInData {
  data: {
    logIn: {
      _id: string,
      email: string,
      password: string,
      jwt: string,
    }
  }
}

interface LogInVariables {}

class LogInMutation extends Mutation<LogInData, LogInVariables> {}

export interface LogInFormContainerProps {}

export default class LogInFormContainer extends React.Component<
  LogInFormContainerProps,
  any
> {
  public render() {
    return (
      <LogInMutation mutation={LOG_IN}>
        {(logIn, { data }) => (
          <LogInForm
            /* tslint:disable */
            handleOk={async (values: any) => {
              console.log('LogInFormContainer values')
              console.log(values)

              const res = await logIn({ variables: { ...values } })

              console.log('res', res)

              // error!!!!
              localStorage.setItem('token', res.data.logIn.jwt);
            }}
            /* tslint:enable */
            loading={false}
          />
        )}
      </LogInMutation>
    )
  }
}

I'm quite new to TS, so it could just be that I've misunderstood how things are supposed to work.

@tanaypratap
Copy link
Contributor

I had the same issue today. Basically the mutation promise can return void or data. You are assuming that it will return data always and that's why the TS error. Fix is quite simple actually
localStorage.setItem('token', res && res.data.logIn.jwt);

@jbaxleyiii jbaxleyiii added the 🚧 in-triage Issue currently being triaged label Jul 9, 2019
@jbaxleyiii
Copy link
Contributor

Thanks for reporting this. There hasn't been any activity here in quite some time, so we'll close this issue for now. If this is still a problem (using a modern version of Apollo Client), please let us know. Thanks!

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Feb 16, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
📚 good-first-issue Issues that are suitable for first-time contributors. ✔ confirmed 🙏 help-wanted 🚧 in-triage Issue currently being triaged ⛑ TypeScript
Projects
None yet
Development

No branches or pull requests

7 participants