ramda-adjunct icon indicating copy to clipboard operation
ramda-adjunct copied to clipboard

feat(877): matchall

Open ltruchot opened this issue 5 years ago • 5 comments

Contains:

  • native curried matchAll
  • ponyfill curried matchAll
  • tests
  • typescript definition
  • export

Decisions/Tradeoffs:

  • native "matchAll" returns an iterable type: RegExpStringIterator. This is why most of the examples look like [..."testtest".matchAll("te")], spreading the iterable into a regular Array. Creating this kind of iterator in polyfill involves symbols, generators, and other ES6 elements that should be polyfilled too, in a long and not perfect chain. So I decided to directly return the Array as a result, making some rarely used optimization of matchAll impossible.
  • EcmaScript official documentation says that matchAll non global regex should throw an error (@see 21.1.3.12 in https://tc39.es/ecma262/#sec-string.prototype.matchall). Main browsers implements that behavior but node.js 12+ doesn't, so there is no test to "test this error" (except for the ponyfill that enforce the correct error).

ltruchot avatar Oct 17 '20 12:10 ltruchot

@char0n about the codeclimate error: not sure if we want to avoid this glue-code in the ponyfill since there is no "helper" or "shared" folder here, but I can do it if you prefer!

ltruchot avatar Oct 17 '20 12:10 ltruchot

@char0n Some news about this feature ?

ltruchot avatar Nov 23 '20 17:11 ltruchot

@ltruchot sorry this fell through my net. Are you willing to continue working on this PR?

char0n avatar Sep 29 '21 19:09 char0n

I was satisfied with my PR, but I'm pretty sure if I read it now I'll found improvment to do. Can you let me few day to check that ?

ltruchot avatar Sep 30 '21 15:09 ltruchot

I was satisfied with my PR, but I'm pretty sure if I read it now I'll found improvment to do. Can you let me few day to check that ?

Sure, I'm sure it's a pristine work ;] I was more about if I do the code review (CR) and will have some suggestions or questions, if I should expect some answer, after this long time. But obviously I can ;] I'll get into CR.

Thank you for your work!

char0n avatar Sep 30 '21 17:09 char0n