optional does not work well with custom

[akphi]

Hi,

I tried to define a property schema nesting custom inside an optional for my serializer and it does not seem to work well. I have looked at the codebase and came up with the following test case:

test("optional properties should work with `custom` serializer", t => {
    var schema = {
        factory: () => ({}),
        props: {
            optionalProp: optional(custom(
                value => value.id,
                () => SKIP
            )),
        }
    }
    t.deepEqual(serialize(schema, { optionalProp: undefined }), {})
    t.end()
})

This will throw TypeError: Cannot read property 'id' of undefined

My expectation is optional will first check if the value is undefined, if so it returns SKIP, otherwise it will proceed, am I missing something here?

[pyrogenic]

optional avoids serializing properties with undefined values. When wrapped with optional, if a your custom serializer returns undefined, no property will appear in the JSON at all. In your case, either return value?.id (or value ? value.id : undefined), or remove the optional and return value ? value.id : SKIP.

[akphi]

@pyrogenic then I guess optional returns SKIP if the value it wraps is undefined. Maybe we should clarify the doc a bit about this. Since my perception is serializer are read from outer to inner -- e.g. list(object()) will do list() first, which led me to believe optional(custom()) will do optional() check first, but it didn't

[pyrogenic]

I guess optional returns SKIP if the value it wraps is undefined

That is exactly correct 😄

    function serializer(...args) {
        const result = propSerializer(...args)
        if (result === undefined) {
            return SKIP
        }
        return result
    }

I think I understand your confusion — I wouldn't think of the optional decorator as a "check". Both list and optional operate on the result of using the using the given schema to serialize data.

You're welcome to suggest a change to the docs.

/**
 * Optional indicates that this model property shouldn't be serialized if it isn't present.
 *
 * @example
 * createModelSchema(Todo, {
 *     title: optional(primitive()),
 * });
 *
 * console.dir(serialize(new Todo()));
 * // {}
 *
 * @param {PropSchema} propSchema propSchema to (de)serialize the contents of this field
 * @returns {PropSchema}
 */

[akphi]

@pyrogenic gotcha, I have submitted a tiny PR #131.

Could you give that a quick look? Thanks!

Let me know if I should just close this issue and keep the PR open as well.

[pyrogenic]

Resolved with #131

[pyrogenic]

Published new docs @ https://www.npmjs.com/package/serializr/v/2.0.3