util.isDeepStrictEqual regex comparison

[Xotic750]

• Version:
  v10.14.1
• Platform:
  Mac OS Darwin Kernel Version 18.6.0
• Subsystem:

const util = require('util');

const rx = /a/;
rx.lastIndex = 3;

util.isDeepStrictEqual(rx, /a/); // true

util.inspect(rx, {showHidden: true}); // '{ /a/ [lastIndex]: 3 }'
util.inspect(/a/, {showHidden: true}); // '{ /a/ [lastIndex]: 0 }'

My expectation would have been false. I see that areSimilarRegExps does not perform a check on lastIndex, and keyCheck performs a comparison of enumerable keys, and lastIndex is not enumerable. This may be considered correct, but it feels wrong to me.

https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/RegExp/lastIndex

Thankyou for your time and consideration.

[cjihrig]

Not saying the existing behavior is right or wrong, but it does behave as documented. The util.isDeepStrictEqual() documentation points to the assert.deepStrictEqual() documentation, which contains a comparison details section. The comparison details section specifically states: Only enumerable "own" properties are considered.

[Xotic750]

Yes, I saw that and yes it behaves as described. And yes, I guess it is a question of "is this behaviour correct for strictly comparing regexes"? They are similar, but you would get a different result from using them.

[Trott]

@BridgeAR

[BridgeAR]

This is an interesting edge case. It would be good to know how other assertion libraries work. In some cases it'll likely be good to distinguish the regular expression based on lastIndex while in other cases it might be better to ignore it. I guess most people will just pass through "fresh" regular expressions where lastIndex is set to 0. That's why my feeling is +0.5 on adding a check for this specific case.

[Xotic750]

These days many are deferring to the Node isDeepStrictEqual spec, so this is becoming the measuring stick. In my own previous library I compared lastIndex as I feel it is the right thing to do, I don't think Lodash's isEqual does but again that is being deferred to this spec. I don't believe that ljharb's is-equal compares this. Chai's does not. Many others are just reworks of the previous Node deepEqual, which did not. Many just rely on comparing left and right toString which clearly does not consider this. For me it feels that a deep equal perhaps should not but a deep strict equal should.

[XadillaX]

How about to add an option parameter for this function?

e.g.

assert.deepStrictEqual(a, b, { supportRegexp: true });

[Xotic750]

At worst, document the edge case and say that the developer must perform this check if necessary?

const util = require('util');

const rx1 = /a/;
rx.lastIndex = 3;
const rx2 = /a/

const isEqual = util.isDeepStrictEqual(rx1, rx2) && rx1.lastIndex === rx2.lastIndex;