Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Feature: 'nullToUndefined' decoder type #37

Open
marcbinz opened this issue Feb 23, 2019 · 3 comments
Open

Feature: 'nullToUndefined' decoder type #37

marcbinz opened this issue Feb 23, 2019 · 3 comments

Comments

@marcbinz
Copy link

Many APIs provide missing data fields with a value of null (see issue #29).

As a consumer of such an API using TypeScript and this very library it is currently not possible to decode such values to undefined.

Personally, I prefer to use undefined instead of null for my TypeScript models and I try to avoid null wherever possible. While this might be just my own preference, I believe it would make sense to add a standard decoder to this library that will automatically transform a given null value to undefined.

Here is snippet that does exactly what I need:
const nullToUndefined = <T>(decoder: Decoder<T>): Decoder<T | undefined> => JTV.union(decoder, JTV.constant(null)).map((val) => val === null ? undefined : val);

I am not sure if this is the best solution to the given problem, but it works.
I am happy to file a PR if that is desired 🙂

@mulias
Copy link
Contributor

mulias commented Feb 25, 2019

Thanks for the thorough explanation.

What if instead of adding more deciders, we opened up the functionality of optional a bit to cover your use case? Like you've pointed out, we can signify that data is missing in a JSON object by giving the field a value of null, or instead by omitting the field entirely. My thought is that in most cases when we consume JSON into typescript we don't care which of those two representations was used. All we really care about is how we're representing the missing data in typescript.

With that in mind, what do you think about having both an optional and nullable decoder which both succeed when the data is null or omitted?

Basically we consolidate this

                                 Decoder type

                     T | undefined      T | null
J                  +------------------+------------------+
S   T | undefined  |  optional        | undefinedToNull  |
O                  |------------------|------------------|
N   T | null       |  nullToUndefined | nullable         |
                   +------------------+------------------+

into this

                                 Decoder type

J                          T | undefined      T | null
S                         +------------------+------------------+
O   T | undefined | null  |  optional        | nullable         |
N                         +------------------+------------------+

If an API were to use omission and null to mean different things for a single piece of data then the user would need to define a custom decoder. I think that's an acceptable trade off.

What do you think?

@marcbinz
Copy link
Author

Sounds like an optimal solution to me 🙂
It provides exactly the functionality intended and is very straightforward.
It actually matches with my first idea of what optional means in the context for this library, so 👍

@katanacrimson
Copy link

katanacrimson commented Mar 28, 2019

@mulias To provide a touch of context for an API to leverage omission and null differently, consider the scenario of a PATCH. When omitted, it is reasonable to consider that no operation is necessary; when null, it is reasonable to consider it a removal of whatever data is stored. This isn't a new pattern either; it's directly analogous to the implementation of UPDATE queries in SQL, wherein omission is a no-op and null is the removal of data in the field(s) of the rows identified for update.

I would argue that what was proposed should not be considered for default behavior; if the goal of the library is to perform type validation, it should attempt to be as precise in implementation as possible by default and the methods to do so should be the most discoverable.

I won't say that providing a prefabricated method of coercing null to undefined should not be done, but it really should not override the behavior of optional or nullable (were it to be introduced).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants