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

Final Reactivity API review #157

Closed
jods4 opened this issue Apr 10, 2020 · 39 comments
Closed

Final Reactivity API review #157

jods4 opened this issue Apr 10, 2020 · 39 comments

Comments

@jods4
Copy link

jods4 commented Apr 10, 2020

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.

Notes: the TS type has a brand-check with a non exported symbol, which means user code can't create Ref instances. At runtime there is no symbol, it's actually just a check on _isRef: true.

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> Canonical Ref implementation: a single read/write reactive value.

👉 Primitives are deep by default. If the value is an object, it'll be wrapped with reactive automatically when read.

shallowRef(val: T): Ref<T> Shallow variant of ref, 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:

  • It caches its result. This is beneficial when the getter is costly but rarely changes.
  • It creates a single reactive dependency for its consumers (although it has to listen to the getter dependencies itself, of course). This is beneficial when there are many consumers of an expression that has many dependencies (N + M instead of N * M).

👉 computed is lazy: it only recomputes its value when someone tries to read it.
👉 Vue exports a different computed, which is tied to the currently running Component, than @vue/reactivity.
⚠ Like ref, but unlike other reactive primitives, computed does not automatically unref its result. Being a Ref itself it would be weird if it returned a Ref -- and if it does it's something that can easily be fixed by unref in the getter code.
⚠ Unlike other primitives, computed is not deep: it doesn't wrap its results in reactive proxies. The idea might be that when its result (e.g. a filtered array) changes, it would return a new array; or that the getter will return a reactive value when it makes sense to. This can be a pitfall as it may introduce non-proxified instances in the state graph (more about this in my first comment).
👉 Those last two points are consistent with how a plain function called from a template behaves.

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.

👉 Proxies are cached, calling reactive twice on the same object returns the same proxy.
👉 Idempotent: does nothing if passed an already reactive object.
👉 Primitives are deep by default. If a property value is an object, it'll be wrapped with reactive when read.
👉 Automatically unref refs when read from properties.
⚠ Works on array, but then doesn't unwrap! This was motivated by the fact that re-ordering an array of values (refs) is impossible if the array auto-unwraps them. vuejs/core#737

shallowReactive(x: T): T Shallow variant of reactive, properties won't be made reactive when read.

⚠ Shallow version doesn't unref, even at first level!

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.

👉 Primitives are deep by default. If a property value is an object, it'll be wrapped with readonly when read.
👉 Automatically unref refs when read from properties.
👉 If the model is meant to be immutable, readonly should wrap a raw object. It can also wrap a reactive object, in which case the readonly proxy will be reactive, by virtue of delegating tracks/triggers to the underlying reactive object. readonly by itself does not track.
⚠ This wrapper introduces a second proxy for the same object! If both readonly and writable proxies are in the state graph, it breaks referential identity (see below)! This is unavoidable if you want an object that enforces read-only access (like props) but you should be very careful when doing so.

shallowReadonly(x: T): T Shallow variant of readonly, properties won't be wrapped with readonly when read.

⚠ Shallow version doesn't unref, even at first level!

isReadonly(x: any): boolean checks if the object is a readonly proxy. Note that isReactive would also return true.

Watching

watchEffect(effect, options) Runs effect and run it again any time its dependencies change.
watch(getter, watcher, options) Evaluates getter and pass its value to watcher. Do it again anytime getter value changes.

👉 Those two watchers are automatically linked to the lifetime of the component that creates them.
📣 I'm quickly listing those two for completeness, as they are not part of the reactivity package but rather defined by vue. The matching low-level feature is effect, see below.

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 allocating ref or reactive.

👉 oldValue and oldTarget are only used to help in DEV mode, they are not present in production builds.
👉 Those functions are exposed but not part of the public documented API; they are subject to breaking changes.

🧪 effect(fn, options) Runs fn and runs it again when its dependencies change. It's the low-level primitive used to implement watch and watchEffect.

🧪 toRaw(object: T): T Returns the raw object wrapped by a proxy (any kind: readonly, reactive).

👉 This is actually exported by Vue, so core ⚙, but won't be documented as such. The requirement to export from Vue stems from usage in devtools.

🧪 pauseTracking() Can be used to read values without taking the dependencies.

📣 For consistency / symetry, would disableTracking be a better name?

