fix(serialization): run custom serializer on getters #4860
Conversation
Codecov ReportAll modified and coverable lines are covered by tests ✅
... and 6 files with indirect coverage changes 📢 Thoughts on this report? Let us know!. |
| @@ -97,7 +97,7 @@ export class EntitySerializer { | |||
| // decorated getters | |||
| meta.props | |||
| .filter(prop => prop.getter && typeof entity[prop.name] !== 'undefined' && isVisible(meta, prop.name, options)) | |||
| .forEach(prop => ret[this.propertyName(meta, prop.name, wrapped.__platform)] = entity[prop.name]); | |||
| .forEach(prop => ret[this.propertyName(meta, prop.name, wrapped.__platform)] = this.processProperty(prop.name, entity, options)); | |||
|
|
|||
| // decorated get methods | |||
| meta.props | |||
There was a problem hiding this comment.
we should do the same here as well, right?
There was a problem hiding this comment.
Yep, had to do some small changes. First off, added a getterName === undefined check to the accessor serializer. This was to avoid it being called twice (for method getters too). Then I modified the processProperty function so that if the property is a function (the method getter case) then it gets the corresponding value by calling the function. Let me know if I'm missing something or if you can think of a more elegant way of doing it 👍
| @@ -97,7 +97,7 @@ export class EntitySerializer { | |||
| // decorated getters | |||
| meta.props | |||
| .filter(prop => prop.getter && typeof entity[prop.name] !== 'undefined' && isVisible(meta, prop.name, options)) | |||
| .forEach(prop => ret[this.propertyName(meta, prop.name, wrapped.__platform)] = entity[prop.name]); | |||
There was a problem hiding this comment.
actually, this also needs to be addressed in the EntityTransformer class, which is used for implicit serialization. which reminds me i wanted to look into deduplicating this in v6, now the two classes are almost the same
There was a problem hiding this comment.
Could you point me to some examples where EntityTransformer is being used as to test it accordingly? Thanks!
There was a problem hiding this comment.
wrap(entity).toObject() is using it, same for JSON.stringify(entity)
There was a problem hiding this comment.
added test cases for wrap(entity).toObject() 👍
| @@ -129,6 +129,9 @@ export class EntitySerializer { | |||
|
|
|||
| /* istanbul ignore next */ | |||
| if (!options.ignoreSerializers && serializer) { | |||
| if (entity[prop] as unknown instanceof Function) { | |||
| return serializer((entity[prop] as unknown as () => T[keyof T & string])()); | |||
There was a problem hiding this comment.
This is the same cast that was being used to obtain the getter method value.
|
Will squash commits once the pr is ready to conform with commit format. |
|
I'm looking into the test failure. Things broke with the |
Fixes #4859