bodar / totallylazy.js

TotallyLazy.js
Apache License 2.0
8 stars 5 forks source link

Type safe parsing #27

Closed philwhiteuk closed 1 year ago

danielbodart commented 1 year ago

If you call the single parse method with undefined it should throw and error not return undefined as that the whole point, it throws.

Changing the default parsers to auto wrap feels a bit weird to me, if I'm honest. Maybe we should chat so I can understand you perspective. Also it feels more like a convenience thing as right now you have actually made something that is the opersite of type safe: it is returning undefined as T which is very bad

philwhiteuk commented 1 year ago

it is returning undefined as T which is very bad

I agree with this. This was the previous behaviour though so the issue for us is that upgrading requires us to now locate all usages and guard against it.

In the majority of cases we use parseAll already anyway. Perhaps a better solution is to drop parse from the interface?

Changing the default parsers to auto wrap feels a bit weird to me, if I'm honest. Maybe we should chat so I can understand you perspective.

Apart from the ^^ above reason ^^ the types of inputs we are parsing are often indeterminate/prone to change and so we rely heavily on runtime type-checking to ensure that we are definitely putting in and getting out what we expected. As mentioned above, in a lot of cases we lean in to array return types as that provides a cheap, convenient way to handle edge-cases and so our code has adapted to dealing with undefined rather than handling errors.

philwhiteuk commented 1 year ago

Closed in favour of https://github.com/bodar/totallylazy.js/pull/30