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

fix(runtime-core): should pause tracking deps when initializing legacy options #2524

Merged
merged 1 commit into from
Nov 27, 2020
Merged

fix(runtime-core): should pause tracking deps when initializing legacy options #2524

merged 1 commit into from
Nov 27, 2020

Conversation

HcySunYang
Copy link
Member

close: #2521

@edison1105
Copy link
Member

The second problem is not fixed.

@LinusBorg
Copy link
Member

Yep. It seems we also need to pause Tracking in doWatch, not sure if we should just wrap the whole thing ...

@HcySunYang
Copy link
Member Author

The second problem is not fixed.

I don't think the second problem is a bug or something that needs improvement. When the props change, the component does need to be updated, and I think the update is not equivalent to re-rendering, of course, it includes re-rendering.

Yep. It seems we also need to pause Tracking in doWatch, not sure if we should just wrap the whole thing ...

Not only the watch but also lifecycle hooks such as the created, I think it behaves similarly to setup: https://github.com/vuejs/vue-next/blob/master/packages/runtime-core/src/component.ts#L566-L573

@skirtles-code
Copy link
Contributor

I'm unclear what people mean when they say 'the second problem'. In my original bug report I mentioned two problems, in bullet points 1 and 3. The contents of bullet point 2 were a necessary step in reproducing the problem in bullet point 3.

While it could be argued that the behaviour in bullet point 2 is also a bug (in my opinion it is) it is something that has been reported and discussed previously (see #1790 and #1181) and it wasn't what I was reporting in #2521.

@edison1105
Copy link
Member

I'm unclear what people mean when they say 'the second problem'. In my original bug report I mentioned two problems, in bullet points 1 and 3. The contents of bullet point 2 were a necessary step in reproducing the problem in bullet point 3.

While it could be argued that the behaviour in bullet point 2 is also a bug (in my opinion it is) it is something that has been reported and discussed previously (see #1790 and #1181) and it wasn't what I was reporting in #2521.

image

@skirtles-code
Copy link
Contributor

OK.

@HcySunYang The 'second problem' is not directly about re-rendering when the props change. That's just a necessary step along the way. The problem is what happens the next time the data changes after that.

@yyx990803 yyx990803 merged commit 0ff2a4f into vuejs:master Nov 27, 2020
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

Successfully merging this pull request may close these issues.

Data read in watch being treated as a rendering dependency
5 participants