Skip to content

Document why watched template refs require theflush: 'post' option to be set #816

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

Merged
merged 7 commits into from
Jan 25, 2021

Conversation

LinusBorg
Copy link
Member

@LinusBorg LinusBorg commented Jan 16, 2021

Description of Problem

  • Watchers are flushed before DOM updated by default (flush: 'pre')
  • Template Refs are udpated after DOM updates
  • So when watching a template ref, flush: 'post' has to be used in order to get a correct element reference at all times.

Proposed Solution

  • Add a new section in the chapter on template refs about this, with an example.
  • Link to this new section from the "Effect Flush Timing" section in the Reactivtiy docs.

Additional Information

@LinusBorg LinusBorg changed the title docs: Document why watched template refs require theflush: 'post' option to be set Document why watched template refs require theflush: 'post' option to be set Jan 16, 2021
@LinusBorg LinusBorg added the enhancement New feature or request label Jan 16, 2021
Copy link
Contributor

@skirtles-code skirtles-code left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've made some small suggestions inline but there's a bigger point I wanted to raise and I wasn't sure where to put it so I've put it here.

I'm sorry for the lengthy essay. I found it really difficult to explain this properly. I hope you can figure out what I mean from my rambling.

In summary, I think the problem reported in vuejs/core#3030 is a bug in Vue. I understand why it looks like user error but I don't think it is.

I think it's important to clarify exactly what's going on here and what the bug is because it impacts how we document this. While there are good reasons why you might want to use flush: 'post' with template refs, I don't think it's quite as straightforward as it appears.

The description below is somewhat simplified but if I went into all the subtle details I'd be writing it forever.


There are 3 queues within the scheduler, which I'm going to call PRE, RENDER and POST. The queues are processed in that order, with one queue being exhausted before moving on to the next queue:

PRE -> RENDER -> POST

I'm going to refer to this 3-stage process as the 'render cycle'.

When the render cycle completes it checks the queues again. If any new entries have been added to the queues it goes back to the start and does it all again. We can end up with something like this:

PRE -> RENDER -> POST | PRE -> RENDER -> POST | ...

It is worth noting that the POST queue is copied just before it is processed, so any new POST items won't be added to the current POST queue. Instead they'll be added to a new POST queue and will be processed on the next loop around the cycle.

I should also stress that this is a distinct concept from nextTick. The loop I've described above all happens within the same 'tick'. The 'next tick' is queued on a microtask and only occurs once this loop is complete.

The ref for a rendered element is updated during the POST phase. Changing the ref will queue up any effects that are dependent on that ref. They'll be added to either the PRE or POST queue depending on their flush setting. Either way, they won't be processed until the next loop around the render cycle.

So even if an effect uses flush: 'pre' it still shouldn't get out of sync with the ref. In practice using 'pre' will cause it to be synced up sooner, so it could be argued that it's a better choice in some cases.

For an element ref, the most obvious difference between 'pre' and 'post' is when the effect is run for the first time. The initial run for 'pre' is before the component is mounted, so the ref won't have been updated yet. But it will still be added to the queue when the ref is updated, so it should still be called with the correct element just after mounting.

So why doesn't it work in vuejs/core#3030?

That's a bug. When it reaches the end of the render cycle it only checks whether the RENDER and POST queues are empty. It doesn't currently check the PRE queue. I reported this bug in vuejs/core#2730. There's a PR associated with that issue that should hopefully fix it. As you might notice, someone on that issue tried to argue that this is 'by design'. I believe that to be the same misunderstanding that I've outlined above. It fails to take account of the looping nature of the render cycle. With the current bug, you end up with unprocessed items leftover in the PRE queue when the current tick completes.

To further illustrate my point, consider the following example:

https://jsfiddle.net/skirtle/e3rnbqpg/

The code is very similar to the example in vuejs/core#3030. The only significant change is that I've added a flush: 'post' watcher, in addition to the original flush: 'pre' watcher. Notice the logging. The flush: 'pre' watcher now works perfectly. The reason this 'works' is because the flush: 'post' watcher triggers another loop around the render cycle, which then processes the PRE queue. This should happen anyway but due to the bug I mentioned earlier it doesn't happen unless there's something in one of the other two queues.

