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

beforeUpdate() is actually executed AFTER render() #7481

Closed
nobelhuang opened this issue Jan 19, 2018 · 14 comments · Fixed by #7822
Closed

beforeUpdate() is actually executed AFTER render() #7481

nobelhuang opened this issue Jan 19, 2018 · 14 comments · Fixed by #7822

Comments

@nobelhuang
Copy link

Version

2.5.13

Reproduction link

https://jsfiddle.net/5vze74yy/

Steps to reproduce

  1. Add render() and beforeUpdate() hook to the instance.
  2. Trigger data value change by any means

What is expected?

Per the API

https://vuejs.org/v2/api/index.html#beforeUpdate

beforeUpdate is supposed to be fired before render(), and state changes in this hook should not trigger additional re-renders.

What is actually happening?

beforeUpdate() is actually invoked after render() in current version 2.5.13.


This could result in infinite update loop, as state changes in beforeUpdate() will trigger another re-rendering.

@HerringtonDarkholme
Copy link
Member

Called when the data changes, before the virtual DOM is re-rendered and patched.

re-render here means exactly the same as patched: it means the generated(or, rendered) virtual dom node is applied to the real dom tree.

On the other hand, render in API means: a method to generate virtual dom. It does not mean it is drawn(or, render) on real dom.

Admittedly, render is too overloaded here. @chrisvfritz any idea?

@yyx990803
Copy link
Member

I think there's indeed a mismatch here, since the description indicates you can mutate state in beforeUpdate which will then be applied - this entails that the hooks is called before the virtual dom render functions is called.

I'll give it some proper thought next week.

@nobelhuang
Copy link
Author

Just saw this updated. Do you guys really think it is a good idea that resolving this by just simply revising the wording of the API docs? I don't think so.

First of all, render is never the exactly same meaning as patch when no target following render term, i.e. render to the DOM, especially when Vue is using virtual DOM. People coming from React background will always consider render as running the render() function, by using your terms, as generating virtual DOM nodes. Of course, unless Vue wants to be very unique on this one...

Secondly, if do decide to take the path that updating the docs to match the current beforeUpdate() behavior, not only the API description needs to be revised, but also the lifecycle diagram (link). Since these two places consistently indicate beforeUpdate() should not be like current behavior, I don't believe it was deliberately designed that beforeUpdate() is called after render(), hence it is a functional bug instead of documentary.

Further more, if it is not allowed to change/update the states of the component in beforeUpdate without triggering additional re-render, this lifecycle hook is much less useful from developers point of view, because it is a very common case, for developers not matter coming from React or Angular, that updating states of components per changes to props. You know, the render() is called in various cases, like its own state changed, props changed, or even the parent forcing update. In React, it has a few similar but separate hooks to allow developers to detect those changes and even update states BEFORE calling render() (link). However, unlike React, Vue has only one lifecycle hook beforeUpdate() documented to be able to detect changes and to update states before render, it becomes the only hopeful place where suppose to perform those common practices.

@chrisvfritz
Copy link
Contributor

chrisvfritz commented Jan 22, 2018

@nobelhuang Don't worry, this isn't considered "resolved" yet. It was my fault the issue was accidentally closed, as I didn't remove the fixes #7481 from my PR after discussing the problem in more detail with @HerringtonDarkholme.

@HerringtonDarkholme
Copy link
Member

HerringtonDarkholme commented Jan 23, 2018

Thanks for the passionate discussion. The doc issue accidentally closes this, so I reopen the issue since it is still in review.

not only the API description needs to be revised, but also the lifecycle diagram

I think so.

beforeUpdate() is called after render()

Can you give an example where called before/after render will give significant difference? The old doc said further state change can occur in beforeUpdate. However, I think that might be inappropriate. State changing can lead to infinite loop, whether it is called before render or not.

However, if beforeUpdate disallow state changing or dispatch action to flux store, in other words, not triggering re-render, I wonder if it matters to call before/after render.

