Closed Ajaxy closed 1 year ago
Hi, thanks for contributing and sorry to keep you waiting. I'm a bit busy at the moment but i'll try to review your commits today. Looks OK at first glance.
Hey @Ajaxy. Sorry for the delay. I think the idea of having an ability to merge several serialization schemes certainly makes sense. However, after giving it all some thought, i'm seeing several problems with the way schemes are merged in your code. Sure, schemes can be combined generically - as objects - but the question remains will the resulting scheme indeed act as expected? Consider the following:
Because the exclude
list takes precedence over the include
list, a field exclude
d in any of the merged schemes won't be included in the result no matter what if we use simple list concatenation. This can render certain groups of schemes useless when merged together. Better approach IMO would be to apply include
/ exclude
lists consecutively for each scheme in the sequence.
What if more than one scheme has the postSerialize
hook defined? It might be good idea to chain postSerialize
hooks into arrays and handle this case appropriately.
Finally, my initial idea was to have all schemes in my code defined as explicitely as possible. Making it too easy to generate schemes ad-hoc (even by just merging other schemes) doesn't fully fit into the way i imagined it. On the other hand, being able to apply schemes to one another would definitely be useful. I think maybe for this purpose we could introduce new field(s) into Scheme object, like extendsScheme
and/or includesSchemes
. The first one would make the current scheme "inherit from" the given one, by taking the parent scheme's attributes and building over them. The second field would allow for inclusion of multiple named schemes into the current one. The order in which schemes are merged does actually matter, that's why two separate fields would be useful.
Have to meditate on this some more but i think such feature will come in few days.
Hi. This makes sense. But it'll be a little harder to mix it the way serialize(user, ['default', 'withStatus', 'withParents', 'withChildren'])
. What if just to suggest a convention or disclaimer to not add anything ambiguous to mixing schemes?
I can imagine that the main case of mixing is just adding some extras to a solid scheme.
Maybe add some partials and allow to mix only them, i.e. serialize(user, {extend: 'default', with: ['_status', '_parents', '_children']})
Yeah, i think i might add a feature like this as well. Stay tuned.
@hauru