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

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

Open
rklancer opened this issue Apr 29, 2015 · 1 comment
Open

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

rklancer opened this issue Apr 29, 2015 · 1 comment

Comments

@rklancer
Copy link
Contributor

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
Copy link
Contributor Author

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

1 participant