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

Document potential issue with dependency not being bound to the computed property #18

Open
Yovar opened this issue May 3, 2017 · 4 comments

Comments

@Yovar
Copy link

Yovar commented May 3, 2017

Hello, nice project!

I'd like to give my input on a potential issue one may encounter if he doesn't understand the logic behind VueJS dependencies. The following code will not update the asynchronous property on username change:

export default {
  data () {
    return {
      username: 'Yovar'
    }
  },
  asyncComputed: {
    greeting () {
      return new Promise(resolve =>
        setTimeout(() => resolve('Hello ' + this.username + '!'), 1000)
      )
    }
  }
}

From my understanding VueJS uses getters on the component's properties to detect if they are accessed between the invocation and the return of a function. In this scenario the setTimeout callback is invoked after the return of greeting. The greeting function should be writed as follows:

    greeting () {
      const username = this.username
      return new Promise(resolve =>
        setTimeout(() => resolve('Hello ' + username + '!'), 1000)
      )
    }

This way, the username property is accessed at a good moment for VueJS to understand the relation between this property and greeting.

I think it might be a common pitfall for JS beginners.

Here is a last example showing how hard it can be to debug using async/await. In this scenario changing locale will update greeting but changing username won't:

const texts = {
  en: {
    greeting: 'Hello {0}!'
  },
  fr: {
    greeting: 'Bonjour {0} !'
  }
}

function getLocalizedText (locale, text) {
  return new Promise(resolve =>
    setTimeout(() => resolve(texts[locale][text]), 1000)
  )
}

function format (pattern, parameters) {
  let index = 0
  parameters.forEach(parameter => {
    pattern = pattern.replace('{' + index++ + '}', parameter)
  })
  return pattern
}

export default {
  data () {
    return {
      username: 'Yovar',
      locale: 'fr'
    }
  },
  asyncComputed: {
    async greeting () {
      const pattern = await getLocalizedText(this.locale, 'greeting')
      const formatted = format(pattern, [this.username])
      // Notice how the two instructions are on the same level.
      // Without a deep understanding of async/await it's pretty hard to understand the property not getting updated on `username` change.
      return formatted
    }
  }
}

Cheers.

@foxbenjaminfox
Copy link
Owner

You're quite right that this might be a bit confusing. Ultimately, it's a pretty standard beginners stumbling block with callbacks. Many beginners often try to write something like:

try {
  setTimeout(function () {
    throw new Error()
  }, 1000)
} catch (err) {
}

and are then confused when it does not work.

The issue here is the same, and in order to be fully beginner friendly it would be great to document it. I'd actually really welcome a pull request with a good explanation of this behavior and how to avoid it. Do you want to try? Otherwise I'll try to write it up on my own, but I'm not quite sure how I'd explain the concepts here in the most beginner-friendly terms.

@dsl101
Copy link

dsl101 commented Aug 21, 2017

Sorry if I've misunderstood, but I've been caught by something similar too. Isn't the code from your first example:

setTimeout(() => resolve('Hello ' + this.username + '!'), 1000)

just not getting the correct value of this because of the arrow function definition?

@Yovar
Copy link
Author

Yovar commented Sep 2, 2017

@foxbenjaminfox Sorry I don't feel confident doing a pull request eplaining this issue because I don't feel confident enough with the English language.

@dsl101 No, in this case this is referencing the context of the greeting function, the Vue instance. This is expected.

@foxbenjaminfox
Copy link
Owner

foxbenjaminfox commented Sep 2, 2017

@Yovar: That's fine. I'll try and think of how I can best write it up myself. Thank you for raising this issue, in any case.

@dsl101: As @Yovar says, the problem here isn't the arrow function. this is supposed to be the Vue instance, and the arrow function lets it remain the Vue instance.

The problem is that inside a callback inside the greeting function isn't (synchronously) part of the greeting function, so Vue cannot register the property accesses there as part of greetings dependencies.

If all that seems a bit too abstract, look at this simple example:

try {
  setTimeout(function () { throw new Error() }, 1000)
} catch (err) {
  console.error(err)
}

Will it log the error or not?

You can try running it, and see that it won't. The bit that throws the error in the callback, isn't actually within the try block. When the callback finally gets called a second later, the try block is long gone.

Imagine if the code was written this equivalent way:

const cb = function () { throw new Error() }
try {
  setTimeout(cb, 1000)
} catch (err) {
  console.error(err)
}

This is the same thing, and perhaps now it's a bit clearer why the catch block won't catch the thrown error. The same principle applies not just with try/catch, but also with Vue's dependency detection.

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

3 participants