🧪 enableTracking() Can be used to track again in a nested block inside pauseTracking (doesn't revert pauseTracking, see resetTracking).

🧪 resetTracking() Resumes tracking state as it was before the last pauseTracking or enableTracking call.

⚠ User is responsible for correct usage of those API: for each pause or enable call there should be a matching resetTracking call. When a reactive effect ends, it assumes the stack is in a valid state and doesn't clean it up automatically.

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.

👉 When creating custom refs, authors are in full control so behavior regarding deep or unref is up to the get implementation.

markRaw(object: T): T prevents an object from being wrapped by reactive.

⚠ This api is dangerous because it's easy to shallowly mark a graph non-reactive and then mess it up by putting deep members into reactive objects, breaking referential identity. It's recommended to consider using readonly instead.

👉 There's no "blessed" design to customize reactive objects at this point, other than wrap a reactive proxy.

Deconstructing

⚠ Direct deconstruction looses reactivity, of course: let { x, y } = useMouse() is just two numbers. Helpers in this section help you convert a reactive object into ref values, e.g. let { x, y } = toRefs(useMouse()) is two Ref<number> that access the underlying reactive object directly.

toRef(reactive: object, property: string): Ref returns a Ref that forwards to reactive[property].

👉 It just forwards to the original property, so all characteristics of the original reactive object apply here: tracking and triggering, deep or not, auto unref or not, etc.

toRefs(reactive: object): { [prop]: Ref } transforms a reactive object into a set of refs that forward to its properties.

📣 Having written some complex components, I feel like there is room for different utility functions, e.g. how to get an object with a subset of reactive properties, or how to remove some properties and keep the rest, etc. This can be filled by user-land libraries, so no need to include anything more in the beta. toRefs makes sense in my opinion and can be kept as-is in core, even if alternatives emerge later.

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.

@jods4
Copy link
Author

jods4 commented Apr 10, 2020

About Proxies

Proxies 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 reactive({ something }) or inert(await fetch('/references')) or readonly(x) on every piece of state.
Because everything is deep by default, you just need to do this on your roots and it will propagate to everything.

⚠ So using deep = false is a hazard. It can leave non-proxies in your graph, which may later be put into deep objects and create two references for the same object.

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 inert as an alternative to readonly and reactive.
Inert is an almost passthrough proxy, here's what it does:

  • unwraps refs;
  • by default is deeply inert: wraps returned values with inert proxies if they are not already reactive;
  • is recognized by isReactive and co (so won't be wrapped by reactive).

Why do we need a new proxy?

  • It adds explicitness and intent to user code: inert(references) clearly means they are not meant to be observed (so does readonly);
  • It doesn't trigger nor track, which is crucial if you do it for perf (e.g. large lists). readonly still tracks. Even if it's not your main use-case, free perf improvement is always welcome.
  • readonly enforces deep immutability. You may not want to track but still want to be able to change one field on the object. Maybe the object is immutable but it should contain mutable reactive references in its descendants. Maybe you intent on using a signal pattern (use a trigger to update large graphs at once without tracking everything individually).

@yyx990803
Copy link
Member

Thanks @jods for the detailed review!

♻ Replace shallowXXX with a second argument

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 true argument really means.

⚠ Unlike most reactivity primitives, computed does not automatically unwrap its result. Being a Ref itself it would be weird if it returned a Ref -- and if it does it's something that can easily be fixed by unref in the getter code.

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".

⚠ Unlike most reactivity primitives, computed is not deep: it doesn't wrap its results in reactive proxies. The idea might be that when its result (e.g. a filtered array) changes, it would return a new array; or that the getter will return a reactive value when it makes sense to. This can be a pitfall as it may introduce non-proxified instances in the state graph (more about this in my first comment).

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 reactive and expect to obtain a proxied version of these objects - so the identity problem should not occur.

Assuming there's no inert, it's also important for computed to not deeply make non-reactive objects reactive: imagine the user provided a shallow reactive array containing non-reactive objects, making these objects reactive actually leads to identity problems.

Works on array, but then doesn't unwrap! This was motivated by the fact that re-ordering an array of values (refs) is impossible if the array auto-unwraps them.

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.

👉 oldValue and oldTarget are only used to help in DEV mode, they are not present in production builds.

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 inert) when I find the time.

@jacekkarczmarczyk
Copy link

@yyx990803

This non-reactive array is created afresh on every computation - it's not meant to be mutated

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:

image

and Vue 3 doesn't

image

@jods4
Copy link
Author

jods4 commented Apr 11, 2020

@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.

Assuming there's no inert, it's also important for computed to not deeply make non-reactive objects reactive: imagine the user provided a shallow reactive array containing non-reactive objects, making these objects reactive actually leads to identity problems.

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) items and I want the user to pick one of them.
Additionally that list can be filtered, so I have a computed that basically performs return items.filter(...). Still non-reactive.
But then when the user clicks on one item I have this listener: @click="selected = item". And boom, because selected is a ref, item will become reactive. Then doing stuff like items.indexOf(selected.value) won't work.
If you are very aware of everything that happens, you can fix it by making selected shallow. But that's tricky stuff that will catch many devs off-guard.

