Drop unnecessary regexp.prototype.flags

[fregante]

b8c179c introduced this heavy-ish dependency but it didn't explain why. Tests pass without it as the property has been supported forever.

[codecov]

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%)	⬇️

---

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.

[ljharb]

a) dependency “weight” is only relevant if correctness is achievable with a lighter dependency
b) this is absolutely necessary; flags was added in ES6, and thus is not present in, for example, node 0.4, which this package supports.
c) if tests pass with this removed, that simply means I’ve failed to provide adequate test coverage, something i will remedy tomorrow, and then rebase this PR to ensure that indeed it makes the tests fail (in which case, thank you for bringing it to my attention!)

[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.