it is not allowed to change/update the states of the component in beforeUpdate

So changing state should be your use case. But as discussed above I suspect if it is proper usage. React has recommend against this, and of course Vue don't want to be unique here.

Vue has only one lifecycle hook beforeUpdate(), .... , it becomes the only hopeful place where suppose to perform those common practices

So the real proposal here should be adding a new lifecycle like componentWillReceiveProps, right? The difference here is that prop change is only top-down. So component will execute receiveProp lifecycle once and trigger state change, and then execute beforeUpdate once, and finally one render. State change in beforeUpdate however can lead to infinite loop.

Other than the request, you can use watch to perform state change, for now.

@nobelhuang
Copy link
Author

@HerringtonDarkholme Thanks for comments.

Can you give an example where called before/after render will give significant difference?...

Surely as you said, if state changing is disallowed in beforeUpdate, it doesn't matter it is called before or after render. But just because the doc and diagram indicate it allows, and I tried then found it led to infinite loop, the calling order matters. By the way, I also tried changing some states at the very beginning of render (I know it was wrong operation) to see if it can circumvent the situation, it turned out infinite loop too, so not only does calling order matter, but also ticks. Given I never read Vue source code, correct me here if wrong.

So changing state should be your use case.

I don't think changing state before render running is only my use case. React against doing that in componentWillUpdate, while still provides componentWillReceiveProps to allow such operation, because developers need this. So yes, if you think beforeUpdate should keep current behavior, it would be better to have a new lifecycle hook like componentWillReceiveProps.

Other than the request, you can use watch to perform state change, for now.

Actually, I was curious why I couldn't find any filed bugs regarding beforeUpdate weird behavior before I filed, then I guessed, just as you mentioned here, people could use watch to achieve the goal in most of cases. However, think about when the component will be re-rendered, it is not just when props changed, it could also be when the parent is forcing to update even if props are not changed at all. Then the watch approach is not sufficient. In addition, one uncommon case, what if there are many props, say 30, and I just want to do some state changes no matter which prop has been changed. In the way of watch, I have to specify all 30 props to be watched explicitly...

Anyway, if you think this is not a functional bug, it would be good to have a componentWillReceiveProps-like lifecycle.

Thanks again for you guys awesome works.

@HerringtonDarkholme
Copy link
Member

just because the doc and diagram indicate it allows

IMHO it should not, whenever it is called, and the new doc has removed that suggestion. Again, beforeUpdate executed before/after render only differs when you change state. So that is probably why no issue has yet been reported on this.

Please do provide a concrete example where you do need to change state in beforeUpdate.

I don't think changing state before render running is only my use case. React (...) provides componentWillReceiveProps to allow such operation, because developers need this.

I agree that is useful. And indeed it is not uncommon to have a component with large prop set. e.g. a bridging component for google map. What I have seen is developers uses helper function to dynamically watch props change. Say,

function dynamicWatch(props, vm, func) { 
  for (let key in props) 
     vm.$watch(key, func)
}

So to summarize the discussion above, can we list these 3 points?

  1. whether beforeUpdate allow state change
  2. should beforeUpdate be call before render.
  3. should Vue provide a propsUpdate helper

Does this wrap up the thread?

@nobelhuang
Copy link
Author

Okay, it seems that I have to provide a couple of examples. I came up with these examples randomly.

Example 1. jsfiddle
Imagine a Timer, initially you can set years, months, days, hours, minutes and seconds respectively to count down, these values can also be changed anytime when you want to start a new timer. However, before a new timer starts counting down, there would be a short pause, and there is a signal above the countdown to indicate if it is currently paused or counting down. As you can see, whenever and no matter which time props of Timer is changed, we need to update some states, like pauseHandle, countDownHandle and startpoint, and this could be done easily and efficiently in beforeUpdate if state changing is allowed. Of course, we can achieve the same result by using dynamicWatch, but the func might be invoked 6 times if all time props change at once.