After discussing this with @LinusBorg I believe there are 2 solutions:

  1. Nothing is deep by default. I actually like this style (brings back Knockout vibes) but it's not an experience Vue 2 devs will accept, as they expect the reactivity to "just work".
  2. Everything is wrapped, even non-reactive stuff. Which is why we should introduce inert.

@jacekkarczmarczyk

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?

It would be problematic if the value returned by computed was readonly.

Take the selected item example one step further:
It's actually a master-detail and the selected item is bound to an editable form.
If filtering the array returned an array of readonly Item, it means your selected item would be readonly too and you wouldn't be able to modify it in the form.

@jods4
Copy link
Author

jods4 commented Apr 11, 2020

I'm gonna keep the issue description updated with the feedback from comments.
@yyx990803

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).

Seems to me trigger only references oldValue here, under a __DEV__ guard:
https://github.com/vuejs/vue-next/blob/master/packages/reactivity/src/effect.ts#L233
It's declared in a DebuggerEventExtraInfos interface.

@backbone87
Copy link

♻ Replace shallowXXX with a second argument

i agree with @yyx990803 here, for further reading on this topic:
https://martinfowler.com/bliki/FlagArgument.html
#152 (comment)

  1. Nothing is deep by default. I actually like this style (brings back Knockout vibes) but it's not an experience Vue 2 devs will accept, as they expect the reactivity to "just work".
  2. Everything is wrapped, even non-reactive stuff. Which is why we should introduce inert.

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.

@jacekkarczmarczyk
Copy link

jacekkarczmarczyk commented Apr 11, 2020

@jods4

If filtering the array returned an array of readonly Item, it means your selected item would be readonly too and you wouldn't be able to modify it in the form

Readonly<Array<Foo>> !== Readonly<Array<Readonly<Foo>>>

You still are able to modify the item even if it's in readonly array

image

As you see here arr[0].foo = 2; is valid. What would be usecase for modifying the array itself (arr[0] = 'foo')?

@jods4
Copy link
Author

jods4 commented Apr 11, 2020

@jacekkarczmarczyk The depth isn't important for the argument.
Let's say the master list component actually binds to a selectedId and I use a computed to find the actual entity that I want to edit:
let selected = computed(() => items.find(selectedId.value)).
This computed returns an object and I definitely wants to edit it.

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?

@jods4
Copy link
Author

jods4 commented Apr 12, 2020

@backbone87

i agree with @yyx990803 here, for further reading on this topic:
https://martinfowler.com/bliki/FlagArgument.html
#152 (comment)

Note that if you want to follow Martin Fowler's advice, then it's shallowRef and deepRef.
Should we rename ref? I don't think so.

Anyway this isn't worth arguing about. If importing a second function named shallowReactive is more popular, I'll modify the issue description by removing the deep: boolean suggestion.

@jods4
Copy link
Author

jods4 commented Apr 13, 2020

Updated issue description
A recent commit removed effect from Core API -> I changed it to advanced.

@yyx990803
Copy link
Member

yyx990803 commented Apr 14, 2020

@jods4

📣 Why is this (effect) exported by vue? It feels like an advanced api for edge cases to me, I would only export it from @vue/reactivity.

This was added by vuejs/core#909 (which was a mistake on my part) and reverted by vuejs/core#961.

📣 This (toRaw) really feels like a niche API and goes against the "everything proxy forget raw" best practice (discussed in my first comment below). Shouldn't we remove it from vue exports and keep it in @vue/reactivity only?

This is needed in some internal use cases (e.g. devtools integration) where we cannot assume @vue/reactivity being available separately (e.g. devtools working with global Vue builds). I think this will stay undocumented in core unless there appears to be a need for it in end user code.

