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

cleanupEffect missing edge test case #5541

Closed
sudongyuer opened this issue Mar 8, 2022 · 1 comment · Fixed by #9503
Closed

cleanupEffect missing edge test case #5541

sudongyuer opened this issue Mar 8, 2022 · 1 comment · Fixed by #9503
Labels
🧹 p1-chore Priority 1: this doesn't change code behavior. scope: reactivity

Comments

@sudongyuer
Copy link
Contributor

Version

3.2.31

Reproduction link

github.com

Steps to reproduce

I want to optimize cleanupEffect code ,and when i changed the code,the tests all passed,that makes me think my change is right,but actually, it's wrong

before

function cleanupEffect(effect: ReactiveEffect) {
  const { deps } = effect
  if (deps.length) {
    for (let i = 0; i < deps.length; i++) {
      deps[i].delete(effect)
    }
    deps.length = 0
  }
}

now

function cleanupEffect(effect: ReactiveEffect) {
  const { deps } = effect
  if (deps.length) {
      deps[0].delete(effect)
    deps.length = 0
  }
}

What is expected?

All tests should fail

What is actually happening?

All tests passed


I want to optimize cleanupEffect code ,and when i changed the code,the tests all passed,that makes me think my change is right,but actually, it's wrong

@sudongyuer
Copy link
Contributor Author

#5537 add edge test case

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
🧹 p1-chore Priority 1: this doesn't change code behavior. scope: reactivity
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants