concord-consortium / lab

HTML5-based scientific models, visualizations, graphing, and probeware
http://lab-framework.concord.org/
MIT License
57 stars 38 forks source link

Support for "compound" model properties (arrays, objects) #87

Open rklancer opened 9 years ago

rklancer commented 9 years ago

Right now, tick history and dirty checking only work as expected if model properties are primitive values (strings, numbers, null, undefined, etc) but not if model properties are reference values -- arrays or objects. However, it would be handy for some model properties to be arrays or objects.

Technically, it is possible to use a non-primitive value, but authors will be surprised when they perform mutations on the reference type and dependent properties don't get updated, or when the previous value is not restored when navigating the tick history.

As proposed in the discussion in #82 (which added joystick controls, which naturally have a single value that has a magnitude and direction) we can add support for compound values relatively easily.

To quote from the pull request thread:

  • Where we return the value to save into tick history we could make a clone.
  • When we set a property, check to see if it's a primitive value, and if it's not, endow the value with a simple toString like method that returns a meaningful string or hash that can be used to check for equality (e.g. it could return a string like obj: a:1,b:2 for {a: 1, b: 2})
  • Where we check for changed property values we use the above-provided method to check for equality if the type is not a primitive type.

As @pjanik suggests, instead of adding a method to the values to allow dirty checking, the saved previousValue could be always a copy (just like we would save a copy to the tick history), and we could do an equality check between the current and previous values.

We might want to consider explicitly supporting only non-nested objects and arrays, and throwing an error if we get nested values. This prevents having to have an equality check also check for cycles (lest the check loop forever) but more importantly we might be wise to discourage excessive nesting in property values (as that's probably a sign you should introduce more properties into the model)

I expect that this simple scheme will work, but there is one place that authors will need to be careful when mutating compound-valued properties. Specifically, that is when the mutation happens outside a model tick or an invalidation.

model.properties[propertyName].x = 10

will not cause anything to change if it happens e.g., in Lab code, because the change won't be observed. Developers will have to be careful to do something like

model.makeInvalidatingChange(function() { 
    model.properties[propertyName].x = 10;
});

in that case. However, this is already the case for many primitive-valued properties!

rklancer commented 9 years ago

Pivotal Tracker story: https://www.pivotaltracker.com/n/projects/442903/stories/93559672