map breaks RemoteData types

[mksmtn]

Hi, thanks for the package, I use it a lot at in my projects. I have a bug report.
map function from the package working only on Success values makes typing incorrect. And I've spent hours to find the root reason for my code giving me a value of a type in runtime that I don't expect.
Typically, Failure and InProgress have the last successfully fetched value, so they're of same type, and it's even built into the RemoteData type. In the current implementation, however, we can use map and change type of a Success value, but InProgress and Failure values stay the same, and it creates a mismatch between compile time types and run time types.

const secondsInt = inProgress(5);  // RemoteData<number>
const secondsStr = map(s => `${s} secs`, secondsInt);  // { tag: 'inProgress', value: 5 }, but it's type is RemoteData<string>

We can't simply change the current implementation to !isNotAsked(rd) && rd.value !== undefined ? fn(rd.value) : rd, because the fn won't be called with undefined even if it's an expected Success value. We can't make fn argument type (a: A | undefined) => B either, as we will force our clients to always map undefined to a B value, thus creating a value for inProgress even when inProgress was called without arguments. I guess the only sound solution, however sad it would be, is to abandon inProgress and Failure values altogether and force our clients to store the latest successful value themself.

[joanllenas]

Hi @mksmtn ,
Well spotted.

I don't use the InProgress or Failure value. That's probably why I have yet to notice this issue myself.
I like the idea. The change would be quite extensive, though.

1. The new RemoteData type would look like this:

export type RemoteData<T, E = string> =
  | NotAsked
  | InProgress
  | Failure<E>
  | Success<T>;

2. Changing the RemoteData type would also affect many parts of the other APIs, such as:

• Removing the value param. on the inProgress() and failure() constructor functions:
• Removing the value param. on the fold() function.
• ( ironically, map would remain intact )
• and other minor internal typing stuff

3. Perhaps, the most drastic change would be on the side of the pipe:

• Removing the hasValue.
• Removing the inProgressValue.
• Removing the remoteDataValue.
• Removing the remoteDataValue.
• Removing the failureValue.

All those pipes wouldn't be needed anymore.

[mksmtn]

@joanllenas will you implement those changes yourself? If you don't have time, probably I will be able to make a PR in June

[joanllenas]

@mksmtn I decided to explore the idea: #54
Let me know your thoughts.