Chai: Inspect doesn't work properly for Maps and Sets - It displays an empty object

[lucasfcosta]

As I was implementing the .deep flag support on the keys assertion I noticed that utils.inspect isn't working properly when it comes to Maps and Sets. Instead of returning the values inside the Map or Set it returns the following string: {}.

You can reproduce this behavior by doing, for example:

chai.use(function (_chai, utils) {
  var inspectSet = _utilsinspect(new Set([{iAmAn: 'item'}, {thisIs: 'anotherItem'}]));
  var inspectMap = utils.inspect(new Map([[{iAmA: 'key'}, 'aValue']]))
  console.log(inspectSet); // --> {}
  console.log(inspectMap); // --> {}
});

What do you guys think should be the desired output when inspecting maps or sets?

IMO the ideal output would be:

chai.use(function (_chai, utils) {
  var inspectSet = _utilsinspect(new Set([{iAmAn: 'item' }, {thisIs: 'anotherItem'}]));
  var inspectMap = utils.inspect(new Map([[{iAmA: 'key' }, 'aValue']]))
  console.log(inspectSet); // Desired --> [ {iAmAn: 'item' }, { thisIs: 'anotherItem' } ]
   console.log(inspectMap); // Desired --> [ [ { iAmA: 'key' }, 'aValue' ] ]
});

---

I think this wouldn't be a difficult fix. Maybe adding a check alongside these other checks and then calling a function to iterate through every entry and format the Map/Set members should be enough. This will be very similiar to what we already do in formatArray.

I will make sure to submit a PR for this before implementing the deep flag for Maps/Sets because this will be very useful to create good test cases.

Please let me know if you've got any idea or opinion about this :wink:

[keithamus]

Good work @lucasfcosta - this is definitely a valid issue. As per the roadmap issue (https://github.com/chaijs/chai/issues/457) we need to do some work to get the inspect functions from chai core into chaijs/loupe. I'd like to do this before we hit 4.0.0, and as soon as I've completed the work I'm doing with chaijs/deep-eql I'll be picking up Loupe as there's a few things I'd like to tackle with it.

[lucasfcosta]

@keithamus Thanks for the info!
Can I tackle this one and send a PR against master here or should I start working at chaijs/loupe?

[keithamus]

I've got some other bits I want to work on for chaijs/loupe so hold off on this one just for now. We'll get this fixed for 4.0 though :smile:

[keithamus]

Marking this as pull-request-wanted as my refactor of loupe is quite far away. If you're reading this, feel free to pick this up and make this change for loupe!

[lucasfcosta]

Important Note

Please enable the tests added on #668 which depend on this before closing this issue.
Which, by the way, are clearly indicated on the code with comments pointing to this issue.

[maxnordlund]

This bit me today, and I see that 4.x has been released without loupe. So the question is, should this be done here in the meantime or is loupe ready for inclusion?

In any case, I think this should be kept open until it has been fixed, yes?

Here's node utils inspect for reference.
https://github.com/nodejs/node/blob/e958ee7a70d0af136fae8a708882cccdd4601658/lib/internal/util/inspect.js

[keithamus]

@maxnordlund we're trying to track issues like this on our board: https://github.com/chaijs/chai/projects/2 as we get many duplicates or similar issues. You can see https://github.com/chaijs/chai/projects/2 for progress on this. Loupe will be released with chai 5 😄

[maxnordlund]

@maxnordlund we're trying to track issues like this on our board: https://github.com/chaijs/chai/projects/2 as we get many duplicates or similar issues. You can see https://github.com/chaijs/chai/projects/2 for progress on this.

Aha, I didn't see that.

Loupe will be released with chai 5 😄

Sweet, I'm looking forward to it. Though for native WeakMap and WeakSet, only the native inspect can look inside them, which may not be a problem but still, worth mentioning.

[keithamus]

Loupe will be at-least as good as Node's console.log, but yes WeakMap and WeakSet are unobservable and so will display as something like WeakSet{...}. Promise can be observed using some node plumbing but in browsers it cannot, and as such in browsers we'll have something like Promise{...}

[jfirebaugh]

This got accidentally closed because #668's description contained the text we need to fix #662 in order to enable all tests. Reopening.

[gagern]

This bug had me wondering for a while why my set was empty until I found it wasn't:

expec(new Set([1,2,3])).to.contain(7)
// => AssertionError: expected {} to include 7

I wonder whether it would make sense to leverage util.inspect when running on Node. That would not only handle sets, but also give access to any custom representation people might have implemented for their own types.

[keithamus]

Here's the todo list for releasing loupe v2: https://github.com/chaijs/loupe/issues/15. Once released we can bring it into Chai and probably release a 4.4 version to get this in before Chai v5.

Any one who wants to help out with the todo list, we'd super appreciate it!