Non-parsable members silently ignored

[MatthiasKunnen]

I have the following code:

TypedJSON.setGlobalConfig({
    errorHandler: e => {
        console.error(e);
        throw e;
    },
});

@jsonObject
class Test {

    @jsonMember
    foo: string;

    @jsonMember
    bar: string | null;
}

const mapper = new TypedJSON(Test);
console.log(mapper.parse({foo: 'foo', bar: 'bar'}));

The following is logged: Test {foo: "foo"}. No errors are raised and no errors are logged.

I understand what causes bar not to parse namely metadata of string | null is Object. However, this silently failing behavior is dangerous. I would like for an early error to be raised, something like: Cannot determine type of Test.bar. Specify constructor or serializers explicitly.. This check should be performed on all knownTypes and when a type is first seen by TypedJSON.

Let me know your opinion on this and I can take a look how this could be implemented.

[Neos3452]

Hi @MatthiasKunnen, this sounds great! I am trying to add checks where they make sense to improve error detection and this would be a really nice addition. I just pushed my latest work on improving type information so that you can give it a try. One thing to keep in mind is that we had some devs asking for possibility to specify plain object as a type so that TypedJSON checks if an input is an object but does not deserialise it to a specific class.

[MatthiasKunnen]

I just pushed my latest work on improving type information so that you can give it a try

I'll give it a spin

One thing to keep in mind is that we had some devs asking for possibility to specify plain object as a type so that TypedJSON checks if an input is an object but does not deserialise it to a specific class.

Is this the any type requested in #78?

I propose a bigger change to TypedJson by following something closer to TypeOrm and TypeGraphQL.

This would result in the following

import {Any} from 'typedjson';

@jsonObject
class Test {

    @jsonMember
    foo: string;

    @jsonMember(String) // type required because can't be reflected
    bar: string | null;

    @jsonMember({
        type: String, // Also possible for easier backwards compatibility
    })
    barAlternative: string | null;

    @jsonMember(Any) // gets set but no transformation on serialize/deserialize
    any: any;
}

const mapper = new TypedJSON(Test);

The first param of @jsonMember would be able to have a type, constructor, or serializer but no combination. This would make the following type:

export interface IJsonMemberOptionsBase extends OptionsBase {
    /** When set, indicates that the member must be present when deserializing. */
    isRequired?: boolean;
    /** When set, a default value is emitted if the property is uninitialized/undefined. */
    emitDefaultValue?: boolean;
    /** When set, the key on the JSON that should be used instead of the class property name. */
    name?: string;
}

export interface JsonMemberConstructorArg extends IJsonMemberOptions {
    /**
     * Sets the constructor of the property.
     */
    constructor: Function;
}

export interface JsonMemberSerializerArg extends IJsonMemberOptions {
    /** This deserializer will be used to deserialize the member. The callee must assure the correct type. */
    deserializer: (json: any) => any;
    /** This serializer will be used to serialize the member. */
    serializer: (value: any) => any;
}

export interface ClassType<T = any> {
    new (...args: any[]): T;
}

export interface JsonMemberTypeArg extends IJsonMemberOptions {
    type: ClassType | Function | object | symbol;
}

export type JsonMemberArg = JsonMemberConstructorArg | JsonMemberSerializerArg | JsonMemberTypeArg;

The cool thing about this is that we can tackle the any type by exporting our own Any symbol as shown in the first code block. This approach would also tackle my other issue, #131, and the strategy change you laid out.

[MatthiasKunnen]

just pushed my latest work on improving type information so that you can give it a try

1.6.0-rc does not seem to have been released on npm, is that on purpose?

[MatthiasKunnen]

@Neos3452, would you be interested in a PR that achieves the code laid out above?

If I find some time I could try and do this.

[Neos3452]

@MatthiasKunnen a PR would be awesome. I have not released a version on npm because my understanding was that you wanted to create a PR of some sort, and then we can include it in rc-1, give it a spin and if everything runs ok, then do a proper release.

[Neos3452]

Hi, again, I pushed the strategy implementation and released 1.6.0-rc2 so you can give it a try.

[MatthiasKunnen]

An update: I'm currently occupied with some other things but intend to come back later and work on this.

[Neos3452]

I also released 1.6.0-rc3 with all your changes.