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

throw Cannot read properties of undefined (reading 'stop') on component unmount, and use nested watch. #5783

Closed
BiosSun opened this issue Apr 22, 2022 · 5 comments
Labels
has workaround A workaround has been found to avoid the problem 🐞 bug Something isn't working need guidance The approach/solution in the PR is unclear and requires guidance from maintainer to proceed further.

Comments

@BiosSun
Copy link

BiosSun commented Apr 22, 2022

Version

3.2.30

Reproduction link

sfc.vuejs.org/

Steps to reproduce

  1. please click 'Toggle' button in demo, you will see the error.

What is expected?

When the 'Toggle' button is clicked, the <Comp> component is unmounted normally.

What is actually happening?

'Toggle' button then change visible to false, then it will cause the <Comp> to be unmount, and throw error.


The issue is present in 3.2.30 and later.

@posva
Copy link
Member

posva commented Apr 22, 2022

I don't think you are supposed to call the watch within a watch because then they are not related. You could however move the watch outside or create a nested effectScope to create watchers whenever you want like this

@posva posva added need guidance The approach/solution in the PR is unclear and requires guidance from maintainer to proceed further. has workaround A workaround has been found to avoid the problem labels Apr 22, 2022
@BiosSun
Copy link
Author

BiosSun commented Apr 23, 2022

Yes, there are many other ways.

But in my project, I want to do is when a property (e.g. visible) of the component is toggled to true, turn on some function, and start watching for the change of state related to this function. And when it toggled to false , the function is turn off, and stop watch.

So, "naturally" I use nested watch. (There may be other, more natural and better ways, which I don't know of.)

Also, since this is an unreasonable way and it does cause an error, is there any way to stop people using it?

@posva
Copy link
Member

posva commented Apr 23, 2022

You can use the solution I showed you at the moment.

I don't know if it makes sense to activate the watcher effectScope within a running watcher

@edison1105
Copy link
Member

edison1105 commented Apr 26, 2022

@posva @LinusBorg
I feel this is a bug,because effects may be changed when an effect stoped.thats why the error is shown.
2993a24#diff-ac9e1628a205e241a9796fcffbdb27ae0dda80494c45a0830ee67309a1e3052fR57-R66

@edison1105 edison1105 added the 🐞 bug Something isn't working label Apr 26, 2022
@Zelig880
Copy link

Hi guys, I just wanted to know that I was going to raise this precise bug.

During Unit Test development we have started to encounter this issue after upgrading from 3.2.27 to 3.2.37. The change that breaks the test is the one highlighted above.

In the previous implementation this.effects.forEach(e => e.stop()) would run with no problem, but in the current implementation the value of "l" changes and breaks the loop.

After some investigation, we have noticed that the value of "l" (the variable used to handle the length of the array) and the actual array length, do not match and this is resulting in undefined (I have not analysed the reason behind this).

The following screenshot shows the console log and its miss-alignement.

image

Updating the for loop without reusing the "l" variable would fix the issue, but I know that the problem is somewhere else and this is not the solution, but i wanted to provide you all the context that I could as I cannot create a reproducible code.

for (i = 0, i = this.effects.length; i++) {
    this.effects[i].stop();
}

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
has workaround A workaround has been found to avoid the problem 🐞 bug Something isn't working need guidance The approach/solution in the PR is unclear and requires guidance from maintainer to proceed further.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants