null check seems unnecessary

[atillabaspinar]

consider this object

const company = {
 addresses: 'aaa'
 contacts: Contact[];
 createdAt: 'some date;
 deletedAt: null;  //this is null!!!
 id: 111;
}

i have a mape like this (deletedAt and createdAt are stripped off)

const map= {
 addresses: 'aaa'
 contacts: Contact[];
 id: 111;
}

then I get deletedAt, although it is not in the map.

I feel like the code here should not check for the null case, if there is no other need for that:

const reducer = (input, map, options) => {
savedOpts = options || savedOpts || {}
Object.keys(input).forEach(function (key) {
if (input[key] !== null) {
innerCompare(input[key], map[key], input, key)
}
})
return input
}

[Acrontum-Carmichael]

What is the input map?

[Acrontum-Carmichael]

Additionally there is: https://www.npmjs.com/package/object-reduce-by-map#example-use

// Pass options
const calculatedWithOptions = objectReduceByMap(input, map, {
  keepKeys: true,
  throwErrorOnAlien: true
})

This will retain keys with null value... however (and at this point neither solution seems accurate) what is better for a documentation driven API output... truly open ended question i don't know which is the correct answer.

[atillabaspinar]

with the objects above, company and map, what I get as the result is

const reducedObject = {
 addresses: 'aaa'
  deletedAt: null;  //this is null!!!
 id: 111;
}

The problem is that, I get deletedAt, even if it is not in the map. So, the problem is not keeping the null properties, on the contrary, stripping them from the result.

[Acrontum-Carmichael]

that is the input object @atillabaspinar what is the input map?

[p-mcgowan]

yeah, maybe the destinction here is null and undefined

undefined is a horrible thing. if a key's value is undefined, that just means it does not exist - in js(on) terms, { key: undefined } is meaningless, as that's just { }

in the case of null, that's the explicit setting to a non-value, or unsetting. It's basically every other languages version of undefined. IMO, you should never write the word undefined anywhere in the code base - you can always do the same thing without actually writing it

So if what keepKeys does is add undefined keys for everything in the map, but ignore null when it's false, then there may be a way to deal with this. undefined and null shouldn't be treated the same

[atillabaspinar]

@johndcarmichael the map, i had wrote above but i fixed a bit here. The important thing is that it does not contain "deletedAt"

map:  {
 addresses = string[]
 contacts: Contact[];
 id: number;
}

[Acrontum-Carmichael]

ahhhh sorry i think i misread the original post - ok i get the issue now.. yea that should not happen

[Acrontum-Carmichael]

it has been a while since i wrote that code - but i think this area could likely be the culprit: https://github.com/johndcarmichael/object-reduce-by-map/blob/master/src/reducer.js#L57 this is where the attribute is removed and looks like the condition needs extending.

[atillabaspinar]

or can it be this line? https://github.com/johndcarmichael/object-reduce-by-map/blob/6a7806ce57603bc13b711fab9fca221ebcf14930/src/reducer.js#L92

So if we have user = { name: null, lastname: 'doe' } and map = {lastname: string }

then name is still in the output where is it should have been removed.

I am not sure why there is check at line 92, but maybe it should be like, if (input[key] !== undefined)

[Acrontum-Carmichael]

Maybes - but have a go at tweaking those conditions; the area i pointed to and the area you pointed to. There are tests written but i seems that this condition was not accounted for.

The thing should handle null values: https://github.com/johndcarmichael/object-reduce-by-map/blob/master/__tests__/nullPass_test.js

But it seems that the condition you have found is not covered in this test: https://github.com/johndcarmichael/object-reduce-by-map/blob/master/__tests__/inputReducer_test.js

In your case when the attribute is not in the map it is not removed as the value is null not undefined where are the check it looks like is only removing the attr when the leaf value is undefined... when the value is null but defined type is not it should be removed which i think is where the issue lives.

Any PR must not break the existing tests though as this tool is used in many APIs

[Acrontum-Carmichael]

@atillabaspinar i think this rc release solves the issue.