📣 Why is this (markReadonly) in core? Users should be pushed towards readonly(x) which deeply wraps their state in proxies by default. Marking plain objects is not deep and can lead to referential problems (see 1st comment).

Good catch - this is in fact a leftover from previous iterations of the readonly implementation. We should remove it.

markRef and markReactive

I was reading through vuejs/core#834 and wondering what is the easiest way to implement the example debounced ref.

First of all, I think it is too low-level to expose markRef + track + trigger in order to implement customized refs. In particular, track and trigger are useless without the op types, which are TypeScript enums with no runtime counterparts and can only be used with TypeScript. Exposing all of these is IMO exposing too much internals that risks future refactor flexibility.

Now, it is actually possible to implement a "custom ref" using a plain ref and a proxy:

import { ref } from 'vue'

function useDebouncedRef(value, delay = 200) {
  let timeout
  const r = ref(value)
  return new Proxy(r, {
    set(target, key, newValue) {
      if (key === 'value') {
        clearTimeout(timeout)
        timeout = setTimeout(() => {
          r.value = newValue
        }, delay)
        return true
      }
    }
  })
}

This, however, has two drawbacks:

  1. It creates a proxy on top of a ref. Ideally this nesting can be avoided for less overhead.

  2. Hard reliance on Proxy, so this pattern cannot be used in the IE11 compatibility build.

So to address the need for "refs with customized behavior", it is more straightforward to directly provide a new API, customRef. With customRef, a debounced ref can be implemented as:

import { customRef } from 'vue'

function useDebouncedRef(value, delay = 200) {
  let timeout
  return customRef((track, trigger) => ({
    get() {
      track()
      return value
    },
    set(newValue) {
      clearTimeout(timeout)
      value = newValue
      timeout = setTimeout(trigger, delay)
    }
  }))
}

As for markReactive - I don't think it is really necessary, because you can create "custom reactive objects" by passing an object with custom get/set (or a custom proxy object) to reactive:

const myCustomReactiveObject = reactive({
  get foo() { /* ... */ },
  set foo() { /* ... */ }
})

There are a number of use cases listed in vuejs/core#834

  • an implementation based on getter/setters rather than proxies (e.g. to work with classes using private fields);
  • an optimized reactive array that doesn't watch changes individually but as a whole;
  • "fake" reactive objects that do limited functionality, e.g. hideRefs({ a: ref(4) }) that only automatically unwrap refs.

These in my opinion unproportionally increases the maintenance surface and potential fragmentation for a very niche set of use cases.

Conclusion:

  • markRef and markReactive will not be added.
  • customRef should address the need of customized refs, without having to use a lower-level combination of markRef + track + trigger + op types.

Proxy identity & inert

I agree that the best practice would be "proxy at creation time" whenever possible. However, I don't really like the idea of introducing yet another proxy type. A base proxy type, unlike a utility function addition, is a much bigger addition to the API surface as it's a whole new concept to internalize.

Then it occurred to me that readonly doesn't really need to be tracking. The proper way to provide a readonly but reactive object that can be mutated only by the provider is to use it on a normal reactive object:

const source = reactive({ /* ... */ })

// readonly, but tracking and reactive (when source is mutated)
const reactiveReadonly = readonly(source)

// readonly, non-tracking and immutable.
const plainReadonly = readonly({ /* ... */ })

You mentioned some additional reasons for why inert is needed in addition to readonly:

You may not want to track but still want to be able to change one field on the object.

This in my opinion should be discouraged - but it can already be done with toRaw.

Maybe the object is immutable but it should contain mutable reactive references in its descendants.

A readonly object should never have descendants that are mutable. It can, however, contain descendant readonly that wraps mutable sources.

Maybe you intent on using a signal pattern (use a trigger to update large graphs at once without tracking everything individually).

This can be done with customRef which gives you flexibility to decide when to trigger.

Conclusion:

  • inert will not be added.
  • readonly will be changed to non-tracking and can be used in place of inert as proposed.

Necessity of markNonReactive

I implemented inert as proposed and did some benchmarking, and it doesn't really provide enough performance benefits. The primary performance overhead in Vue 3's reactivity system is largely the proxy creation and get traps themselves - the tracking operations are actually the much cheaper part. From benchmarks, proxy cost vs. tracking cost is at a roughly 5 to 1 ratio.

So, in real perf-sensitive scenarios, markNonReactive should still be preferred because it would provide several times the perf gain of inert.

Identity Hazard Patterns

I still believe that cases where identity really becomes an issue is rare - they are caveats. I believe it's better to clearly identify the patterns that would lead to identity hazard and explicitly document them.

So far we've discovered two category of identity hazards:

  1. Reactive copy received from template method call vs. original declared in setup:

    setup() {
      const items = [{}, {}, {}]
    
      return {
        items,
        hasItem(item) {
          // item is a proxy of the original
          return items.indexOf(item) > -1
        }
      }
    }
  2. Original exposed to template (due to computed) vs. reactive copy retrieved on render context (shown in Necessity of reactive call on Component API result #121 (comment), modified for clarity):

    setup() {
      const items = [{}, {}, {}]
      const search = ref('')
      const selected = ref(null)
    
      return {
        search,
        selected,
        // used in template, but exposed as original because computed doesn't
        // make return values proxies
        filteredItems: computed(() => {
          return items.filter(x => x.name.includes(search.value))
        }),
        isSelected: item => {
          // `item` is original from filteredItems
          // but `selected.value` is the reactive copy
          return item === selected.value
        }
      }
    }

readonly can be used to avoid both cases. However, that is a style guidance and can only be enforced via linters (similar to props destructuring).

I do have an idea for a compiler-based approach that should also get rid of the issue in SFCs, but I think that should be discussed at a later time, after we get to maybe RC stage.

@jods4
Copy link
Author

jods4 commented Apr 14, 2020

Updated issue description

  • A recent commit removed markReadonly from Vue completely -> removed from issue description.
  • Updated toRaw comments and "level" according to previous comment from Evan.

@jods4
Copy link
Author

jods4 commented Apr 14, 2020

markRef and customRef

That's interesting design. I see the point about going a little too far when exposing the actual value of the track/trigger types, although they're integral to the full API.
Especially for a ref, where track can be assumed to always be get and the field name bears no relevance, same for trigger being set.

I am not 100% convinced because customRef, as I understand it, removes some power by assuming: (1) every read is tracking, (2) trigger is always called from the setter.

Here are some examples for fun (for simplicity written with trigger/track without argument, no markRef either).

Promise
This is a very lightweight read-only ref. It will return undefined until resolved, won't track anymore after that.
Bonus points for exposing a second property error that returns undefined until the promise rejects, with no additional dependencies.

function promiseRef(promise) {
  let state = 0 // 0 = unsettled, 1 = value, 2 = error
  let outcome = undefined

  promise.then(
    x => { state = 1; outcome = x; trigger() },
    x => { state = 2; outcome = x; trigger() })

  return { 
    get value() { 
      if (state === 0) track()
      else if (state === 1) return outcome
    }
    get error() {
      if (state === 0) track()
      else if (state === 2) return outcome
    }
  };
}

Observable (RX)
Same deal, read-only ref that will expose the latest value and stop tracking when done.
Supports manually unsubscribing or automatically when unmounted.

function observableRef(obs) {
  let done = false
  let value = undefined

  let sub = obs.subscribe({ 
    next(x) { value = x; trigger() },
    complete() { done = true },
  })

  onUnmounted(() => { sub.unsubscribe(); done = true })

  return {
    get value() {
      if (!done) track()
      return value
    },
    stop() {
      done = true
      sub.unsubscribe()
    }
  }
}

I am noticing a pattern here, so let me suggest a new customRef, that works a bit like the built-in Promise:

function customRef(factory) {
  let newRef
  const track = () => track(newRef, 'set', 'ref')
  const trigger = () => trigger(newRef, 'get', 'ref')  
  newRef = factory(track, trigger)
  newRef._isRef = true
  return newRef
}

📣 This allocates 2 functions with closure for each ref, is it negating some benefits of custom refs in the first place? Would it be ok to pass to factory static functions that take one argument, i.e. track(this) and trigger(this)?

With this new tool in hand, let's redo the debounce (promise and observable are trivial to convert as well):

function debouncedRef(value, delay = 200) {
  return customRef((track, trigger) => {
    let timeout
    return {
      get value() { track(); return value },
      set value(x) {
        // I'm keeping assignment outside of timeout, 
        // because I want to read the real values always, if I read.
        // I'm only debouncing the notifications, e.g. if a watcher is doing  network requests.
        // This is important if you're doing "if (search.value == sent)" to avoid out-of-order replies.
        value = x
        clearTimeout(timeout)
        timeout = setTimeout(trigger, delay)
      }
    }
  })
}

customRef as proposed doesn't apply deep observation to values. As this is an advanced api I think it can be left to consumer to decide what's best in their use case (add reactive or readonly where/if appropriate). This leads to maximum flexibility and efficiency.

I'm gonna pick up the discussion on reactive and Proxies later.

@jods4
Copy link
Author

jods4 commented Apr 14, 2020

Noticed an issue with the Promise example.
Because of auto unwrapping, you won't be able to use the error property.
So it should be an example of custom ref only if you don't care about rejecting.

@yyx990803
Copy link
Member

yyx990803 commented Apr 15, 2020

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 .value property and should not have any other properties on it (except for the internal _isRef). Its sole purpose should be a value container and that's it. If you need to expose related functions, return an object containing the ref and the functions.

Therefore I think the factory should still return the object in the format of an options object containing { get, set } instead of the actual ref object itself.

@jods4
Copy link
Author

jods4 commented Apr 15, 2020

I agree adding other properties is an error. A ref is by definition a single value and with unwrapping (in views or unref) other properties are unusable anyway. 2+ properties should be a reactive object.

If you need to expose related functions, return an object containing the ref and the functions.

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 { get(), set?() } or returning { get value(), set value?() } is not making a whole lot of difference.

If you look at previous art (e.g. Knockout, Mobx), there are examples of this:

  • My observable wrapper above has a useful stop, basically Mobx fromStream;
  • The same disposable concept is useful for other sources (e.g. web sockets), basically Mobx fromResource.
  • The Debounce/Throttle example could use a setImmediate(x) that bypasses debouncing/throttling. It's useful e.g. if you want to clear your search box from code without lag/delay in UI (search.setImmediate('')).
  • Mobx utils also have a lazyObservable (= lazyRef) that takes a sink function which changes the ref value when you trigger it with update().
  • In Knockout users had created validatable refs (with validation rules attached and a .validate() method, could be composed into complete objects).
  • In KO as well: transactional refs (with commit, rollback).

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?

@yyx990803
Copy link
Member

yyx990803 commented Apr 15, 2020

@jods4

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)

Well, when you put methods on a ref:

  • You can't pass it to the view in a single property, because refs are unwrapped in the template. You'll have to do something awkward like return { ref, method: ref.method }. <-- and by the way, this will break in your x.double example because you are relying on this so it must be called with x being the receiver.

  • Putting all your methods on the ref means whenever you pass the ref to another function you are always giving it access to all those methods. Sometimes you may want to only pass the value itself but not the methods controlling its state. With an object you can choose between passing only the ref, only the method, only some methods, or everything.

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 this, standalone functions), and the API design will try to nudge you that way (which is why I prefer returning { get, set })

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.

@yyx990803
Copy link
Member

I edited the customRef implementation in this comment to use a factory. This has also been implemented in master branch of vue-next.

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.

@basvanmeurs
Copy link

@jods4

mega Why is this (effect) exported by vue? It feels like an advanced api for edge cases to me, I would only export it from @vue/reactivity.

This was added by vuejs/vue-next#909 (which was a mistake on my part) and reverted by vuejs/vue-next#961.

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 effect simply comes closest to what is required here, but we could be wrong. Do you have any advice for this case @jods4 and @yyx990803 ?

@yyx990803
Copy link
Member

@basvanmeurs to avoid going off-topic let's continue this here

@jods4
Copy link
Author

jods4 commented Apr 15, 2020

@yyx990803

I edited the customRef implementation in this comment to use a factory. This has also been implemented in master branch of vue-next.

Great, thanks! I think this modified customRef enables the construction of any ref you'd want, in any style.

Identity Hazard Patterns

The new readonly is almost identical to inert: it's a deep unwrapping passthrough that doesn't track.

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 toRaw.
Most common concern is keeping non-reactive immutable state -- such as lists of reference data or static tables/reports/charts -- and the new readonly covers this well.

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 reactive on some stuff that was immutable.

I think it would be worthwhile that Vue 3 examples and tutorials put forth that all state created in setup should be ref, reactive or readonly. Following this pattern can avoid headaches, esp. for devs who don't understand fully what's going on internally.

⚠ The new readonly has an important change from previous one: it's only reactive if you wrap a reactive wrapper. If you build a reactive and readonly from the same raw object, the readonly facade won't be reactive (it won't track reads).
You can also now end up with 3 proxies for the same object: reactive, readonly over raw, readonly over reactive.
Given that having 2 proxies is a tricky situation in itself, I think this just needs to be documented and people should be careful using the API properly. I guess guideline is: use readonly either on an immutable raw object that will never be reactive; or on its reactive proxy (which you should have created immediately and thrown away the raw object).

👉 In all cases, toRaw returns the raw object as you'd expect.

I'll follow-up on "custom reactive" and update the issue description with recent changes later.

@yyx990803
Copy link
Member

yyx990803 commented Apr 15, 2020

⚠ The new readonly has an important change from previous one: it's only reactive if you wrap a reactive wrapper. If you build a reactive and readonly from the same raw object, the readonly facade won't be reactive (it won't track reads).

Correct. I was working on an update to the Composition API Reference just now and realized this would also require the following changes:

  • isReactive should only return true for proxies created by reactive, or a readonly that wraps a reactive.
  • A new utility isProxy is introduced, which returns true for either reactive or readonly.
  • Since markNonReactive skips conversion for all kinds of proxies, it should be renamed to markRaw.

@jods4
Copy link
Author

jods4 commented Apr 15, 2020

@yyx990803 It's been a few alpha since watch was split in two and watchEffect now executes sync.
As you noted this means DOM is not available, which can be fixed by either setting up the effect inside onMounted or having a guard if (!dom) return at the beginning of you effect.

Given that this is a fairly common thing and given that watchEffect already takes an option object, would it be convenient to be able to request execution only after being mounted (i.e. opt in previous behavior)?

setup() {
  const dom = ref()
  watchEffect(() => dom.value.focus(), { mounted: true })
  return { dom }
}

@backbone87
Copy link

@jods4 i think your example is bad practice, because it induces wrong typing: ref<HTMLElement>() while in reality this is ref<HTMLElement | undefined>() everywhere outside of the watcher OR when the element is conditionally skipped in render.

for this specific use case, you can use the safe navigation operator:

setup() {
  const dom = ref<HTMLElement | undefined>()
  watchEffect(() => dom.value?.focus())
  return { dom }
}

@backbone87
Copy link

@yyx990803
me being unaware of the implementation detail:

  • isReactive should only return true for proxies created by reactive, or a readonly that wraps a reactive.
  • A new utility isProxy is introduced, which returns true for either reactive or readonly.

these return true for ref as well?

@yyx990803
Copy link
Member

@jods4 if it can be done with onMounted(() => watchEffect(() => {})) I don't see the point of adding an option for it.

@backbone87 no, only isRef returns true for refs.

@jods4
Copy link
Author

jods4 commented Apr 16, 2020

convenience? it's a common thing when using DOM. It was handy in the first alphas.

@backbone87
Copy link

backbone87 commented Apr 16, 2020

no, only isRef returns true for refs.

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?

@yyx990803
Copy link
Member

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

@jods4
Copy link
Author

jods4 commented Apr 19, 2020

Updated issue description

  • Removed some of the issue "introduction" now that beta is out.
  • Removed changes/additions keys since there are none left.
  • Added shallow* as the parameter proposal got negative feedback only.
  • Added customRef
  • Added toRef
  • Added a note that there is no support for customizing reactive objects (other than wrapping the core implementation).
  • Renamed markNonReactive to markRaw
  • Changed behavior of isReactive, added isProxy

👉 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.

@jods4
Copy link
Author

jods4 commented Apr 19, 2020

@yyx990803 it's a nit but when updating the issue I noticed customRef is exposed by core.
As it's an advanced api meant for library authors and power-users, I wouldn't feel bad hiding it inside @vue/reactivity, like toRaw.

Also nit: still finding pauseTracking vs enableTracking unintuitive naming.

@thecrypticace
Copy link

I agree regarding the naming of enable vs. pause.

As it stands currently enableTracking and pauseTracking are opposites. One turns on tracking and the other turns off tracking during an effect, fn call, block of code, etc…

The name enableTracking implies the existence of a corresponding disableTracking but it's actually called pauseTracking. Likewise, the name pauseTracking implies the existence of a corresponding resumeTracking but it's actually called enableTracking. I understand these are very niche and very advanced APIs but the naming is definitely confusing. I'd say that either enable/disable or resume/pause are reasonably appropriate and well-paired names.

In looking through all the uses of enable / pause they are always paired with resetTracking. This is because tracking state is a LIFO stack. I'm not sure what the best name for this operation is but I would generally expect, from intuition, that an enable/disable or resume/pause call would be paired with a balancing disable/enable or pause/resume — not reset.

And while it doesn't resolve the intution mismatch of pairings, I think that restoreTracking or restorePreviousTracking would be clearer names because either one of these communicates the intent far better. To me "reset" implies going back to an initial state and not to the previous one.

Thoughts?

@jods4
Copy link
Author

jods4 commented Apr 23, 2020

Updated issue description
Added triggerRef

@jods4
Copy link
Author

jods4 commented May 5, 2020

Follow-up on custom reactives

(sorry it's been a while, been busy lately)
@yyx990803

  • I agree with you that this is a bit of a niche, although a useful one.
  • I think that it won't be possible to come up with a small, well-designed api that doesn't leak too much in the timeframe v1.
  • Thanks to some of your recent changes (namely the removal of a Vue internal dependency on toRaw behavior), a custom reactive can be faked well enough with markRaw + trigger + track. It's not a supported scenario but if you really want to and are ready to update your code with future releases, you can do it.

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.
I won't go into many examples because as I said, closed topic for v1, but here's just one:

If you make a deep proxy that adds some feature to your state (e.g. maybe you want to create transactional state; or validation; ...), you're gonna conflict with the deep reactive proxy in one way or another:

  • if your proxy is on top, then reactive will wrap it again, not seeing the underlying reactive proxy that's already there;
  • if the reactive proxy is on top, it might "miss" access to the underlying object by your own proxy, or it might create multiple identities if the underlying raw object is otherwise "reactified";

For this specific example, being able to compose all "features" (reactivity, transactional state, etc.) into a single proxy layer would be most efficient and avoid issues.

Computed public API

#163 made me realize that it's not totally clear if some part of computed api is public / supported or not, namely the effect property, which in turn exposes all of ReactiveEffect.

Computeds expose effect on purpose: to allow stopping the computed, which is otherwise impossible (e.g. when used outside a component), which would produce memory leaks if some dependencies outlive the computed (e.g. they are statics).

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 stop(computed) -- as you have stop(effect).

Note that if effect is internal and you add stop(computed), then you hide all of effect api as well. The discussion in #163 is interested in access to effect.onStop.

@basvanmeurs
Copy link

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 const originalValue = ref.value; ref.value = null; ref.value = originalValue; but just providing a trigger would be useful in many use cases - even if it would only be used during development or for testing, developers find it handy.

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 pauseTracking and resetTracking, but we can offer an untracked function which allows executing a function without tracking. See vuejs/core#1130

@RobbinBaauw
Copy link

RobbinBaauw commented May 9, 2020

First of all, I'm missing a way to trigger a ref manually, even though values do not change.

I'm pretty sure there already is, and is even exported by vue. From the OP:

⚙️ 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).

@basvanmeurs
Copy link

basvanmeurs commented May 14, 2020

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 reactive is most convenient in my case. This is mostly because I don't need to change the reference to the object itself but only the properties. Concluding: when the reference to the object is not expected to change, reactive should be favored over ref.

But there I experienced one super annoying thing about using reactive. When using a watch function, you need to specify all properties that you want to watch on. Although this allows fine-grained dependency specification, in practice the syntax leads to very ugly code:

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 watch with a reactive object.

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.

@LinusBorg
Copy link
Member

LinusBorg commented May 14, 2020

But there I experienced one super annoying thing about using reactive. When using a watch function, you need to specify all properties that you want to watch on.

... 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.

My proposal is to add the toRefsArray

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 Object.values(toRefs()) is enough.

@jods4
Copy link
Author

jods4 commented May 14, 2020

@basvanmeurs Most of the time the easiest thing to do is use watchEffect.
I see many people relying on watch with a "what ref am I watching?" mindset, when watchEffect is much more powerful.

Just write your code in watchEffect and it will run again when its dependencies change, without you having to declare what the dependencies are.

watch is still useful but more in optimized and special scenarios.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

8 participants