search

vercel / arg

Simple argument parsing
https://npmjs.com/arg
MIT License
1.23k stars 54 forks source link

Refactor `type` and `isArray` handling internally #34

Closed blakeembrey closed 5 years ago

blakeembrey commented 5 years ago

Splitting the internal handler refactor from https://github.com/zeit/arg/pull/33 into new PR since I'm not sure it'll land with the discussion in https://github.com/zeit/arg/issues/30. This change made it easier to re-use type in more than one place in the codebase (currently all the logic for array handling is within the "separated args" loop).

Qix- commented 5 years ago

@blakeembrey You're right about how arg.of functions. I was writing up an example as to why we should pass the full array as the previous value and realized you're 100% right - the previous value from the end of the array makes absolute and complete sense. You're living in 3019, I swear.

My only two opinions at this point:

Feel free to prove me wrong again.. :)

blakeembrey commented 5 years ago

@Qix- I can definitely make it private still, no major gripes - it's a very small block of code 👍

For the generic type, it's nicer to use generics in the signature since you know the what T will always be. E.g. <T> (value: string, name: string, prev?: T) => T only means that prev is the same type as returned (| undefined), and the return type can still be any if you really wanted it. It would just make it a bit harder to write type safe code that way, since we know that prev?: T always (as it comes from the previous invocation of the function).

Qix- commented 5 years ago

as it comes from the previous invocation of the function

But that also means that we must return the same type as the parameter, correct? That's the part I have doubts about.

blakeembrey commented 5 years ago

@Qix- Yes, but it's fine for T = any or T = string | number if the use-case you're thinking of is changing types each execution.

Qix- commented 5 years ago

Hmm so I guess the idea is that if I'm writing a typescript function as a handler and I return a new type, I would have declared that type as a potential return type...

Okay, no problem. Keep it as-is :) I'll give it one more look through.

Qix- commented 5 years ago

Last nit then it looks good to me :)

Qix- commented 5 years ago

Looks great. Thanks @blakeembrey :)

Qix- commented 5 years ago

Released as 4.1.0.