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

Value-like equality in computed #802

Closed
jamiewinder opened this issue Jan 26, 2017 · 9 comments
Closed

Value-like equality in computed #802

jamiewinder opened this issue Jan 26, 2017 · 9 comments

Comments

@jamiewinder
Copy link
Member

jamiewinder commented Jan 26, 2017

I have the following code:

import { LocalDate, Instant } from 'js-joda';
import { computed } from 'mobx';
import { now } from 'mobx-utils';

const currentDate = computed(() => {
    return LocalDate.ofInstant(Instant.ofEpochMilli(now(10000)));
});

This basically produces an observable LocalDate value based on mobx-utils' now utility which will be accurate enough for my use (never more than 10 seconds out). However, the computed itself will observably change every 10 seconds, despite the practical 'value' of the returned object very rarely changing.

This for example will log every 10 seconds, not ~ once a day:

autorun(() => {
  document.write(currentDate.get().toString());
});

Is there a better way I could be doing this? It'd be nice to be able to instruct MobX how to determine equality of the return values (i.e. (a, b) => a.equalTo(b)) so it knows when the computed value should observably change.

EDIT: This appears to achieve what I'm after, but is a little hacky:

let lastDate = null;
const currentDate = computed(() => {
  const nextDate = LocalDate.ofInstant(Instant.ofEpochMilli(now(10000)));
  if (!nextDate.equals(lastDate)) {
    lastDate = nextDate;
  }
  return lastDate;
});
@mweststrate
Copy link
Member

@jamiewinder did you try computed.struct?

@urugator
Copy link
Collaborator

urugator commented Jan 26, 2017

Ability to provide own comparator has been already proposed, but it's probably not needed that often...
Another option would be the ability to implement [Mobx.Symbol.equals] on arbitrary objects.

Some other workarounds:

  1. In the end autorun uses comperable string, so you could introduce another computed
const currentDate = computed(() => {
  const nextDate = LocalDate.ofInstant(Instant.ofEpochMilli(now(10000)));
});
const currentDateAsString = computed(() => {
  return currentDate.get().toString(); // or timestamp or anything comparable by value/reference/structure
});
autorun(() => {
  document.write(currentDateAsString.get());
  // it's a bit hacky but you can still access currentDate without subscribing via Mobx.untracked 
});
  1. Use other observable and update it when needed via reaction
const boxedLocalDate = Mobx.observable.box(LocalDate.ofInstant(Instant.ofEpochMilli(Date.now()));
reaction(() => MobxUtils.now(10000), epochMilli => {  
  const newDate = LocalDate.ofInstant(Instant.ofEpochMilli(epochMilli));  
  if (!boxedLocalDate.get().equals(newDate)) {    
    boxedLocalDate.set(newDate); 
  }    
});   
autorun(() => {
  document.write(boxedLocalDate.get().toString());
});
// Don't forget to dispose reaction after you dispose autorun

EDIT: @mweststrate even if struct would work, it wouldn't work the same as LocalDate.equals

@jamiewinder
Copy link
Member Author

jamiewinder commented Jan 26, 2017

@mweststrate - I don't think computed.struct can be used in the same way as computed? Isn't it just a decorator, or is that what you're suggesting (i.e. define it as a class property rather than a variable)?

The name suggests it'd fit the bill in this case as I think all the comparable properties of a LocalDate in js-joda are numeric properties.

@urugator - Thanks for the suggestion. I've currently gone for this general solution:

function computedWithComparator<T>(
    func: () => T,
    eq: (a: T, b: T) => boolean,
    setter?: (value: T) => void
) {
    let firstTime = true;
    let lastEmitted: T;
    return computed(() => {
        const value = func();
        if (firstTime || !eq(value, lastEmitted )) {
            lastEmitted = value;
            firstTime = false;
        }
        return lastEmitted;
    }, setter);
}

It's a bit of a clumsy name, but it seems to be doing the trick. Usage:

const currentDate = computedWithComparator(
    () => LocalDate.ofInstant(Instant.ofEpochMilli(now(10000))),
    (a, b) => a.equals(b)
);

@urugator
Copy link
Collaborator

urugator commented Jan 26, 2017

Isn't it just a decorator?

You can pass options to computed function (would be nice to have anchors on headings)

@mweststrate
Copy link
Member

@jamiewinder, yes, should work as direct api as well, or pass { compareStructural: true } as second arg, works the same :). Edit: what @urugator said. Your last suggestion btw could be nice for mobx-utils!

@jamiewinder
Copy link
Member Author

jamiewinder commented Jan 26, 2017

@mweststrate @urugator

Thanks both, I didn't know about the computed options argument! Just when I thought I knew it all..!

I can submit this to mobx-utils too if you think it's useful, but might it be better as another option on the standard computed (I.e. the equality predicate)? Or do you think it's too specialised?

Thanks again

@jamiewinder
Copy link
Member Author

@mweststrate Some more thoughts on this for critique!:

// Extra 'equals' option on computed options (could be a better term). Takes the last emitted value and the new computed value, and returns `true` if they're considered equal, otherwise `false`. This won't be called the first time a computed is calculated.
computed(() => {
    return ...;
}, { equals: (a, b) => a.isEqualTo(b) });

// This mimics the current behaviour
computed(() => {
    return ...;
}, { equals: (a, b) => a === b);

// This is the same as { compareStructual: true }
computed(() => {
    return ...;
}, { equals: (a, b) => deepEquals(a, b));

It might get messy on a decorator, but maybe:

const localDateEqual = (a: LocalDate, b: LocalDate) => a.equals(b);
// ...
@computed({ equals: localDateEqual }) public get today() {
    return ...;
}

Or maybe, gulp:

const computedLocalDate = createComputedDecorator<LocalDate>({ equals: (a, b) => a.equals(b) });

@computedLocalDate public get today() {
    return ...;
}
@computedLocalDate public get yesterday() {
    return ...;
}

@mweststrate
Copy link
Member

Sorry for the late response @jamiewinder! An additionals equals option to computed sounds like a good suggestions. PRs welcome ;-)

@mweststrate
Copy link
Member

Version 3.2.1 added support for custom equality checks. Thanks @jamiewinder!

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

3 participants