Added handling for undefined and null on expect empty (fails on negate flag too)

[Daio-io]

• Added handling for undefined and null on expect empty

@keithamus Is chai 4.x.x going to be focused towards ES6? If so I can use Object.keys() to coerce primitives.

Also there is the option to do this:

else if (obj.hasOwnProperty('length')) {
  expected = obj.length === 0;
 }

Which would cover any new objects with length, but it would add support for checking if a function takes parameters or not (not sure that's what you would want from an empty check..).

Just an idea. I will wait for your feedback.

[keithamus]

This is not ideal - setting the negate flag will apply to the rest of the chain. Better to do a simple type check at the beginning and error out early.

[keithamus]

Overall some good work @Daveloper87 - but see my comments above.

I think in summary, I see the assertion for empty being something like this, taking a leaf from the lengthOf assertion.

Assertion.addProperty('empty', function () {
    var obj = flag(this, 'object');
    new Assertion(obj).to.exist;
    this.assert(
      Object.keys(obj) === 0
      , 'expected #{this} to be empty'
      , 'expected #{this} not to be empty'
    );
  });

This should assert first that the object is not null or undefined - after which it will look at the keys to ensure they are 0.

[Daio-io]

Thanks for your comments, The lengthOf approach and the approach you have outlined in your comment looks a lot cleaner. Was not sure if it was ok to use other assertions, but looking at everything again it seems like its used throughout. So that looks good to me.

My only question is around Object.keys(). Is chai 4.x.x only supporting ES6? because Object.keys('some string') will fail because it can not be coerced to an object so will throw a TypeError.

[keithamus]

Using other assertions will probably one day not be ideal - but don't worry about that for now, just follow existing code conventions.

Do you have evidence about ES6 Object.keys? Without offending I'd be surprised if ES6 makes breaking changes, so I'm somewhat doubtful of this. Looking at the spec[1] it seems to use [[ToObject]] which explicitly mentions turning string literals into new String(value) (which have keys).

[Daio-io]

Sorry, I did not explain that well.

Object.keys('hello') would fail in browsers that do not currently have support for ES6. This will include node (without harmony). If you try that in node console or a browser like Safari, you should see a TypeError

[keithamus]

Ah, ES5 Object.keys is missing [[ToObject]] call. Learn something new every day 😄! Luckily we can easily emulate this by wrapping the value in Object():

var obj = 'hello';
Object.keys(Object(obj)); // ["0", "1", "2", "3", "4"]

var obj = [1, 2, 3];
Object.keys(Object(obj)); // ["0", "1", "2"]

var obj = 42;
Object.keys(Object(obj)); // []

Thoughts?

[Daio-io]

Ah great! Likewise! I have pushed a new commit based on your comments.

numbers and functions will be classed as empty, but I guess if you expected an empty array and got a function, you have bigger problems...

[keithamus]

Great work @Daveloper87 😄.

It'll be a while before the 4.x.x branch gets merged - #457 is our roadmap for now, which we're trying hard to focus on (namely modularising the chai codebase). If you've caught the PR bug then feel free to take a look at some of those and submit some PRs, they'd be very welcome!

[Daio-io]

@keithamus Great! I will have a look over the coming weeks and try to dig in :)