shakilsiraj / json-object-mapper

A TypeScript library to serialize and deserialize object graph from/to JSON in a fast and non-recursive way
MIT License
58 stars 18 forks source link

Bugfix: nested array is undefined #21

Closed devpreview closed 7 years ago

devpreview commented 7 years ago

Before:

class TestObject {
    @JsonProperty()
    public field: string[];
}

let json = {};
let result = ObjectMapper.deserialize(TestObject, json);
Array.isArray(result.field); // return false, 'result.field' is undefined

After:

class TestObject {
    @JsonProperty()
    public field: string[];
}

let json = {};
let result = ObjectMapper.deserialize(TestObject, json);
Array.isArray(result.field); // return true, 'result.field' is empty Array
ghost commented 7 years ago

I'm curious, the "Before" looked like normal behaviour to me. Maybe I misunderstood, but if a field is completely absent from a JSON file we're trying to deserialize, why should it return a value in the TS object ? The issue #10 is indeed about a behaviour that is wrong to me, but this is not the use case described in this issue. I'm confused.

devpreview commented 7 years ago

@Orhleil I think is that the behavior should not depend on the input JSON.

devpreview commented 7 years ago

@Orhleil Maybe you're right.

I fix this bug:

class TestObject {
    @JsonProperty()
    public field: string[];
}

let json = {
    'field': []
};
let result = ObjectMapper.deserialize(TestObject, json);
Array.isArray(result.field); // return false, 'result.field' is undefined
devpreview commented 7 years ago

I think that the correct behavior is:

class TestObject {
    @JsonProperty()
    public field: string[];
}

ObjectMapper.deserializeArray(TestObject, {}); // return `undefined` or thrown error
ObjectMapper.deserializeArray(TestObject, []); // return empty array

let result = ObjectMapper.deserialize(TestObject, {});
// result.field is undefined or thrown error

let result = ObjectMapper.deserialize(TestObject, { `field`: [] });
// result.field is empty array
ghost commented 7 years ago

Yes, that last comment shows the correct behaviour :)

Sent from my Bq Aquaris X5 Plus using FastHub