I'd like to make a final quick note about some of the ways that my description above is a simplication:

  1. If there are multiple components involved then it gets a little bit more complicated. A single iteration of the render cycle can actually end up running the PRE and RENDER queues multiple times, allowing it to handle 'pre' watchers on props. Such watchers need to run after the parent is rendered but before the child renders. This process is further complicated because parent/child rendering circumvents the usual queuing mechanism for rendering.
  2. The initial mounting that occurs when calling app.mount(...) behaves somewhat differently because it doesn't go through the scheduler queue in the normal way. I don't want to go into detail here but that's a can-of-worms all of its own. Here's an example, based on the earlier example, but where everything happens during the initial application mounting https://jsfiddle.net/skirtle/23jaeq4s/1/. Notice that the watcher works fine, despite not using flush: 'post'. The first time it's called the ref value is undefined but that's to be expected. The key thing is that is does get called again when the ref is updated.

@LinusBorg
Copy link
Member Author

LinusBorg commented Jan 22, 2021

@skirtles-code Thanks for the detailed writeup. I'll try and look into this. But my first intuition is that not flushing pre-effects that were added to the scheduler queue during an update is done on purpose and with a reason.

After all, pre effects expect to have their changes reflected in the render update that is meant to follow them. But flushing those pre effects that were added during the update in the post phase means that there's no update following them (unless they trigger another pre effect in their callback).

In the case of a template ref, that's not an issue, as the effect make sense to run in the post phase.

But at the moment I can't rule out (but don't have an example come to mind immediately either) that other pre effects could potentially be triggered during an update - effects which only make sense when a component render update follows them, as pre effect can usually expect. I think that generally, flushing a pre effect in the post phase can lead to unexpected results where the scheduler doesn't enter the next cycle and therefore, the DOM doesn't get updated.

As I said, this is just an intuition more or less, might be totally wrong ^^

@skirtles-code
Copy link
Contributor

This comment is a response to the previous comment from @LinusBorg but it is probably not directly relevant to the documentation. I'll post any thoughts I have about the PR itself separately.


I don't think this was intentional but I can't be sure.

During the RC phase I reported a number of bugs with pre watchers. At that point, pre effects didn't have their own queue, they were bundled into the main scheduler queue. Evan split the queue in this commit, as part of trying to fix some of the bugs I reported:

vuejs/core@3692f27#diff-6142a09a188ff9ccb70f2ac08cb772e3ad9798386f3af4c95dd1567d9ab376ceR203

The GitHub diff isn't super helpful. The most relevant changes are in scheduler.ts. Line 170 makes flushing the pre queue a separate stage within flushJobs but line 203 isn't updated to take that into account. Perhaps this was intentional but it looks like an oversight to me. Prior to this change any new pre jobs would have been added to the main queue, so it would have looped round and processed them. The way its implemented now, the pre effects do still get flushed eventually but that 'eventually' won't be until something else triggers the scheduler queue, which may be never.

If it had been intentional then it would have been trivial to add in a warning to avoid the potential confusion.

The two JSFiddles I included in my previous comment show that a pre effect can run after the post stage in some cases. If it is a deliberate design decision to stop pre effects running then these various edge cases also need to be addressed because they aren't consistent with that design.

I think a key point that is worth re-iterating is that using flush: 'post' to watch an element ref doesn't run the watcher as part of the same render cycle that updates the ref. Going through this step by step, the first two stages are what you might expect, the component renders and then the ref is updated as part of the post phase. However, the watcher is not run as part of that same post phase. Instead, the whole render cycle goes round again, going through another pre phase and another render phase before finally reaching the post phase that runs the watcher. The extra pre and render phases may be empty, in which case everything appears to have happened in the same post phase, but it's also entirely possible that they aren't empty.

In my opinion, the correct behaviour is for all effects to be processed within the same tick. The intuitive notion of three clearly distinct stages, pre/render/post, doesn't really reflect reality. Edge cases around watching props already lead to pre effects being flushed during the render phase, so those phases actually overlap (the code for that is in renderer.ts, which calls flushPreFlushCbs). I wouldn't want pre effects to run during the post phase, I think that would be confusing, but once the post phase completes it feels natural to me that a new 'render cycle' can begin immediately, even if there's nothing to render. Perhaps referring to it as the 'render cycle' isn't helpful because that implies its sole purpose is rendering, which is misleading. It's just an effects cycle, no actual rendering needs to occur. The word that matters is cycle. It should keep cycling through the phases until everything is processed.

Copy link
Contributor

@skirtles-code skirtles-code left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this is good to merge.

I still think we're documenting around a bug, but given the current behaviour it does make sense to have this in the documentation. People keep hitting this problem.

@NataliaTepluhina NataliaTepluhina merged commit efbbe59 into master Jan 25, 2021
@NataliaTepluhina NataliaTepluhina deleted the linusborg/template-ref-watchers-post branch February 24, 2021 08:34
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants