[WIP] New API proposal

[busypeoples]

This PR is not meant to be merged!
Should only be used for validation of ideas at first.

Discussion and possible implementations for #20 and #21.

[CharlesMangwa]

TL;DR;

1. Thank you for your contributions @busypeoples 💖
2. Do you want to add anything to this PR before I start playing with it?
3. I think we might need to rename a thingor two so that non-FP folks won't be lost (especially thinking about Catamorphism here 😛).
4. React's Suspense will soon allow the library to handle a lot of use cases we use to deal with. How could we make React Data Fetching play nicely with Suspense?
5. Let's try to iterate over the new API and implement it so that we can move on and migrate to the newest version of React (with new lifecycles and context API). If we upgrade React first, I'm afraid it would make merging this PR more cumbersome.

---

Hi @busypeoples! First of all: thank you for your great contribution! It's taking us a bit longer than expected, but I'm convinced we're getting somewhere with all this! Since we've discussed about this PR, a lot of things have changed, for the better though! I'm now more and more convinced that this new approach could really help us move forward and drastically reduce our fair share of the work (inversion of control ftw!).

I took a look at your PR and I really love what I saw. The API we'd exposed is simplified and gives more power to userland, that's great! Do you think you want to add something or I can already start playing a bit with it? (I mean: I'll will, for sure, just asking to be polite here 😝)

Right now, my only concerns are maybe about a few namings here and there. For instance, I'm not really an expert in FP tbh, so I didn't get what cata could possibly refer to right away (Data.js#L22_L25):

const create = (type : string, data: any) : DataHandler => {
  return {
    type,
    data,
+   cata: definitions => {
+     const fn = definitions[type] || noop
+     return fn(data)
+   },
  }
}

But after some research, I've stumbled upon the principle of Catamorphism 😅That could definitely me be ignorant though, but I think if we can try to use names/concepts that talk more to JS folks.

---

The other subject I had in mind was how this library would position itself with the upcoming React's Suspense API? Basically React will be able to take care of fetching a resource and caching its result (whether it's an API call or an image or a component, etc) like so:

// App.js

...
<React.Placeholder delayMs={1500} fallback={<Spinner size="large" />}>
    <UsersList />
</React.Placeholder> 
...

// UsersList.js

/**
 * cache & createResource will be provided by Suspense
 * while fetchUsersFromAPI is a regular fetch/XMLHTTPRequest
 **/
...
const UsersListResource = createResource(fetchUsersFromAPI)

const UsersLists = () => (
  <React.Fragment>
    <h1>Users</h1>
    <ul>
      {UsersListResource.read(cache).map(user => (
        <UserCard key={user.id} user={user} />
      ))}
    </ul>
  </React.Fragment>
)
...

So as we can see here, the library will have a built-in way to smartly handle requesting external resources. In our use case, I still think we'd have the advantage with the new API to be able to handle all the states of our request (from the initialization to the success/error, plus the ability to run on demand). But I'm just putting it here so that we can think about a nice way to make React Data Fetching work hand in hand with Suspense.

---

Let me know about your current state with this PR whenever you'll have time so that we can merge it and move forward. I'd like to do it in order to migrate to the latest version of React (which would mean new lifecycle methods, new context API etc). I thought about doing it now, but it would just make it harder for us to merge your work :) While waiting for that, I'll start to play a little bit with this PR, and more importantly: thanks again!

[busypeoples]

Thanks for the great feedback @CharlesMangwa
This PR is just an initial approach. We should definitely simplify the API for user land. Maybe we can simplify and also rename cata to something that more people can straight away understand. But we can also update to the latest React version and figure out how this might work with suspense. Maybe it's even better if we do this all at once, what do you think? Maybe this will simplify things even more.

[CharlesMangwa]

I'm totally OK with doing all this at once @busypeoples. I've already upgraded to React 16.4.2 and modified the lifecycles accordingly (fd46636). Nothing remains but switching <FetchProvider /> & ` to the new context API.

About the API itself now, I found it pretty straightforward tbh. Maybe I might be biased for using Reason and being used to variants & pattern matching, but I found this example quite approachable to JS folks:

const fakeFetch = () => {
  return new Promise((res, rej) => {
    setTimeout(() => {
      res({
        name: "User A"
      })
    }, 1000)
  })
}

...


<Fetch
  fetch={fakeFetch}
  subscribe={{
    onWillMount: () => console.log("Component will mount"),
    onDidMount: () => console.log("Component has mounted"),
    onSuccess: (data) => console.log("Data has been updated!", data)
  }}
  options={{ lazy: true }}
>
  {(props) => (
      <div>
        {props.data.cata({
          SUCCESS: result => (
            <div>
              <h3>Username</h3>
              <p>{result.name}</p>
            </div>
          ),
          FAILURE: error => <div>Something went wrong!</div>,
          LOADING: () => <div>Loading...</div>,
          INIT: () => <div>Nothing Loaded Yet.</div>
        })}
        <button onClick={props.run}>Fetch now</button>
    </div>
  )}
</Fetch>

But let's try to gather some opinions about this and see if Suspense could have an impact on this API.

[CharlesMangwa]

Quick update to this PR to confirm that React's newest version, lifecycles & context API have been implemented (fd46636 & b5b3a59).

[CharlesMangwa]

Quick update on this PR. I discussed about this proposal with a colleague a few weeks ago to gather his opinions & ideas and lots of interesting things came out of this conversation. I'll try to report all the notes I took, in no particular order, which is the result of our discussion:

1 - Naming:

• "Maybe you should rename the lazy option [EN: a boolean] into fetchOnMount, makes more sense"
• "Same with subscribe: how about events instead?"
• "Indeed, cata() might no be really straightforward. Something like states() could make more sense maybe"
• "Also the fetch prop could be rename request to avoid the redundancy"

2 - Changes:

• " Instead of data and run, it'd rather have the component render props exposing this object: { data, run, states }. By doing this I could have access to just the result if that's all I want, or the different states if I need them. I don't want to have to"

3 - Ideas:

• "Nothing really important but you should also cover the case where people write success instead of SUCCESS. It's all a matter of preferences of course, toUpperCase/toLowerCase should do the trick 😄 "
• "What if we could have a FetchManager class which would allow me to manage every single <Fetch /> instances in my app, from wherever I want? I could be able to see at a given time, all the calls running in my app, cherry-pick the ones I'd like to cancel or do something else. That'd be cool!"

It'd be interesting to see what we would like to share here. As an initial idea, FetchManager could share an object with each <Fetch />'s data, run(), cancel(), props maybe? (implementation idea)

• "It would like to be able to cancel a request!"

Already implemented in 5f1b146 😁

---

That's all I wrote down during this conversation and I think every single point here is really interesting! So let's continue this discussion, I'm convinced we'll end up with a very interesting API.

[EDIT] : Forgot to tag you in the 1st place but thank you @iremlopsum for your help 🙌

[busypeoples]

@CharlesMangwa Excellent feedback! I will have time in the coming days to work on this. Let's coordinate in the coming days.

[CharlesMangwa]

@busypeoples Awesome! Let me know how I can help you when you'll be working on this!

[busypeoples]

@CharlesMangwa Will try to rework this tomorrow and then maybe we can do another feedback round based on these changes.

[CharlesMangwa]

Hey @busypeoples! Quick update about your Add a basic example commit to let you know that CRA 2.0 is finally out! This might remove the need of some workarounds you used there I guess: https://reactjs.org/blog/2018/10/01/create-react-app-v2.html

[busypeoples]

@CharlesMangwa excellent! Still didn't have enough time, to straighten things out. This will happen this week. Definitely interested in getting this off the ground!

[CharlesMangwa]

Good to read that @busypeoples! Can't wait to see where we can get with this. On my side, I'll start working on an initial proposal of the FetchManager API, will commit it under the /next branch. I'll let you know so that you can have a look at it and tell me how you feel about it!

[CharlesMangwa]

Just committed my 1st approach of FetchManager @busypeoples: e4672f3 ! Whenever you'll have time your feedback is more than welcome!