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

computed value becomes undefined inside onUnmounted hook unless it's accessed before #3099

Closed
kiaking opened this issue Jan 26, 2021 · 3 comments
Labels
🔨 p3-minor-bug Priority 3: this fixes a bug, but is an edge case that only affects very specific usage. 🐞 bug Something isn't working scope: reactivity

Comments

@kiaking
Copy link
Member

kiaking commented Jan 26, 2021

Version

3.0.5

Reproduction link

https://codesandbox.io/s/priceless-lalande-eh4wx?file=/src/views/Home.vue

Steps to reproduce

  1. Go to Codesandbox.
  2. Open console window in Codesandbox.
  3. First access "Home" page, then move to "About" page.
  4. You'll see computed value becoming undefined.
  5. If you un-comment console.log(computedObj.value) right above the onUnmounted hook. The value becomes visible inside onMountedhook.

What is expected?

Not sure. Is this expected?

What is actually happening?

Not sure. I think it would be OK for computed or any other reactive value to be undefined inside onUnmounted, but I think it would be better to have consistent behavior.

@LinusBorg
Copy link
Member

in the unmounted stage, effects have been stopped, and stopped effects (if using a scheduler) return undefined:

https://github.com/vuejs/vue-next/blob/7f086369003b50101849f4bcb8dd2978c1ea87ff/packages/reactivity/src/effect.ts#L86-L88

In that sense, it's expected.

But if we don't want this behaviour for computed, we could implement a check for effect.active in computed's implementation, and run effect.raw() for stopped computeds, around here:

https://github.com/vuejs/vue-next/blob/7f086369003b50101849f4bcb8dd2978c1ea87ff/packages/reactivity/src/computed.ts#L50-L57

The downside would be that the value would no longer be cached, but at least consumers should still be able to get the value.

@kiaking
Copy link
Member Author

kiaking commented Jan 26, 2021

Yeah that's true. Getting back cached computed value sounds right. I think this is tiny issue not sure if it's worth doing something about it, but I can understand it could be confusing because if the computed is accessed once, it looks like computed is working inside unmounted hook... maybe we should document it as an edge case...?

@LinusBorg
Copy link
Member

I'd personally rather make the change I proposed. The value may not be cached, but you will likely access it once only in that hook, so there's no real performance impact to be expected.

@HcySunYang HcySunYang added 🐞 bug Something isn't working 🔨 p3-minor-bug Priority 3: this fixes a bug, but is an edge case that only affects very specific usage. labels Apr 2, 2021
@github-actions github-actions bot locked and limited conversation to collaborators Oct 19, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
🔨 p3-minor-bug Priority 3: this fixes a bug, but is an edge case that only affects very specific usage. 🐞 bug Something isn't working scope: reactivity
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants