Mobx.dart: Consider adding an option for an equality check to @observable

[shyndman]

Hello!

At the moment, @observables generate setters that always set the field value and report change, whether or not the new value differs from the field's current value.

It seems like it could be useful to add an option to @observable to include an equality check to minimize the reporting of change.

If provided as an option, instead of a default, the change would be non-breaking.

Maybe something like:

@observable(checkEqualityOnSet: true)
String myObservableProperty;

checkEqualityOnSet is a little wordy though. Suggestions welcome.

[shyndman]

Started playing with this, and it would need to be slightly different. We'd need to expose an additional annotation — one that takes args, leaving the existing one that takes no args (because it's an instance)

[pavanpodila]

We have this logic on the Observable class, which is purely based on references/values and not structures. I think we can reuse that part here.

Reference: https://github.com/mobxjs/mobx.dart/blob/master/mobx/lib/src/core/observable.dart#L27

[shyndman]

Ah, interesting. How would you recommend reusing that piece?

[pavanpodila]

We can move the comparator logic to a global function that takes in a previous and new value and does equality or structural comparisons. Something like:

bool compareEqual<T>(T newValue, T oldValue) { }

Perhaps we can even pass a comparator function with a string name ?

[shyndman]

I think that we might even be able to pass a function reference to the annotation, as long as it's top-level or static. I'll poke around.

[pavanpodila]

I think annotations are const only constructors but do confirm

[shyndman]

They are, but I believe that top-levels and statics are const values. I'll double check.

Yeah:
https://dartpad.dartlang.org/4541ffc0e8ec013dd21a8b504d2eef05

[pavanpodila]

So static-fns possible right? It wasn't clear if the runtime allows it from your example. Syntactically seems legit :-)

[shyndman]

yup. static and top-level functions. I updated the gist/dartpad.

[pavanpodila]

Awesome!! Road is clear then.

[pavanpodila]

BTW, I was thinking of starting a RFC type README that outlines the approach for all new features we add to mobx from here on. The README will be sent in as a draft-PR and allows commenting and discussions. It will act as a spec for the implementation with a section even on the implementation strategy. This is very similar to the way dart-lang is doing it.

Worth a shot? Can we do for this feature? I also want to do this for the spying/tracing functionality.

[Edit]
We can put this under /rfc at the top level. The rfc PR, when merged, is a green signal for implementation, which will be a separate PR. This approach will also allow some GSOC style contributions

[shyndman]

Yeah, definitely. Let's do it.

Also, so looking forward to some spying/tracing. I just got through a bug where I had to figure out what was triggering a reaction that I didn't want triggering, and it wasn't great. :)

[pavanpodila]

@shyndman, Started the draft PR for spy: https://github.com/mobxjs/mobx.dart/pull/451/files#diff-9808bc3c52d0ffc05141ad43decd7cdd, we can follow a similar model for the equality comparator

[hmayer00]

This sounds great! Would it make sense to add a comparator parameter to @computed as well? There would be two steps there, since unlike for Observable, Computed doesn't already have an equals parameter. It does however have an _isEquals method that looks like it's just waiting for a comparator option, so that part should be straightforward (and I'd be happy to implement and PR). The codegen piece is very new to me, but I could take a swing at that too, modeling it off of the work for @observable whenever that drops.

Lemme know if you're interested, or if I'm on the wrong trail here, and you think Computed is a bad fit for a parameterized comparator, for whatever reason.

[pavanpodila]

@hmayer00 yes a computed will absolutely benefit from a comparator. I think it was a miss from the earlier PR. Happy to take a PR from you. I can review where needed.

[hmayer00]

👍 Wrapping something up first but should be able to get to that this week.