Return false for non-empty objects

[a-morn]

Currently always returns true. Should return false for non-empty objects.

Note that Object.keys(obj).forEach(key => { if (key in obj) return false; }); doesn't do anything :(

[kalinchernev]

Hi @a-morn I've previously explained the goal of the function and probably discussed that as well but can't find these.

The reason for this helper to be is to clean-up the specification before giving it to the user. So, it's not a question of an empty object, but an object with properties which are empty objects. The helper was added during the work on supporting open api specification because there is a difference between v2 and v3 of what properties are required in the final spec and what fields are required not to be present.

It's not that all this logic shouldn't be refactored btw, it's just trying to plumb original logic with new rules ...

[a-morn]

Hi @kalinchernev!

To clarify: the current functionality is that isEmptyObject always returns true. Object.keys(obj).forEach(key => { if (key in obj) return false; }); does not do anything. So the function might as well have been function isEmptyObject() { return true }. Its output is not dependant on its input. If that is intended then we could just remove the function and the if statement on line 29 altogether :)

[kalinchernev]

@a-morn that's my test:

const isEmptyObject = obj => Object.keys(obj).every(key => !(key in obj));

// should return true
const obj1 = {foo: {}}

// should return false 
const obj2 = {foo: { bar: 'baz' }}

isEmptyObject(obj1)
>> false
isEmptyObject(obj2)
>> false

I tested with current function and indeed it's also wrong. Though update does not achieve improvement. Thanks for making me double-checking it though, it's for sure wrong and needs to be fixed :)

If you have a better way, please update the PR

[kalinchernev]

@a-morn thanks so much for taking the initiative to improve this!
Could you please also add a test for this helper here in order to self-explain for future discussions?
Thank you!

[a-morn]

Test added. Please check that the test cases look right.

Thanks for maintaining this lib! :)

[kalinchernev]

@a-morn nice! Thanks!

[bvanderlaan]

Object.values does not exist in Node v6
This should result in a major/braking change version update so as to not break those still using Node v6

[a-morn]

Could change it too:

Suggested change

	return Object.values(obj).every(
	return Object.keys(obj).map(key => obj[key]).every(

and consider it a bug fix. This shouldn't warrant a major version bump.

Or transpile the entire lib.

[kalinchernev]

No transpilations please. I had to think about that, but didn't. Most certainly it's not a major version bump.

[a-morn]

@bvanderlaan #145