Drop unnecessary `regexp.prototype.flags`

[fregante]

https://github.com/inspect-js/node-deep-equal/commit/b8c179c5aa91c8a2f71f053e2d9e2d477780250e introduced this heavy-ish dependency but it didn't explain why. Tests pass without it as the property has been supported forever.

[codecov[bot]]

Codecov Report

Merging #91 (adee886) into master (29c8a0d) will decrease coverage by 0.00%. The diff coverage is 100.00%.

@@            Coverage Diff             @@
##           master      #91      +/-   ##
==========================================
- Coverage   98.12%   98.11%   -0.01%     
==========================================
  Files           2        2              
  Lines         213      212       -1     
  Branches       88       88              
==========================================
- Hits          209      208       -1     
  Misses          4        4              

Impacted Files	Coverage Δ
index.js	98.10% <100.00%> (-0.01%)	:arrow_down:

---

Continue to review full report at Codecov.

Legend - Click here to learn more Δ = absolute <relative> (impact), ø = not affected, ? = missing data Powered by Codecov. Last update 29c8a0d...adee886. Read the comment docs.

[fregante]

node 0.4, which this package supports.

Ouch!

dependency “weight” is only relevant if correctness is achievable with a lighter dependency

Indeed. It just feels unnecessary to support something a handful of people use, at the expense of everyone else. If people haven't moved on from Node 0.4, I wouldn't expect them to update dependencies either, so an hypothetic deep-equal v3 with Node 6+ wouldn't affect them.

[ljharb]

If you know you only target browsers with flags support, you can (and should) certainly configure your bundler to alias out the package in favor of x => x.flags or similar. In node, though, the cost is effectively zero.

[ljharb]

Also, updating node is much harder than updating deps, so your assumption doesn’t hold up. Certainly a v3 could drop that support, but that seems capricious.

[ljharb]

Rebased; I expect to see everything fail below node 6 (when flags was added).

[fregante]

In node, though, the cost is effectively zero.

I disagree:

• regexp.prototype.flags means npm installing 18 modules, which doesn't take 0ms
• regexp.prototype.flags is required regardless of support, which takes longer than 0ms;
• regexp.prototype.flags is executed regardless of support, which likely takes longer than the V8 implementation

[fregante]

updating node is much harder than updating deps

Yeah but would you connect Windows XP to internet? So why would one run a severely outdated server? 😄

[ljharb]

You're right that there's some install time impact. At runtime, the impact should be negligible. In other words, the cost isn't precisely zero, but it's effectively zero, as I said.

Regardless, a minor inconvenience for modern node/browser users, versus "you can't use this package" for users that need to run on older node/browsers, is not an equivalent comparison.

[fregante]

Would it make sense to leave them on v2 and release a v3 for modern engines? They can use v2 forever.

[ljharb]

Even if i wanted to sign up for the maintenance burden of two simultaneous release lines (otherwise the v2 users would miss out on bugfixes and new features), .flags remains a brittle approach if someone has done delete RegExp.prototype.flags, so it wouldn’t be a complete improvement.