-
Notifications
You must be signed in to change notification settings - Fork 668
Add setComputed method #55
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
Comments
A computed property should be something you test...not an invariant
…On Sep 29, 2017 9:19 AM, "Edd Yerburgh" ***@***.***> wrote:
We should add a setComputed method to set the value of a computed value.
—
You are receiving this because you are subscribed to this thread.
Reply to this email directly, view it on GitHub
<#55>, or mute the thread
<https://github.com/notifications/unsubscribe-auth/AACounMb_7NTE8-W5Wj4tn5y5Me20M7Xks5snIwFgaJpZM4PoPjH>
.
|
I agree, most of the time you should test it, but if there's a use case for setting a computed property I think we should add it. I think @jackmellis described a use case well:
|
Only just saw this after creating a similar issue on the Avoriaz repo. A |
I can see a pragmatism to this. I do agree with @blocka though and think that the downside with setComputed is that it is encouraging the use of computed properties in the same way you would use something like props or data. You could end up in a situation where you are unit tested 100% by setting computed and have a component that does not work. Guess it is a black box vs white box test thing. I'm interested in the scenario/thoughts for needing this and if it would be better to create a separate component that you pass the computed property to as a prop and then test that component? |
@Austio why not make both options available and let people decide for themselves? |
@jonnyparris that is a fair point, i think with the feature we should add something to documentation with some potential gotchas with it. |
I made a PR — #84 |
What do you think about adding something like
It's only skeleton of idea, I'm not sure if it's even possible. |
I think we should add setComputed, because we already have |
Can you please add this method to docs? Thank you |
Thanks for reminding me. I'll add it to my list. If you are able make a PR it would be very helpful! |
any hope on bringing back this feature, What if my computed value was set by getters from vuex and im mocking the getters? And then i want to test a watcher that is listening for changes on that value? I would want a way to force update the computed value |
Can't you just mock vuex in that case? Or at least mapGetters?
…On Apr 17, 2018 2:36 AM, "jorySsense" ***@***.***> wrote:
any hope on bringing back this feature,
What if my computed value was set by getters from vuex and im mocking the
getters?
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<#55 (comment)>,
or mute the thread
<https://github.com/notifications/unsubscribe-auth/AACouiwNquto6fztzBR8ghkGvBi8Xrk5ks5tpSr7gaJpZM4PoPjH>
.
|
Sure the works for mounting but you can't trigger the watcher on mount. You need to update the reactive data that the watcher is observing. If it's a computed value, or a getter thats mocked from the store you can't force the computed value to update. |
Whilst I agree a 'setComputed' method should be added, you actually don't need it.
|
@nath-codes thats great for mounting, the issue is further explained #331 |
We should add a
setComputed
method to set the value of a computed value.The text was updated successfully, but these errors were encountered: