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

The returned data are not reactive anymore #68

Closed
mydnicq opened this issue May 31, 2017 · 12 comments
Closed

The returned data are not reactive anymore #68

mydnicq opened this issue May 31, 2017 · 12 comments

Comments

@mydnicq
Copy link

mydnicq commented May 31, 2017

When 'vue-apollo' gets a result from 'apollo-client' via 'SmartQuery' and sets the result to component properties, I found those properties are not reactive anymore. This problem arises when you use 'v-model' where a user wants to update previously received data.

To get a picture of the problem see the code from a fictional vue component:

<template>
  <div>
    <template v-if="loading > 0">
      Loading ...
    </template>
    <template v-else>
      <div>
        <input v-model="Post.description"/>
      </div>
      <button v-on:click="update">Update</button>
    </template>
  </div>
</template>

<script>
import gql from 'graphql-tag'

const getPost = gql`
  query Post($id: ID!) {
    Post(id: $id) {
      id
      imageUrl
      description
    }
  }`

const updatePost = gql`
  mutation updatePost($description: String!, $imageUrl: String!, $id: ID!) {
    updatePost(id: $id, description: $description, imageUrl: $imageUrl) {
      id
      imageUrl
      description
    }
  }`

export default {
  data: () => ({
    Post: {},
    loading: 0
  }),
  apollo: {
    Post: {
      query: getPost,
      loadingKey: 'loading',
      variables () {
        return {
          id: this.$route.params.id
        }
      }
    }
  },
  methods: {
    update () {
      // The description property is not updated with the user input.
      const { id, imageUrl, description } = this.Post

      this.$apollo.mutate({
        mutation: updatePost,
        variables: {
          id,
          imageUrl,
          description
        }
      }).then((result) => {
        console.log(result)
      }).catch((error) => {
        console.log(error)
      })
    }
  }
}
</script>

I want to know if this is a bug or expected behavior?

@Akryum
Copy link
Member

Akryum commented May 31, 2017

The data coming from Apollo is immutable. I recommend you to use a new data property if you want to modify it.

@Samuell1
Copy link

@tadejstanic apollo object is freezed you need to create other if you want use it with inputs and editing it i using Object.assign or lodash cloneDeep

Example:

  apollo: {
    Post: {
      query: getPost,
      loadingKey: 'loading',
      variables () {
        return {
          id: this.$route.params.id
        }
      }
      result ({ data }) {
        this.data = Object.assign({}, data.Post)
      }
    }
  }

@mydnicq
Copy link
Author

mydnicq commented Jun 1, 2017

At first, I was also looking for a solution in both proposed directions, which ends in new data property. But I thought we can do this at 'SmartQuery' level when vue-apollo assign the results to the component data properties.

So what do you think about this modification in 'smart-apollo.js':

nextResult (result) {
    const { data, loading } = result

    if (!loading) {
      this.loadingDone()
    }

    if (typeof data === 'undefined') {
      // No result
    } else if (typeof this.options.update === 'function') {
      this.vm[this.key] = this.options.update.call(this.vm, data)
    } else if (data[this.key] === undefined) {
      console.error(`Missing ${this.key} attribute on result`, data)
    } else {
      // THE MODIFICATION
      this.vm[this.key] = Object.assign({}, this.vm[this.key], data[this.key])
      // this.vm[this.key] = data[this.key] THIS WAS BEFORE
    }

    if (typeof this.options.result === 'function') {
      this.options.result.call(this.vm, result)
    }
  }

@ndarilek
Copy link

This modification might be nice to include, I find myself copying objects lots. I typically use _.toPlainObject(...) from lodash. Requesting data from an API, then modifying that data is the basis of just about every web app out there. Is there an instance where you wouldn't want the ability to modify retrieved data, or where doing the conversion would break other behavior?

@Austio
Copy link
Contributor

Austio commented Jun 30, 2017

Personally, i prefer the approach as is. It aligns with flux in that there is a one way flow of data. Apollo gets data, vue-apollo maps it in and then pass those down as props to other components. By letting any one of those components update the object, you lose flux.

@Musbell
Copy link

Musbell commented Nov 3, 2017

@Samuell1 your example seems to not work :( but when i change this.data to this.Post, it worked :). Thanks a lot 👍

@Samuell1
Copy link

Samuell1 commented Nov 3, 2017

@Musbell yeah, because you need create data object in data function

data () {
   return {
     data: null
   }
}

@Musbell
Copy link

Musbell commented Nov 3, 2017

@Samuell1 noted :)

@Akryum Akryum closed this as completed May 11, 2018
@dweldon
Copy link

dweldon commented Dec 21, 2018

Thank you to the participants on this thread - I just got caught on this issue as well.

I'd strongly recommend adding something to the docs (maybe in "basic usage") that calls out the fact that data coming back from queries should not be mutated. Consider using the "warning" or "danger" style to really make sure everyone sees it.

@Samuell1
Copy link

@dweldon its in Apollo client docs.

@dweldon
Copy link

dweldon commented Dec 21, 2018

@Samuell1 Thanks for pointing that out. Many users (myself included) may not need to use the apollo client directly, and therefore haven't done a careful read of its documentation before coming to vue-apollo. While I agree this may be a case of RTFM, it seems like this is an opportunity to prevent new users from getting confused.

@Samuell1
Copy link

@dweldon but vue-apollo is using apollo client directly, because its just integration but i know what you mean, maybe there should be link with explanation in docs somewhere.

Looks like it will be soon resolved and not needed? apollographql/apollo-client#3883

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

7 participants