Chai: Type check for parsePath variable would be nice to have

Created on 8 Sep 2017  路  3Comments  路  Source: chaijs/chai

When calling the parsePath function (https://github.com/chaijs/chai/blob/master/chai.js#L10117) type checking can help users identify incorrect usage of API.

Problematic line:

function parsePath(path) {
   var str = path.replace(/([^\\])\[/g, '$1.[');

When running this (slightly incorrect) code:

expect(data).to.have.nested.property({'a':'1'})

The following error is thrown:

TypeError: path.replace is not a function

Because path in this case is an object {'a':'1'}
Would be nice to throw a more descriptive error :)

bug pull-request-wanted

Most helpful comment

Hey @keithamus , I would like to give a try to this.

All 3 comments

Hey @sverrirs thanks for the issue

You're totally right, so I've raised this as a bug with a pull-request-wanted label. Our property assertion should only accept strings - so we should raise the error there. Here's the assertion code:

https://github.com/chaijs/chai/blob/dbeae08fe526cbe79634bae07613fc427a8a53aa/lib/chai/core/assertions.js#L1759-L1825

And here's an example of how we can do type-checking to ensure that name is a string:

https://github.com/chaijs/chai/blob/dbeae08fe526cbe79634bae07613fc427a8a53aa/lib/chai/core/assertions.js#L1152-L1165

So effectively, we'll need to add the following the property assertion:

if (typeof name !== 'string') {
  var msgPrefix = flag(this, 'message')
  msgPrefix = msgPrefix ? msgPrefix + ': ' : ''
  throw new AssertionError(msgPrefix + 'the argument to `property` must be a string')
}

Anyone who wants to is welcome to make this PR; if you do decide you want to make this PR please add a comment below so others do not work on it at the same time!

The value only needs to be a string if the nested flag is set. If the nested flag isn't set, the assertion should also accept Symbols:

const prop = Symbol();
expect({[prop]: 4}).to.have.property(prop);

Arguably, it should accept numbers too, even though there is type coercion:

expect({42: 'secret'}).to.have.property(42);

Hey @keithamus , I would like to give a try to this.

Was this page helpful?
0 / 5 - 0 ratings

Related issues

endymion00 picture endymion00  路  3Comments

meeber picture meeber  路  3Comments

qbolec picture qbolec  路  5Comments

meeber picture meeber  路  4Comments

JuHwon picture JuHwon  路  5Comments