sylvainpolletvillard / ObjectModel

Strong Dynamically Typed Object Modeling for JavaScript
http://objectmodel.js.org
MIT License
467 stars 28 forks source link

Serializing Computed Properties #68

Closed sfeldkamp closed 6 years ago

sfeldkamp commented 6 years ago

The docs show the following example for using objectmodel with es6 classes

Model

class Character extends Model({ lastName: String, firstName: String }){
   get fullName(){ return `${this.firstName} ${this.lastName}`; }
}

Instance

const rick = new Character({ lastName: "Sanchez", firstName: "Rick" });
rick.lastName = 132;
TypeError: expecting lastName to be String, got Number 132
console.log(rick.fullName); // "Rick Sanchez"

However in this case JSON.stringify(rick) returns only {"lastName":"Sanchez","firstName":"Rick"}.

This is true even when a fullName: String property is added to the model definition. See this JSFiddle: https://jsfiddle.net/udvrdo86/5/

Our Use Case

We want to use ObjectModel on the server to define a API results and to reduce code duplication among API that return results in the same shape. We also need to support derived (computed) properties.

Workarounds

1) Instantiate the ObjectModel on the client from the API result. We aren't yet certain we want behavior rich models on the client, we may prefer plain data objects.

2) Set the values in the constructor. This works, but it's not as clean as it could be.

3) Pass the computed values into the model during instantiation. This works, but code that computes the value is spread around and it places burden on the caller to remember to do this.

Proposals

1) Model definition could include special syntax for defining a computed value.

2) Models could call an es6 getter when serializing.

sylvainpolletvillard commented 6 years ago

Hi @sfeldkamp , thanks for the detailed issue

Actually this problem is not bound to ObjectModel and there is a way to solve it without changing the library.

You just need to override the standard toJSON method to define how your classes are serialized in JSON. See this issue for more info : https://stackoverflow.com/questions/42107492/json-stringify-es6-class-property-with-getter-setter

class Character extends Model({ lastName: String, firstName: String }){
   get fullName(){ return `${this.firstName} ${this.lastName}`; }

   toJSON(){
      return { lastName: this.lastName, firstName: this.firstName, fullName: this.fullName }
   }
}

If you don't want to always add this method on all your classes, some guys have provided more automatic ways to do this in StackOverflow answers

sfeldkamp commented 6 years ago

Great, we're happy to do that. Is there any danger to overriding the toJSON method like that?

sylvainpolletvillard commented 6 years ago

Not sure what you mean by danger. Overriding the method is totally fine, you are not touching some native object prototypes so this is not considered a bad practice. Also, it gives you more control on how the serialization is done, so there is a lesser risk to expose private data.

sfeldkamp commented 6 years ago

Sorry, I was trying to ask in a roundabout way whether the lack of support for serializing these properties was an oversight in the library and if so, would you be willing to consider a pull request.

sylvainpolletvillard commented 6 years ago

No, it's not an oversight. ObjectModel intends to be as transparent as possible. It only provides runtime validation and does not change the original behaviour of the objects (except minor usecases such as null-safe object traversal). The standard behaviour is to not serialize computed properties, and the library does not intend to change this.

This makes sense though, since getters are just a special kind of functions and are supposed to be logic, not data. But you may have a legit reason to override this, hence the toJSON method.

sfeldkamp commented 6 years ago

Thank you for the explanation and for the library. We are grateful.