MVCoconut / coconut.data

Observable Data Layer.
The Unlicense
20 stars 8 forks source link

Get rid of macro cast in Value. #77

Open back2dos opened 3 years ago

back2dos commented 3 years ago

Currently, there's a macro based transform used for Value that was originally primarily intended for @:external, so that one may write the following (example from readme):

var compass:Compass = ...;
var log:ChipLog = ...;
var movement = new Movement({
  heading: compass.degrees / 180 * Math.PI,
  speed: log.knots * KNOTS_IN_METERS_PER_SECOND,
});

This is cute and all, but since it relies on a @:from macro, it has a number of disadvantages:

  1. top down inference will not work
  2. when having conditionals, they are not observed, i.e. having heading: if (settings.useCompass) compass.degrees / 180 * Math.PI else someOtherMethod is expected to become something like heading: Observable.auto(() -> if (settings.useCompass) compass.degrees / 180 * Math.PI else someOtherMethod) but in reality it becomes heading: if (settings.useCompass) Observable.auto(() -> compass.degrees / 180 * Math.PI) else Observable.auto(() -> someOtherMethod), because top down inference enters into each branch before applying the @:from macro

This was all designed with Haxe 3 in mind and today, I'd consider this a suitable alternative:

var compass:Compass = ...;
var log:ChipLog = ...;
var movement = new Movement({
  heading: () -> compass.degrees / 180 * Math.PI,
  speed: () -> log.knots * KNOTS_IN_METERS_PER_SECOND,
});

There are two ways of making this work:

  1. Remove (at first deprecate) the @:from macro and add a @:from ()->T cast to Value (or perhaps even to Observable for that matter). This will solve problem 2, but not the lack of top down inference
  2. Make the constructor consume a ()->T and wrap that in an auto observable internally. This solves both issues. The breaking change would easily be avoided with an implicit cast (and a deprecated @:from macro that makes code like the above compile), but there's a relative performance overhead from wrapping an observable into an auto observable.

I'm inclined towards option 2, because the overhead won't matter most of the time. Its significance should be rare enough to let the user handle it somehow (worst case by making a class that is not a model, but passes the checker).

In any case I wish to deprecate the direct expression -> auto observable syntax, because it may produce code that doesn't work as intended. If anyone is strongly opposed to that, now is the time to be vocal about it.