talyssonoc / structure

A simple schema/attributes library built on top of modern JavaScript
https://structure.js.org/
MIT License
301 stars 20 forks source link

Allow custom toJSON method on class for serialization. #88

Closed loweoj closed 5 years ago

loweoj commented 5 years ago

This feature would allow you to specify a static toJSON method on the instance that would be called with the serialized JSON. It would allow you to modify the JSON before being output. Adding removing and modifying attributes to the final output.

e.g.

var User = attributes({
  name: String,
  age: Number
})(class User {
  static toJSON(json) {
    json.someProp = true;
    return json;
  }
});

const user = new User({
  name: 'John',
  age: 42
});

expect(user.toJSON()).to.eql({
  name: 'John',
  age: 42,
  someProp: true
});

It should also work on nested structures:

var Book = attributes({
  title: String,
})(class Book {
  static toJSON(json) {
    json.added = '123456789';
    return json;
  }
});

var User = attributes({
  name: String,
  age: Number,
  favouriteBooks: {
    type: Array,
    itemType: Book
  }
})(class User {
  static toJSON(json) {
    json.name = 'Hello ' + json.name;
    return json;
  }
});

const user = new User({
  name: 'John',
  age: 42,
  favouriteBooks: [new Book({ title: 'Jack' })]
});

expect(user.toJSON()).to.eql({
  name: 'Hello John',
  age: 42,
  favouriteBooks: [{
    title: 'Jack',
    added: '123456789',
  }]
});
talyssonoc commented 5 years ago

Hello, @loweoj, sorry for the delay on answering this issue. I really like the idea, but I'm not convinced yet the name should be toJSON since it already receives the output of the serialization :thinking:, did you think of any alternative name?

talyssonoc commented 5 years ago

Actually, after discussing with some other contributors there are some raised concerns about it:

After considering these points, I'm not sure anymore if we should add this feature.

wenderjean commented 5 years ago

In my opinion, the static method toJSON seems more a kind of callback that should actually be named as afterSerialize or something like that, but, we can fall in a lot of problems dealing with this approach since it doesn't matter the feature you're gonna be dealing with the serialization will be always compromised to the behavior declared in afterSerialize.

The second point is, it will assume that the structure model should do more than it is proposed to do, for example, to harbor unnecessary business serialization logic.

I agree with @talyssonoc that complex serialization should live in a serializer layer and this feature will just add complexity to solve a problem more related to architectural design.

loweoj commented 5 years ago

Yup makes sense. Thanks for the input guys.