Example 2. jsfiddle
Imagine a jigsaw puzzle game, we have a board component that can host all puzzle pieces. As pieces are added to the board, the board could show different background color as hint for the player indicating if he/she is approaching the goal. As you can see, there is actually no props changes that can be detected for TheBoard component, its state change is based the $slots.default which is not reactive, and it is the parent that is forcing TheBoard to re-rendered. It is convenient to update this state in beforeUpdate. Of course, I know we can use a method to work out that value replacing the use of boardColor, but if boardColor will be used many times in the template, that method would be called many times in one render process, in that case, I know we can write render function to alleviate, but don't you think it will be easier to do that in beforeUpdate?

I am feeling I lost my point through this thread. To wrap up, I only want to state that, because both the API doc and the lifecycle diagram consistently indicate you can change states in beforeUpdate, I don't think it was initially designed not to allow to do so, hence I think it's a functional bug.

Jinjiang pushed a commit to vuejs/v2.cn.vuejs.org that referenced this issue Jan 30, 2018
vuejs-jp-bot pushed a commit to vuejs-jp-bot/jp.vuejs.org that referenced this issue Jan 31, 2018
* Create new page

* Update

* Improve intro

* Update

* Add Example

* Include real world example and explanation about testing

* Added "Why vue" en, cn srt files into assets folder (vuejs#1367)

* added english srt file

* added chinese srt file

* typo

* fix: Correct sidebar anchor targets (vuejs#1366)

Change function execution order to correct sidebar anchor targets; more concise fix to vuejs/v2.vuejs.org#1348.

* Images not found. (vuejs#1365)

Hi,
The images link for `hn.png` and `hn-architecture.png` can be found on `../../images/`.

* [Doc EN]: `event.md` add space to new part (vuejs#1363)

* New in with + symbol

Signed-off-by: Bruno Lesieur <[email protected]>

* Review of 2.5.0 doc

Signed-off-by: Bruno Lesieur <[email protected]>

* Review

Signed-off-by: Bruno Lesieur <[email protected]>

* Fix syntax typo

Signed-off-by: Bruno Lesieur <[email protected]>

* Add space between new line of documentation

Signed-off-by: MachinisteWeb <[email protected]>

* Add @posva review

Signed-off-by: MachinisteWeb <[email protected]>

* Retrieve tweeningValue from onUpdate callback in documentation (vuejs#1350)

The tweeningValue no longer seems to be available in the tween object itself. Instead, the tweeningValue is available in the tweened object that passed as a parameter to the onUpdate callback.

* Add watch usages (vuejs#1356)

* Add watch usages!

Add `watch` usages!

* Update index.md

* tweaks to watch api examples

* Avoid updating license every year (vuejs#1353)

* Update spelling error and add some details about what we are testing

* Update

* typo for chinese subtitles of "why vue" video (vuejs#1371)

* Adding Why Vue.js video to Introduction page (vuejs#1377)

* Added video into guide introduction

* Added modal styles to page, put video line on one file, and reset iframe margin

* Fixed typo in Gulp example in deployment.md (vuejs#1376)

* Update deployment.md with Grunt example (vuejs#1375)

* Decoupled video player from Vimeo (vuejs#1374)

* Update Installation guide to use https://caniuse.com (vuejs#1372)

* [HOTFIX] initVideoModal error in common.js (vuejs#1378)

* Showing all possible params of watch() (vuejs#1380)

Surprised that the documentation didn't include the fact that the 2nd argument is the previous value. This can be really useful in some cases!

* refactor & update sponsors display

* include bronze data

* fix link

* fix link

* add build script

* update deploy docs

* Small fixes (vuejs#1381)

* fixed video modal bug in guide/index.md

* removed unnecessary sponsors content which has been in theme layout now

* fixed sponsors dropdown menu

* fixed button style in team page

* update Guillaume's core focus

* Improve based on Sarah Drasner feedback and fix some grammar

* Fixed TYPO Automatic Key Modifiers (vuejs#1388)

* update community deployment instructions

* Tweak wording of `.passive` modifier explanation

* Add guide link in Vue.filter API (vuejs#1394)

* Update filters, global filters needs to go before Vue instance creation (vuejs#1392)

Global filters defined after creating the Vue instance throws `Failed to resolve filter`. Reference https://forum.vuejs.org/t/global-filters-failing-to-resolve-inside-single-file-components/21863/6

* update tree-view example to add v-for key

* add details of object merging to mixins page

* Revise beforeUpdate API entry, fixes vuejs/vue#7481 (vuejs#1395)

* fix vue team distance sorting

* Add explicit version to download links (vuejs#1398)

* Add explicit version to download links

* improve CDN section of installation page

* demo from ’Object Change Detection‘ doesn't work (vuejs#1397)

vm.$set(this.userProfile, 'age', 27)    ==>   vm.$set(vm.userProfile, 'age', 27);

* Updated description of Weex (vuejs#1396)

* updated description of Weex (close vuejs#1382)

* Update comparison.md

* Update "incubated to" -> "incubated by"

* Update comparison.md

* Update comparison.md

* fix vue component require syntax for modern vue-loader

* The Web Optimization Project has optimized your repository! (vuejs#1389)

The Web Optimization Project optimized this repository. This commit contains the optimized files in this repository.

* Fix wrapperfind(error) typo and add example to test for whitespace

* Update

* Use factory function to save redundant logic

* Add factory function explanation and link to vue test utils guides.

* Update using codebryo feedback

* Change Github to GitHub (vuejs#1399)

* Fixed js error when click the page (vuejs#1401)

* Change cookbook entry number and reformat sentence

* Change order

* fix: typo in v-show description (vuejs#1408)

* Create new page

* Update

* Improve intro

* Update

* Add Example

* Include real world example and explanation about testing

* Update spelling error and add some details about what we are testing

* Update

* Improve based on Sarah Drasner feedback and fix some grammar

* Fix wrapperfind(error) typo and add example to test for whitespace

* Update

* Use factory function to save redundant logic

* Add factory function explanation and link to vue test utils guides.

* Update using codebryo feedback

* Change cookbook entry number and reformat sentence

* Change order

* Rebase
re-fort pushed a commit to vuejs/jp.vuejs.org that referenced this issue Feb 1, 2018
* Create new page

* Update

* Improve intro

* Update

* Add Example

* Include real world example and explanation about testing

* Added "Why vue" en, cn srt files into assets folder (#1367)

* added english srt file

* added chinese srt file

* typo

* fix: Correct sidebar anchor targets (#1366)

Change function execution order to correct sidebar anchor targets; more concise fix to vuejs/v2.vuejs.org#1348.

* Images not found. (#1365)

Hi,
The images link for `hn.png` and `hn-architecture.png` can be found on `../../images/`.

* [Doc EN]: `event.md` add space to new part (#1363)

* New in with + symbol

Signed-off-by: Bruno Lesieur <[email protected]>

* Review of 2.5.0 doc

Signed-off-by: Bruno Lesieur <[email protected]>

* Review

Signed-off-by: Bruno Lesieur <[email protected]>

* Fix syntax typo

Signed-off-by: Bruno Lesieur <[email protected]>

* Add space between new line of documentation

Signed-off-by: MachinisteWeb <[email protected]>

* Add @posva review

Signed-off-by: MachinisteWeb <[email protected]>

* Retrieve tweeningValue from onUpdate callback in documentation (#1350)

The tweeningValue no longer seems to be available in the tween object itself. Instead, the tweeningValue is available in the tweened object that passed as a parameter to the onUpdate callback.

* Add watch usages (#1356)

* Add watch usages!

Add `watch` usages!

* Update index.md

* tweaks to watch api examples

* Avoid updating license every year (#1353)

* Update spelling error and add some details about what we are testing

* Update

* typo for chinese subtitles of "why vue" video (#1371)

* Adding Why Vue.js video to Introduction page (#1377)

* Added video into guide introduction

* Added modal styles to page, put video line on one file, and reset iframe margin

* Fixed typo in Gulp example in deployment.md (#1376)

* Update deployment.md with Grunt example (#1375)

* Decoupled video player from Vimeo (#1374)

* Update Installation guide to use https://caniuse.com (#1372)

* [HOTFIX] initVideoModal error in common.js (#1378)

* Showing all possible params of watch() (#1380)

Surprised that the documentation didn't include the fact that the 2nd argument is the previous value. This can be really useful in some cases!

* refactor & update sponsors display

* include bronze data

* fix link

* fix link

* add build script

* update deploy docs

* Small fixes (#1381)

* fixed video modal bug in guide/index.md

* removed unnecessary sponsors content which has been in theme layout now

* fixed sponsors dropdown menu

* fixed button style in team page

* update Guillaume's core focus

* Improve based on Sarah Drasner feedback and fix some grammar

* Fixed TYPO Automatic Key Modifiers (#1388)

* update community deployment instructions

* Tweak wording of `.passive` modifier explanation

* Add guide link in Vue.filter API (#1394)

* Update filters, global filters needs to go before Vue instance creation (#1392)

Global filters defined after creating the Vue instance throws `Failed to resolve filter`. Reference https://forum.vuejs.org/t/global-filters-failing-to-resolve-inside-single-file-components/21863/6

* update tree-view example to add v-for key

* add details of object merging to mixins page

* Revise beforeUpdate API entry, fixes vuejs/vue#7481 (#1395)

* fix vue team distance sorting

* Add explicit version to download links (#1398)

* Add explicit version to download links

* improve CDN section of installation page

* demo from ’Object Change Detection‘ doesn't work (#1397)

vm.$set(this.userProfile, 'age', 27)    ==>   vm.$set(vm.userProfile, 'age', 27);

* Updated description of Weex (#1396)

* updated description of Weex (close #1382)

* Update comparison.md

* Update "incubated to" -> "incubated by"

* Update comparison.md

* Update comparison.md

* fix vue component require syntax for modern vue-loader

* The Web Optimization Project has optimized your repository! (#1389)

The Web Optimization Project optimized this repository. This commit contains the optimized files in this repository.

* Fix wrapperfind(error) typo and add example to test for whitespace

* Update

* Use factory function to save redundant logic

* Add factory function explanation and link to vue test utils guides.

* Update using codebryo feedback

* Change Github to GitHub (#1399)

* Fixed js error when click the page (#1401)

* Change cookbook entry number and reformat sentence

* Change order

* fix: typo in v-show description (#1408)

* Create new page

* Update

* Improve intro

* Update

* Add Example

* Include real world example and explanation about testing

* Update spelling error and add some details about what we are testing

* Update

* Improve based on Sarah Drasner feedback and fix some grammar

* Fix wrapperfind(error) typo and add example to test for whitespace

* Update

* Use factory function to save redundant logic

* Add factory function explanation and link to vue test utils guides.

* Update using codebryo feedback

* Change cookbook entry number and reformat sentence

* Change order

* Rebase
Jinjiang pushed a commit to vuejs/v2.cn.vuejs.org that referenced this issue Feb 1, 2018
Jinjiang pushed a commit to vuejs/v2.cn.vuejs.org that referenced this issue Feb 3, 2018
* Create new page

* Update

* Improve intro

* Update

* Add Example

* Include real world example and explanation about testing

* Added "Why vue" en, cn srt files into assets folder (#1367)

* added english srt file

* added chinese srt file

* typo

* fix: Correct sidebar anchor targets (#1366)

Change function execution order to correct sidebar anchor targets; more concise fix to vuejs/v2.vuejs.org#1348.

* Images not found. (#1365)

Hi,
The images link for `hn.png` and `hn-architecture.png` can be found on `../../images/`.

* [Doc EN]: `event.md` add space to new part (#1363)

* New in with + symbol

Signed-off-by: Bruno Lesieur <[email protected]>

* Review of 2.5.0 doc

Signed-off-by: Bruno Lesieur <[email protected]>

* Review

Signed-off-by: Bruno Lesieur <[email protected]>

* Fix syntax typo

Signed-off-by: Bruno Lesieur <[email protected]>

* Add space between new line of documentation

Signed-off-by: MachinisteWeb <[email protected]>

* Add @posva review

Signed-off-by: MachinisteWeb <[email protected]>

* Retrieve tweeningValue from onUpdate callback in documentation (#1350)

The tweeningValue no longer seems to be available in the tween object itself. Instead, the tweeningValue is available in the tweened object that passed as a parameter to the onUpdate callback.

* Add watch usages (#1356)

* Add watch usages!

Add `watch` usages!

* Update index.md

* tweaks to watch api examples

* Avoid updating license every year (#1353)

* Update spelling error and add some details about what we are testing

* Update

* typo for chinese subtitles of "why vue" video (#1371)

* Adding Why Vue.js video to Introduction page (#1377)

* Added video into guide introduction

* Added modal styles to page, put video line on one file, and reset iframe margin

* Fixed typo in Gulp example in deployment.md (#1376)

* Update deployment.md with Grunt example (#1375)

* Decoupled video player from Vimeo (#1374)

* Update Installation guide to use https://caniuse.com (#1372)

* [HOTFIX] initVideoModal error in common.js (#1378)

* Showing all possible params of watch() (#1380)

Surprised that the documentation didn't include the fact that the 2nd argument is the previous value. This can be really useful in some cases!

* refactor & update sponsors display

* include bronze data

* fix link

* fix link

* add build script

* update deploy docs

* Small fixes (#1381)

* fixed video modal bug in guide/index.md

* removed unnecessary sponsors content which has been in theme layout now

* fixed sponsors dropdown menu

* fixed button style in team page

* update Guillaume's core focus

* Improve based on Sarah Drasner feedback and fix some grammar

* Fixed TYPO Automatic Key Modifiers (#1388)

* update community deployment instructions

* Tweak wording of `.passive` modifier explanation

* Add guide link in Vue.filter API (#1394)

* Update filters, global filters needs to go before Vue instance creation (#1392)

Global filters defined after creating the Vue instance throws `Failed to resolve filter`. Reference https://forum.vuejs.org/t/global-filters-failing-to-resolve-inside-single-file-components/21863/6

* update tree-view example to add v-for key

* add details of object merging to mixins page

* Revise beforeUpdate API entry, fixes vuejs/vue#7481 (#1395)

* fix vue team distance sorting

* Add explicit version to download links (#1398)

* Add explicit version to download links

* improve CDN section of installation page

* demo from ’Object Change Detection‘ doesn't work (#1397)

vm.$set(this.userProfile, 'age', 27)    ==>   vm.$set(vm.userProfile, 'age', 27);

* Updated description of Weex (#1396)

* updated description of Weex (close #1382)

* Update comparison.md

* Update "incubated to" -> "incubated by"

* Update comparison.md

* Update comparison.md

* fix vue component require syntax for modern vue-loader

* The Web Optimization Project has optimized your repository! (#1389)

The Web Optimization Project optimized this repository. This commit contains the optimized files in this repository.

* Fix wrapperfind(error) typo and add example to test for whitespace

* Update

* Use factory function to save redundant logic

* Add factory function explanation and link to vue test utils guides.

* Update using codebryo feedback

* Change Github to GitHub (#1399)

* Fixed js error when click the page (#1401)

* Change cookbook entry number and reformat sentence

* Change order

* fix: typo in v-show description (#1408)

* Create new page

* Update

* Improve intro

* Update

* Add Example

* Include real world example and explanation about testing

* Update spelling error and add some details about what we are testing

* Update

* Improve based on Sarah Drasner feedback and fix some grammar

* Fix wrapperfind(error) typo and add example to test for whitespace

* Update

* Use factory function to save redundant logic

* Add factory function explanation and link to vue test utils guides.

* Update using codebryo feedback

* Change cookbook entry number and reformat sentence

* Change order

* Rebase
Jinjiang pushed a commit to vuejs/v2.cn.vuejs.org that referenced this issue Feb 4, 2018
* Create new page

* Update

* Improve intro

* Update

* Add Example

* Include real world example and explanation about testing

* Added "Why vue" en, cn srt files into assets folder (#1367)

* added english srt file

* added chinese srt file

* typo

* fix: Correct sidebar anchor targets (#1366)

Change function execution order to correct sidebar anchor targets; more concise fix to vuejs/v2.vuejs.org#1348.

* Images not found. (#1365)

Hi,
The images link for `hn.png` and `hn-architecture.png` can be found on `../../images/`.

* [Doc EN]: `event.md` add space to new part (#1363)

* New in with + symbol

Signed-off-by: Bruno Lesieur <[email protected]>

* Review of 2.5.0 doc

Signed-off-by: Bruno Lesieur <[email protected]>

* Review

Signed-off-by: Bruno Lesieur <[email protected]>

* Fix syntax typo

Signed-off-by: Bruno Lesieur <[email protected]>

* Add space between new line of documentation

Signed-off-by: MachinisteWeb <[email protected]>

* Add @posva review

Signed-off-by: MachinisteWeb <[email protected]>

* Retrieve tweeningValue from onUpdate callback in documentation (#1350)

The tweeningValue no longer seems to be available in the tween object itself. Instead, the tweeningValue is available in the tweened object that passed as a parameter to the onUpdate callback.

* Add watch usages (#1356)

* Add watch usages!

Add `watch` usages!

* Update index.md

* tweaks to watch api examples

* Avoid updating license every year (#1353)

* Update spelling error and add some details about what we are testing

* Update

* typo for chinese subtitles of "why vue" video (#1371)

* Adding Why Vue.js video to Introduction page (#1377)

* Added video into guide introduction

* Added modal styles to page, put video line on one file, and reset iframe margin

* Fixed typo in Gulp example in deployment.md (#1376)

* Update deployment.md with Grunt example (#1375)

* Decoupled video player from Vimeo (#1374)

* Update Installation guide to use https://caniuse.com (#1372)

* [HOTFIX] initVideoModal error in common.js (#1378)

* Showing all possible params of watch() (#1380)

Surprised that the documentation didn't include the fact that the 2nd argument is the previous value. This can be really useful in some cases!

* refactor & update sponsors display

* include bronze data

* fix link

* fix link

* add build script

* update deploy docs

* Small fixes (#1381)

* fixed video modal bug in guide/index.md

* removed unnecessary sponsors content which has been in theme layout now

* fixed sponsors dropdown menu

* fixed button style in team page

* update Guillaume's core focus

* Improve based on Sarah Drasner feedback and fix some grammar

* Fixed TYPO Automatic Key Modifiers (#1388)

* update community deployment instructions

* Tweak wording of `.passive` modifier explanation

* Add guide link in Vue.filter API (#1394)

* Update filters, global filters needs to go before Vue instance creation (#1392)

Global filters defined after creating the Vue instance throws `Failed to resolve filter`. Reference https://forum.vuejs.org/t/global-filters-failing-to-resolve-inside-single-file-components/21863/6

* update tree-view example to add v-for key

* add details of object merging to mixins page

* Revise beforeUpdate API entry, fixes vuejs/vue#7481 (#1395)

* fix vue team distance sorting

* Add explicit version to download links (#1398)

* Add explicit version to download links

* improve CDN section of installation page

* demo from ’Object Change Detection‘ doesn't work (#1397)

vm.$set(this.userProfile, 'age', 27)    ==>   vm.$set(vm.userProfile, 'age', 27);

* Updated description of Weex (#1396)

* updated description of Weex (close #1382)

* Update comparison.md

* Update "incubated to" -> "incubated by"

* Update comparison.md

* Update comparison.md

* fix vue component require syntax for modern vue-loader

* The Web Optimization Project has optimized your repository! (#1389)

The Web Optimization Project optimized this repository. This commit contains the optimized files in this repository.

* Fix wrapperfind(error) typo and add example to test for whitespace

* Update

* Use factory function to save redundant logic

* Add factory function explanation and link to vue test utils guides.

* Update using codebryo feedback

* Change Github to GitHub (#1399)

* Fixed js error when click the page (#1401)

* Change cookbook entry number and reformat sentence

* Change order

* fix: typo in v-show description (#1408)

* Create new page

* Update

* Improve intro

* Update

* Add Example

* Include real world example and explanation about testing

* Update spelling error and add some details about what we are testing

* Update

* Improve based on Sarah Drasner feedback and fix some grammar

* Fix wrapperfind(error) typo and add example to test for whitespace

* Update

* Use factory function to save redundant logic

* Add factory function explanation and link to vue test utils guides.

* Update using codebryo feedback

* Change cookbook entry number and reformat sentence

* Change order

* Rebase
@fnlctrl
Copy link
Member

fnlctrl commented Feb 11, 2018

Reopening as @Kingwl probably closed it accidentally in a PR to his fork

@fnlctrl fnlctrl reopened this Feb 11, 2018
ntkme added a commit to vue-relay/vue-relay that referenced this issue Mar 3, 2018
ntkme added a commit to vue-relay/vue-relay that referenced this issue Mar 3, 2018
@ntkme
Copy link

ntkme commented Mar 4, 2018

I have a real use case in vue-relay, where I need to listen to any change in vm.$props, then re-fetch from relay store and update the data accordingly before render().

I attempted the following methods which all have their own issue.

Re-fetch in beforeUpdate()

This will trigger another re-rendering because it's fired after render().

Watch All Props Separately

This works correctly, but it brings a performance penalty: when multiple props changed at once, callback runs multiple times with the latest version of $props. I have logic to avoid running re-fetch the same data multiple time, but that logic still has to run multiple times.

Watch vm.$props

Ideally this is exactly what I want to do but apparently it's a proxy that never changes.

Watch a Computed Value That Uses All Props

Modified based on the suggestions from #844, this method works perfectly that it only triggers callback once.

export default {
  data () {
    return {
      updateSwitch: true
    }
  },
  computed: {
    update () {
      // Read props that need to be listened for changes.
      Object.keys(this.$props).forEach(key => this[key])
      // Return a different value each time. `Date.now()` is not guaranteed to be unique.
      return (this.updateSwitch = !this.updateSwitch)
    }
  },
  watch: {
    update () {
      // Use this as a beforeRender hook.
    }
  }
}

At some level this issue can be worked around by the last method, but it's really ugly... It would be nice if we can have a lifecycle hook that addresses this issue natively.

@Kingwl
Copy link
Member

Kingwl commented Mar 5, 2018

sorry about that
this is a wrong operation

@yyx990803
Copy link
Member

This is considered a bug and the behavior has been adjusted to match the lifecycle diagram and the previous docs description. We can revert the docs after 2.6 is out.

@Jinjiang
Copy link
Member

Jinjiang commented Aug 9, 2018

I'm not sure whether it is still a bug, but in this case below, the class attribute of the root element in real DOM is still rendered before beforeUpdate hook.

So I miss the time to access the existing class attribute before and cannot add/remove class in this.$vnode.data.class or this.$vnode.data.staticClass.

http://jsfiddle.net/eywraw8t/254766/

@yyx990803 @chrisvfritz 🧐

@Matheus-Ribeiro95
Copy link

@ntkme, thanks bro! This saved me.

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

Successfully merging a pull request may close this issue.

10 participants