validatorjs / validator.js

String validation
MIT License
23.01k stars 2.29k forks source link

Getting rid of "includes" polyfill-type function in util folder #2130

Open pano9000 opened 1 year ago

pano9000 commented 1 year ago

Hello all,

I recently noticed the includes function in the util folder, which was originally added in 2018, as some sort of polyfill for the Array.prototype.includes() method:

https://github.com/validatorjs/validator.js/commit/9457642473d3e282310a829754929463e6998941 - "Created includes function for array to cater older browsers"

I see a few issues with this in the meantime:

node .\benchmark.js
  with native `Array.prototype.includes()` x 843,902,658 ops/sec ±0.18% (91 runs sampled)
  with current `Array.prototype.some()` polyfill type x 147,008,135 ops/sec ±0.10% (95 runs sampled)
Fastest is with native `Array.prototype.includes()`

So question here is: Am I wrong about Babel? If not: Are there any reason to still keep this includes function? That util function is not exported, so there also shouldn't be any backwards compatibility issues, that we need to keep in mind.

What are your thoughts? Happy to take care about the PR for this then

Kind regards,

Pano

WikiRik commented 1 year ago

Related PR; https://github.com/validatorjs/validator.js/pull/2099

I think that Babel can take care of this, but I don't know if this project has the proper configuration for this.

pano9000 commented 1 year ago

@WikiRik thank you for the comment, and making me aware about #2099 I will make a comment there, about my thoughts here