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

Rendering updates occur before watch handlers #1706

Closed
skirtles-code opened this issue Jul 25, 2020 · 3 comments
Closed

Rendering updates occur before watch handlers #1706

skirtles-code opened this issue Jul 25, 2020 · 3 comments

Comments

@skirtles-code
Copy link
Contributor

Version

3.0.0-rc.4

Reproduction link

https://jsfiddle.net/skirtle/rd4y1xj3/9/

Steps to reproduce

  1. Make sure the developer tools are open as the example relies on a debugger statement.
  2. Click the button.
  3. The example should pause in the debugger at the start of the watch.
  4. Notice that one of the rendered values has changed in the DOM.
  5. Allow the code to resume running.
  6. Note that the other rendered value has now changed.

What is expected?

In Vue 2 the watch would run prior to rendering updates. See:

https://jsfiddle.net/skirtle/je5qdrLp/4/

What is actually happening?

The watch runs after rendering, triggering a second phase of rendering.


This appears to be because the default value for flush is 'post'. However, that seems like a strange default as a watch (when used via the options API) will generally make some kind of data change that will require further rendering updates.

In particular, consider the case where watch is used to load data asynchronously. Typically that will involve setting some sort of isLoading flag to show a load mask while the data is loading. With Vue 2 that would have triggered a single rendering update but with Vue 3 RC4 it will go through two lots of rendering.

Worse, as the watch hasn't run yet the template could be trying to render inconsistent data.

With Vue 3 the updated hook will also run twice. However, the watch handler is called between the DOM update and the call to the updated hook. This leads to a potentially confusing scenario where the DOM doesn't reflect what is in the data when updated is called. It seems counter-intuitive for a watch to run between a DOM update and the corresponding updated hook.

@yyx990803
Copy link
Member

yyx990803 commented Jul 27, 2020

This is intended and expected behavior.

However, that seems like a strange default as a watch (when used via the options API) will generally make some kind of data change that will require further rendering updates.

Your understanding is exactly the opposite of the intended design: watchers are intended for performing non-state side effects like operating on the DOM, so it defaults to post flush so that if a state change triggered both a render update and a watcher, the watcher would be accessing the already updated DOM by default.

Mutating state in a watcher is an anti-pattern and you should be using computed properties to build up your reactive state graph instead of relying on watchers to "chain" state changes together.

I don't understand your data loading example.

Worse, as the watch hasn't run yet the template could be trying to render inconsistent data.

Again the opposite: the expected state used for rendering should be the exact after direct mutation and before any watcher is run. If you mutate state inside a watcher callback then it will trigger another update. It's never inconsistent.

All of this boils down to your pattern of relying on watchers watching some state to mutate other state - which you should refactor into computed properties or explicitly use pre-flush watchers.

@skirtles-code
Copy link
Contributor Author

@yyx990803 My apologies. In my attempt to be concise I failed to explain myself clearly.

My test case was only designed to illustrate the problem, it certainly wasn't a compelling example of correct watch usage. By the time I had stripped it down to a minimal example it did indeed appear to be a classic case of watch misuse. I should have acknowledged that in my original post. Lesson learned.

I will try to be clearer this time.

I don't understand your data loading example.

I'll attempt to illustrate this using the Hacker News Clone Example. Specifically, this file:

https://github.com/vuejs/vue-hackernews-2.0/blob/7a5c8fcc5a1ac2e6b72db9f42c6e5a6cb81a092d/src/views/ItemView.vue

The relevant parts for my explanation are:

export default {
  data: () => ({
    loading: true
  }),

  computed: {
    item () {
      return this.$store.state.items[this.$route.params.id]
    }
  },

  beforeMount () {
    this.fetchComments()
  },

  watch: {
    item: 'fetchComments'
  },
  
  methods: {
    fetchComments () {
      if (!this.item || !this.item.kids) {
        return
      }
      
      this.loading = true
      fetchComments(this.$store, this.item).then(() => {
        this.loading = false
      })
    }
  }
}

In Vue 2 the following steps would occur when $route.params.id changes:

  1. The computed property item would be invalidated.
  2. The watch would be triggered and the new value for item would be evaluated.
  3. fetchComments would set loading to true.
  4. The component would perform a rendering update.

