-
Notifications
You must be signed in to change notification settings - Fork 546
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
Final Reactivity API review #157
Comments
About ProxiesProxies have one major drawback: they break referential identity by introducing two different references for the same object. From various examples in the comments of other issues, it's clear that even simple cases can quickly lead to bugs if you try to mix non-proxies with deep proxies. And Vue makes every state deeply proxified by default (👉 motivation: automatically unwrap refs in templates). After a lengthy discussion in the comments of #121 @LinusBorg suggested that the best solution to this problem is to establish the best practice: everything in your state should be a proxy, whether reactive or not. For additional safety I suggest to add: proxy at creation time. When you new an object, make a copy or deserialize, e.g. from network, wrap it in a proxy and loose the original immediately. If you don't keep the raw object around, you can't mess it up. So the idea is to call
Sometimes you know that state is immutable. Or maybe it's mutable but you don't want or don't need it to be reactive. For those cases I propose
Why do we need a new proxy?
|
Thanks @jods for the detailed review!
Exporting multiple functions makes the code more self-explanatory: other devs reading your code don't need to lookup the docs to understand what the second mysterious
This is an interesting point - however, I can hardly imagine this being useful in anyway. The underlying rule of thumb is that "computed doesn't do anything with the values you return" - which is imo more straightforward than an additional rule of "if you return a ref from a computed it also unwraps it".
Again, the idea is "computed doesn't do anything with the values you return". Take a computed that maps a reactive array for example - the resulting value is a non-reactive array containing reactive objects. This non-reactive array is created afresh on every computation - it's not meant to be mutated or observed, and users will not expect to do so because it is essentially ephemeral. Users would never pass such ephemeral objects to Assuming there's no
This is unfortunately the only way to make it behave without ambiguity. In practice, I've also found that whenever the user intentionally puts refs in an array, it is typically in relatively advanced use cases and they do expect to keep the ref identity instead of treating the array as plain values. A computed property can be used to map the original refs array to unwrapped version.
This has been updated in a previous version and I believe they are now available at all times (if available for the type of operation). I'll have to sign off for the weekend, but I'll address some of the other points (in particular |
Does it make sense then to make the value of computed readonly? Or does the "computed doesn't do anything with the values you return" rule apply in terms of types as well? Vue Composition API marks it as readonly: and Vue 3 doesn't |
@yyx990803 Just to be clear, I'm fine with ⚠ points -- or at least I don't have better alternatives to propose. I just listed everything to be exhaustive.
I think this is very important. From various examples it became clear that if you start putting non-reactive objects in your state, you can very quickly get unwanted reactive variants and then bugs. Here's one example: I am building a list of static (non-reactive) After discussing this with @LinusBorg I believe there are 2 solutions:
It would be problematic if the value returned by computed was readonly. Take the selected item example one step further: |
I'm gonna keep the issue description updated with the feedback from comments.
Seems to me |
i agree with @yyx990803 here, for further reading on this topic:
my gut feeling also likes the first solution more than the second one. the major problem i see here is not that we can not agree on of the two (or come up with an even better one), but the time it would take. everyone is eager for a vue 3 stable release. both solutions seem like a compromise which are not 100% satisfying and since it is such an integral part of how the whole framework is supposed to work and be used, i personally would be ok with waiting a little bit longer until these foundations are rock solid. |
You still are able to modify the item even if it's in readonly array As you see here |
@jacekkarczmarczyk The depth isn't important for the argument. If you prefer to stick with arrays: your computed doesn't necessarily "compute a new array" (e.g. filter) which doesn't make much sense for modifying but it might "pick an array amongst several existing ones" and you might want to modify it. Random example: you build a tool to input statistical data (e.g. time series) you have a switch to input the male and female data in a single UI: const male = reactive([14, 6, 34]);
const female = reactive([ 12, 3, 10, 9]);
const isEditingMale = ref(true);
// model.value is an array and I intend on modifying it
const model = computed(() => editMale.value ? male : female); Why would you prefer to disallow mutation of the value returned by getter? |
Note that if you want to follow Martin Fowler's advice, then it's Anyway this isn't worth arguing about. If importing a second function named |
Updated issue description |
This was added by vuejs/core#909 (which was a mistake on my part) and reverted by vuejs/core#961.
This is needed in some internal use cases (e.g. devtools integration) where we cannot assume
Good catch - this is in fact a leftover from previous iterations of the readonly implementation. We should remove it.
|
Updated issue description
|
|
Noticed an issue with the Promise example. |
I agree a factory pattern should be more flexible, however, you should not be constructing the ref object yourself. A ref by definition only has a single Therefore I think the factory should still return the object in the format of an options object containing |
I agree adding other properties is an error. A ref is by definition a single value and with unwrapping (in views or
I disagree with this, though. An "object containing the ref and the functions" isn't as good because that object isn't a ref itself, so you'll have to separately pass around the ref where you want to use it (to functions, to the view) and its wrapper wherever you want to manipulate it. That breaks encapsulation: keeping data and methods that act on data together. What's so bad with adding methods on ref? I can do it anyway: let x = ref(0);
x.double = function() { this.value = this.value * 2 } For consumers of this api, returning If you look at previous art (e.g. Knockout, Mobx), there are examples of this:
BTW: Mobx is really cool, it integrates with every framework including Vue 2. Maybe they could use some of this stuff for tighter integration with Vue 3. I have no idea if it'll help them, maybe they should be part of this discussion? |
Well, when you put methods on a ref:
IMO the MobX utilities that arbitrarily extends built-ins lacks consistency and is messy. It is my recommendation to stay away from attaching custom properties, and in general Vue composition API encourages separating state (reactive objects and refs that contains only state) and methods (no reliance on That said, if you intend on using it in a different style because of your preference, I can't stop you from doing it. But it's also not my priority to convince you to change your programming style, as long as the API allows you to do everything you want to do. |
I edited the I think most of the practical concerns have been addressed in this review - some style preferences can be argued, but that shouldn't have significant influence on the API shape. Closing for now in order to move forward, feel free to continue the discussion though. |
To be honest, we were disappointed to see this function being removed for the exports. Our Vugel project is a WebGL canvas renderer for vue3. The aim of Vugel is to provide a framework for high-performance rendering of many elements. Use cases include games, infographics and gantt charts. In the specific case of a HTMLCanvasElement, the opportunity arises to integrate the renderer with a 'normal' vue application by using a component that wraps around a HTMLCanvasElement generated by vue. This is exactly what we've done. The benefit is that vue properties and state mechanisms, such as vuex, can be reused between HTML parts and WebGL parts of the application. That would be really nice, as in our own use cases we'd like to use HTML for forms and tables, and WebGL for charts with huge amounts of elements. While they share the same state (partially) and data. We think that using |
@basvanmeurs to avoid going off-topic let's continue this here |
Great, thanks! I think this modified Identity Hazard PatternsThe new The only difference is that it doesn't allow writes, trigger or not. I agree with you that writing to state objects that are non-tracking is an edge case. It is still a valid case (e.g. if you want to use a signal pattern), but you can achieve it by keeping the raw object somewhere, or extracting it with I think this is a satisfying API for handling identity confusion 👍 I also agree with you that this is just style, people can do whatever they want. Yet it might be more of a problem than you think, though. Story: the very first day I put a dev on my team to work with Vue 3, he encountered a hard to understand identity issue. Before you ask: he didn't call I think it would be worthwhile that Vue 3 examples and tutorials put forth that all state created in setup should be
I'll follow-up on "custom reactive" and update the issue description with recent changes later. |
Correct. I was working on an update to the Composition API Reference just now and realized this would also require the following changes:
|
@yyx990803 It's been a few alpha since Given that this is a fairly common thing and given that setup() {
const dom = ref()
watchEffect(() => dom.value.focus(), { mounted: true })
return { dom }
} |
@jods4 i think your example is bad practice, because it induces wrong typing: for this specific use case, you can use the safe navigation operator: setup() {
const dom = ref<HTMLElement | undefined>()
watchEffect(() => dom.value?.focus())
return { dom }
} |
@yyx990803
these return true for |
@jods4 if it can be done with @backbone87 no, only |
convenience? it's a common thing when using DOM. It was handy in the first alphas. |
sorry, if being ignorant/unaware about existing info: what are the use-cases to check for isReactive/isProxy separately over isRef? i cant make up any use-case (except for library authors maybe), where i want to use any of the is* functions. edit: i am just trying to categorize these is* functions "you should use it when you write components" or "you usually use these only in certain circumstances like libraries/generic functionality". guess this is also a question that would be answered in the docs. is there a working draft of the vue 3 docs out there? |
Yes, these are "utilities" that are typically used only in libraries. Think of them as "rarely needed but it wouldn't make sense it they are not available" See updated Composition API Reference: https://composition-api.vuejs.org/api.html |
Updated issue description
👉 I'm gonna keep this issue up to date until 3.0 release so feel free to use it as a cheatsheet if you want. I'll stop after 3.0 because I don't think a closed github issue is the right place for documentation in the long term. |
@yyx990803 it's a nit but when updating the issue I noticed Also nit: still finding |
I agree regarding the naming of enable vs. pause. As it stands currently The name In looking through all the uses of enable / pause they are always paired with And while it doesn't resolve the intution mismatch of pairings, I think that Thoughts? |
Updated issue description |
Follow-up on custom reactives(sorry it's been a while, been busy lately)
So I think that's a closed topic for v1. I'm just gonna note that I don't agree with you on how unimportant this is.
Computed public API#163 made me realize that it's not totally clear if some part of Computeds expose To me, this doesn't look much like other APIs you expose publicly and I wonder if this is a conscious choice or if you'd rather expose a Note that if |
First of all: reactivity as it currently is, is already awesome. What an amazing toolbox, and I keep finding new uses and design patterns while working more with it. Big thumbs up for Evan and all other guys involved. That said, I think that we can extend the reactivity toolbox a bit more, making it more versatile, will certainly benefit the developers (and its' popularity). I already started building an application in vue3 and experienced some things that are lacking. First of all, I'm missing a way to trigger a ref manually, even though values do not change. It can be very handy to cause all observers to update. I resorted to I also ran into this other scenario where I wanted to get the current value of some ref without tracking it. It had to do with setting the scroll position on the div, while also listening to onScroll events, but I bet that there are other use cases as well. I think it's a bad idea to expose |
I'm pretty sure there already is, and is even exported by
|
Another thing I experienced while working with the composition API. It was, at first, confusing to me what to use in case of a reactive deeply nested object: const limits = ref({sx: 0, sy: 0, ex: 0, ey: 0}); Or: const limits = reactive({sx: 0, sy: 0, ex: 0, ey: 0}); In general both work fine in templates. In practive so far I've found that But there I experienced one super annoying thing about using watch([toRef(limits, "sx"), toRef(limits, "ex"), toRef(limits, "sy"), toRef(limits, "ey")], () => {...}) Or requiring conversion using iterators.. watch([someOtherRef, ...Object.values(toRefs(limits))], () => {...}) Or requiring a new variable.. const refs = toRefs(limits);
watch([refs.sx, refs.sy, refs.ex, refs.ey], () => {...}) In my humble opinion 'less is more'. We should provide some utility so that we don't need to create ugly code, use Object.values() or introduce unnecessary variables every time we want to use My proposal is to add the toRefsArray, by adding an argument allowing to select a subset instead of all refs: export function getRefsArray<K extends object>(reactiveObject: K, properties?: (keyof K)[]): Ref<any>[] {
if (properties) {
return properties.map((property) => {
return toRef(reactiveObject, property);
})
} else {
return [...Object.values(toRefs(reactiveObject))] as Ref<any>[];
}
} Allowing the syntax: watch(toRefsArray(limits, ["sx","sy","ex","ey"]), () => {...})
Or
watch([someOtherRef, ...toRefsArray(limits)], () => {...}) The syntax is much less verbose and readable than the others in my opinion. And is more efficient as no temporary variables or Object.values() conversions are required. |
... if you need their individual previous values or really only want to watch the first level of properties for changes. Otherwise you can do this: watch(() => limits, (obj) => ...., { deep: true }) Just be aware that the new and old "value" here will be a reference to the same object.
Not sure I see requirement for such fine grained control in core. there's definitely situtions where this might be handy but it's iterally 3 lines of code to do yourself, and in most situations, either a deep watch or simple |
@basvanmeurs Most of the time the easiest thing to do is use Just write your code in
|
I had previous threads about reactivity (#129, #121) but they evolved a lot in the comments so I think a clean plate would be helpful.
Key:
⚙Core API (exported by
vue
);🧪Advanced API (exported by
@vue/reactivity
);👉 Worth noting.
⚠ Pitfall, potential issue.
📣 Should be discussed.
Ref
⚙
Ref<T> = { value: T }
is the interface that describes a single reactive value.⚙
isRef(x: any): boolean
can be used to check if something is a ref.⚙
unref(x: T | Ref<T>): T
returns a value, unwrapping if it's a ref. Very useful helper for generic code (e.g. libraries) which may not know if a parameter is reactive or not.⚙
ref(val: T): Ref<T>
CanonicalRef
implementation: a single read/write reactive value.⚙
shallowRef(val: T): Ref<T>
Shallow variant ofref
, its value won't be made reactive when read.⚙
computed<T>(get: () => T): Ref<T>
⚙
computed<T>({ get(): T, set(val: T): void }): Ref<T>
A computed is a reactive cache for a computation. The main differences with a regular getter are:
⚙
triggerRef(ref: Ref): void
forces a trigger (change) of a Ref, even if it did not change.Can be used to easily create signals (manual change tracking when mutating non-reactive graphs). Works with any kind of ref (ref, shallow, computed, custom).
Reactive
No type/interface here, as reactive objects are regular objects!
⚙
isProxy(x: object): boolean
indicates if an object is a reactive proxy, of any kind (reactive
,shallowReactive
,readonly
,shallowReadonly
).⚙
reactive(x: T): T
Canonical implementation of a reactive object, returns a proxy that wraps the original object and tracks reads / trigger writes.⚙
shallowReactive(x: T): T
Shallow variant ofreactive
, properties won't be made reactive when read.⚙
isReactive(x: object): boolean
indicates if an object is reactive, including readonly objects that wrap reactive objects.⚙
readonly(x: T): T
Creates a readonly proxy around an object. It's considered reactive:isReactive
is true and as such this prevents objects from being made reactive, e.g. inside deep reactive object graphs.⚙
shallowReadonly(x: T): T
Shallow variant ofreadonly
, properties won't be wrapped with readonly when read.⚙
isReadonly(x: any): boolean
checks if the object is a readonly proxy. Note thatisReactive
would also return true.Watching
⚙
watchEffect(effect, options)
Runseffect
and run it again any time its dependencies change.⚙
watch(getter, watcher, options)
Evaluatesgetter
and pass its value towatcher
. Do it again anytimegetter
value changes.Low-level primitives
🧪
track(target, type, key)
🧪
trigger(target, type, key, newValue?, oldValue?, oldTarget?)
Those two functions are what enables read/write detections. They are not meant to be used directly (hence not exported by
vue
) but could be used as building blocks to create new lightweight primitives without allocatingref
orreactive
.🧪
effect(fn, options)
Runsfn
and runs it again when its dependencies change. It's the low-level primitive used to implementwatch
andwatchEffect
.🧪
toRaw(object: T): T
Returns the raw object wrapped by a proxy (any kind: readonly, reactive).🧪
pauseTracking()
Can be used to read values without taking the dependencies.🧪
enableTracking()
Can be used to track again in a nested block insidepauseTracking
(doesn't revertpauseTracking
, seeresetTracking
).🧪
resetTracking()
Resumes tracking state as it was before the lastpauseTracking
orenableTracking
call.Extending primitives
⚙
customRef(factory: (track: () => void, trigger: () => void) => { get(): T, set(value: T) }): Ref<T>
Enables the creation of refs with custom logic inside, such as debouncing/throttling, wrapping Promise or Observable and more.⚙
markRaw(object: T): T
prevents an object from being wrapped byreactive
.Deconstructing
⚙
toRef(reactive: object, property: string): Ref
returns a Ref that forwards toreactive[property]
.⚙
toRefs(reactive: object): { [prop]: Ref }
transforms a reactive object into a set of refs that forward to its properties.Out of scope
I worked on this issue to help freeze a good api for
reactivity
, which is at the core of Vue.There are related topics that could be reviewed such as: what's the best patterns for mixins, how should Vue unwrap in views, etc.
I kept those out of this issue as they are not in
reactivity
itself but build on top of it.The text was updated successfully, but these errors were encountered: