Support for ECMA6 .to.be.a('map') & .to.be.a('set')

[davelosert]

Right now, when I test a variable on to.be.a('Map') (or 'Set'), it throws:
AssertionError: expected {} to be a map.

Are you guys planning on implementing ECMA6 features?

[logicalparadox]

Would be a great idea! Our type detection is implemented in chaijs/type-detect. PR's accepted.

/cc @keithamus

[keithamus]

Thanks for the issue @Charminbear.

We can totally add these! It'd be quite simple to do so. All of our native types are listed in utils/type.js L11-19. If you'd like to extend these to add the es6 types, that'd be great! To my knowledge the new ES6 types are as follows:

[object Set]
[object WeakSet]
[object Map]
[object WeakMap]
[object Symbol]
[object Promise]
[object Int8Array]
[object Uint8Array]
[object Uint8ClampedArray]
[object Int16Array]
[object Uint16Array]
[object Int32Array]
[object Uint32Array]
[object Float32Array]
[object Float64Array]
[object DataView]

Of course, ES6 makes things a little more challenging because Symbol.toStringTag exists - which allows runtime overriding of this. So maybe, if you feel up to the challenge, you could refactor the whole function to be more generic. Rather than returning natives[str] it'd have to return something like str.match(/^\[object (.*)\]$/)[1].toLowerCase(), making the whole function:

module.exports = function (obj) {
  if (obj === null) return 'null';
  if (obj === undefined) return 'undefined';
  if (obj === Object(obj)) return 'object';
  return Object.prototype.toString.call(obj).match(/^\[object (.*)\]$/)[1].toLowerCase();
};

Of course the real challenge comes with adding tests for these. I'll leave it up to your discretion how much effort to put into the tests, but there should be enough coverage to ensure new PRs don't regress the behaviour (perhaps stubbing out Object.prototype.toString to return your desired return types would suffice?). If you need more guidance on this let me know and I'd be happy to help.

[davelosert]

@keithamus That sounds good to me. I'm gonna have a look into it this week. Thanks a lot for the guidance already, thats already lots of thinkwork.

[logicalparadox]

We never switched over to "type-detect"? Could have sworn we did. Could we use this opportunity to do so please?

[keithamus]

Sorry @logicalparadox didn't notice your message!

Yeah, @Charminbear if you could kindly make the pr against type-detect and also make a PR against this repo to move to type-detect that'd be just swell. Plus you'll end up on our hall of fame!

[davelosert]

@keithamus Yes of course, im gonna do it like this then. :)

[davelosert]

@keithamus @logicalparadox
I finally could spend some time on this issue today. I managed to simplify the getType-Method to:

  function getType(obj) {
     var type = Object.prototype.toString.call(obj).match(/^\[object (.*)\]$/)[1].toLowerCase();
     if(type === 'string' && typeof obj === 'object') return 'object'; // Let "new String('')" return 'object'
     if (obj === null) return 'null'; // PhantomJS has type "DOMWindow" for null
     if (obj === undefined) return 'undefined'; // PhantomJS has type "DOMWindow" for undefined
     return type;
   }

I had to remove the obj === Object(obj)-condition as it resolved to true also for all wrappers of primitives (e.g. Arrays). The only test suffering from this was new String('') - so i added a condition for that special case. For all other cases it would actually suffice to have your Regexp - but for some weird reason, PhantomJS toString returns [object DOMWindow] for undefined and null (they already have an Issue to that here).

As you said, the challenge was with testing: Mocha's Browser-implementation obviously doesnt like it when you Stub out Object.prototype.toString, as it uses it even before the calls to after or afterEach - and then runs into some kind of infinity loop (it bascially ignores all tests that do so). So i had to restore the method within the first call to the stub itself. Not really beautiful but it works.

I wrote some simple good-case-tests for each of the new Type you posted above. It's hard to think about other test than these when you can't even use the real Types, so I have a question:
On which Browsers do those tests actually have to be able to run? All supported ones of chai itself?

Depending, one could think to actually use the already implemented types (and I would happily do that, too).

[keithamus]

Hey @Charminbear, thanks for the write-up. Glad to see you're making progress.

Having the catch for null and undefined isn't the worst - as long as we comment/test it to ensure it doesn't get removed.

As for what our tests run on: https://github.com/chaijs/chai/blob/master/sauce.browsers.js is one list, plus we need to support PhantomJS & Node. So the full list:

• Chrome
• Firefox 22
• Opera 12
• Opera 11
• IE10
• Safari 6
• Safari 5
• Node .10
• PhantomJS

Native support for ES6 constructors is... sporadic at best. We could test for presence and have a conditional test - but if we do I'd very much still like to see a stubbed out version that runs in every browser, so we know we're catching it.

Right now, if you've got a good chunk of code then the best course of action is to set it up as a PR, and we can discuss the finer points there.

[davelosert]

Hey @keithamus
Okay, pull request is out, so we can move the discussion about this there.

One more thing about the second task:
While I was trying to move this repo to type-detect, i realized that they have a minor difference in expectations: While native chai expects new Number() to be a 'number' and new String() to be a 'string', type-detect right now expects the same for the first, but ' object' for the latter. Maybe you guys should decide if both is going to be an object or both the native type - depending on your decision, i can adjust the tests while im already on it.

[logicalparadox]

@Charminbear My thought is that native chai is the authority: number and string. @keithamus agree?

[keithamus]

Yup. Agree.

[davelosert]

Alright. Then im gonna wait until I'm done with my work on type-detect and you guys published a newer version before moving here to not break any tests.

[rocketraman]

👍

Note that currently

assert.deepEqual(new Set(['foo']), new Set(['bar']))

results in a passing test! If Chai does not know how to compare a type, should it not throw an error so that the user knows to work around the limitation?

[hildjj]

Status on this? I don't see a pending PR, as alluded to above.

[keithamus]

@hildjj we had a PR to detect the type, and it is now available since Chai 3.0.0. @rocketraman's issue is a different issue, which should absolutely be addressed - but let's make a new issue to tackle that - this one is done.

[keithamus]

@hildjj @rocketraman see chaijs/deep-eql#4. PRs welcome 😄

[teameh]

Maybe a bit off topic.. but are there any assertions for for ES6 Maps (and Sets)?

For example something like:

expect(myMap).to.have.key('some_key');

I can use:

expect(myMap.has('some_key'), 'map should have some_key').to.be.true;

for now.. but that doesn't work great.

[keithamus]

@tiemevanveen you should raise a new issue - we can discuss the design of this and work towards adding it as a feature 😄

[teameh]

Here you go #632 :)