In Vue 3 things would be a little different:

  1. The computed property item would be invalidated.
  2. As item is a rendering dependency the component will perform a rendering update.
  3. The watch would be triggered.
  4. fetchComments would set loading to true.
  5. As loading is also a rendering dependency the component will render again.

In this simple application the extra rendering run doesn't cause any serious problems. However...

  • In a larger application the performance hit could be much larger, especially if a component near the top of the component tree renders twice and makes significant changes that cascade down through the descendant components.
  • When the first rendering update occurs the template is seeing 'inconsistent state'. The example above gets away with it but imagine a similar application that protects against null child data using a v-if="!loading" check. That would've worked fine in Vue 2 but in Vue 3 the template will run before that flag gets set. I'm happy to put together a JSFiddle if it isn't clear what I'm describing.

The example of using watch in the Vue 2 Guide is also affected. I've put it into JSFiddle to demonstrate, with console logging in the updated hook to keep track of the updates:

Vue 2: https://jsfiddle.net/skirtle/3txfq02z/6/
Vue 3: https://jsfiddle.net/skirtle/6bzw853y/10/

When a single character is typed in the box the Vue 2 example performs a single update. The Vue 3 example performs two updates.

The key lines from the code are:

watch: {
  // whenever question changes, this function will run
  question: function (newQuestion, oldQuestion) {
    this.answer = 'Waiting for you to stop typing...'
    this.debouncedGetAnswer()
  }
},

The first thing it does is assign a new value to answer. Updating state may not be why a watch is used but it is nevertheless something that happens, with the consequence that Vue 3 will trigger an extra rendering update.


There are dozens of examples in the Vuetify source. A quick search for watch: shows that it's being used in over 60 files. Not all of those update state, but most of the ones I checked did.

Sure, there are also cases where Vuetify uses $nextTick in a watch. The new Vue 3 behaviour will make those mostly unnecessary. But that will come at the expense of having to sprinkle flush: 'sync' or flush: 'pre' all over the place to ensure state is kept in sync and rendering performance doesn't take a hit.

The timing change for watch is not the only significant timing change. beforeMount and beforeUpdate have also moved to run after the render function. In theory this shouldn't matter because nobody would use them to synchronise state prior to rendering. However, in reality this is a commonly used escape hatch in complex applications. With the new behaviour this will also trigger an extra rendering update, just like watch. I don't believe there's any equivalent to flush: 'pre' to get these hooks working the way they did in Vue 2.

There are several examples of beforeMount/beforeUpdate in the Vuetify code. Here's one:

https://github.com/vuetifyjs/vuetify/blob/9f6f5a58481e5de6e95f8e7a84242cced940d70c/packages/vuetify/src/components/VNavigationDrawer/VNavigationDrawer.ts#L293

That file also features some entertaining uses of watch.


To the best of my knowledge the design decisions that led to these timing changes are not documented anywhere. If they are I would love to see that. Currently the only justification I have for the new watch behaviour is that it allows some uses of $nextTick to be removed. I'm sure many people in the community would welcome that but I'm not sure they would feel the same way if they knew what the downsides were.

If the final decision is that the trade-offs are worth it then I feel it needs to be properly documented so that everyone can appreciate what has changed and why.

This was referenced Aug 3, 2020
yyx990803 added a commit that referenced this issue Sep 18, 2020
BREAKING CHANGE: watch APIs now default to use `flush: 'pre'` instead of
`flush: 'post'`.

  - This change affects `watch`, `watchEffect`, the `watch` component
    option, and `this.$watch`.

  - As pointed out by @skirtles-code in
    [this comment](#1706 (comment)),
    Vue 2's watch behavior is pre-flush, and the ecosystem has many uses
    of watch that assumes the pre-flush behavior. Defaulting to post-flush
    can result in unnecessary re-renders without the users being aware of
    it.

  - With this change, watchers need to specify `{ flush: 'post' }` via
    options to trigger callback after Vue render updates. Note that
    specifying `{ flush: 'post' }` will also defer `watchEffect`'s
    initial run to wait for the component's initial render.
@bekliev
Copy link

bekliev commented Oct 17, 2020

@skirtles-code great breakdown!

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

No branches or pull requests

3 participants