search

async-library / future

The future of React Async and react-async-hook
12 stars 0 forks source link

Use cases, features and considerations #3

Open ghengeveld opened 5 years ago

ghengeveld commented 5 years ago

Let's collect and decide on the uses cases and features we want to support with Async Library, either directly or through a separate package. They are grouped, but in no particular order. Core would be @async-library/core, Integrations would be e.g. @async-library/react-hook. User provided would be through passing a prop/option. Plugins would be separate packages.

⚙️ Core

🧩 Integration

🔌 User provided / plugin

dai-shi commented 5 years ago

Here's the comparison of usages with hooks in three libs. They are all similar and slightly different.

immediate fetch

react-async

const { data, error, isPending } = useFetch(`https://swapi.co/api/people/${id}/`);

react-async-hook

// define this outside of render
const fetchFunc = async id => (await fetch(`https://swapi.co/api/people/${id}/`)).json();

const { result, error, loading } = useAsync(fetchFunc, [id]);

react-hooks-async

const { result, error, pending } = useFetch(`https://swapi.co/api/people/${id}/`);

fetch in callback

react-async

// define this outside of render
const fetchFunc = async id => (await fetch(`https://swapi.co/api/people/${id}/`)).json();

const { data, error, isPending, run } = useAsync({ deferFn: fetchFunc });
return <button onClick={() => run(1)}>click<button>;

react-async-hook

// define this outside of render
const fetchFunc = async id => (await fetch(`https://swapi.co/api/people/${id}/`)).json();

const { result, error, loading, execute } = useAsyncCallback(fetchFunc);
return <button onClick={() => execute(1)}>click<button>;

react-hooks-async

// define this outside of render
const fetchFunc = async ({ signal }, id) => (await fetch(`https://swapi.co/api/people/${id}/`, { signal })).json();

const { result, error, pending, start } = useAsyncTask(fetchFunc);
return <button onClick={() => start(1)}>click<button>;

custom fetch hook with abortability

react-async

const useFetchHero = () => useAsync({
  deferFn: useCallback(async ([id], _props, { signal }) => {
    return (await fetch(`https://swapi.co/api/people/${id}/`, { signal })).json();
  }, []),
});

react-async-hook


const useFetchHero = (id) => useAsyncAbortable(async (signal, id) => {
  return (await fetch(`https://swapi.co/api/people/${id}/`, { signal })).json();
}, [id]);
// It does not support the pattern with callback?

react-hooks-async

const useFetchHero = () => useAsyncTask(useCallback(async ({ signal }, id) => {
  return (await fetch(`https://swapi.co/api/people/${id}/`, { signal })).json();
}, []));
ghengeveld commented 5 years ago

Good to have this as a reference, thanks. I've been thinking to remove the distinction between promiseFn and deferFn and introduce a different way to handle scheduling/triggering, a bit like react-hooks-async.

slorber commented 5 years ago

thanks @dai-shi that's a great comparison

For the future it would be great to discuss what we have found great/bad in our own design decisions.

For me I like how my lib api better (particularly that the async fn and the dependency array can typecheck, or freedom given by passing a no-arg async function). Also think the ESLint plugin can be configured to check the dependencies (need to verify that, but there's a conf for custom hooks). Not totally fan of how I handle cancel signal (I'd rather not have a custom hook for this usecase).

React-async seems quite similar to me but less convenient and easy to typecheck.

For react-hooks-async I'm not totally sure to understand why the task abstraction is needed. Maybe you can explain better @dai-shi what's the advantages of using your lib that I may not be able to see?

ghengeveld commented 5 years ago

@slorber Could you elaborate on what you think is better about react-async-hook? Maybe some examples would help. Type safety is going to be important going forward, especially now that React Async is being refactored to TypeScript, so I totally agree with you there. React Async was not designed with TypeScript in mind, but I'm eager to change it so that it does handle type checking properly.

I'm also not really keen on the task abstraction in react-hooks-async, I think it doesn't really fit the mental model of most developers. Nevertheless the composability is very powerful so we may want to offer something like that as an alternative API, or preferably design the core API in such a way that it caters to both audiences.

The thing I've come to reconsider with React Async is the deferFn, particularly the fact that its API differs from promiseFn. The naming isn't great either. I think we should be talking about "async functions" rather than "promise functions", because we should embrace async/await as the preferred way to write them. Hence I'm looking to combine promiseFn and deferFn in a single fn option, and come up with a different mechanism to handle the execution (manual or automatic). I don't yet know what that will look like, ideas are welcome.

dai-shi commented 5 years ago

I had two motivations in developing react-hooks-async. One is abortability of promises, and the other is composability of async tasks. The main use case in my mind is useEffect, not callbacks which I think is relatively easy without a library (but then, users suggest that this library should also be usable for callbacks, so I modified.) As a side note, I guess I would try to design a new API once Suspense lands, so my aim is to make the lib valuable even after Suspense comes, that is the use case without caching (or with delayed fetching).

Abortability

A general interface for abortable promises is (a: AbortController) => Promise. This is not only for fetch, but for any async functions. In my lib, Axios and setTimeout are also wrapped with this interface. I wouldn't expect normal developers use this interface directly but library authors. So, my custom fetch hook with abortability comparison in the previous comment is rather hypothetical.

Composability

As I thought developing useAsync/useFetch naively is somewhat trivial, I wanted something unique in my lib. That is to replace redux-saga in some use cases. Of course, it doesn't replace it at all, but I mean some small use cases could be done much more simply without it. I also saw a requesting comment like that in Reddit. So, useAsyncCombine* hooks are crucial in react-hooks-async. (My experiment without composability is react-hooks-fetch.)

If we have the async task abstraction, we can combine them as we like. There could be more combining hooks than what I have now. Honestly, I'm not sure if this useEffect chaining (hope you get what it means?) is a good practice. There might be a better abstraction with Suspense.

Apart from the technical challenge in react-hooks-async, the use cases I want to cover in this Async Library project is something like redux-saga (I'd admit I have zero experience with it though). Abortability is also my concern, but it should be taken into account already hopefully. (or not yet for non-fetch use cases?)

One of the questions I have is, given the domain of "async" is larger than that of "data fetching", what would be async use cases other than data fetching? Or is it equal?

slorber commented 5 years ago

What I like in my API is:

1) I don't need promiseFn/deferFn which imho is a bit confusing and not needed.

2) In my lib, naming is not an issue, because it's the first arg the user does not need to use a named attrbute to provide the function (not sure it's the best but

3) I can catch type errors like useAsync(fetchByStringId,[1]) because 1 is not a string id, and thing like that. TS params are not any[], also true on the returned manual execution function, you can't call "execute([1])" on it.

4) still have the freedom to always use a 0-arg async fn: useAsync(() => fetchByStringId(id),[id])

5) I think the shape of the hook is suitable to be verified by the hooks eslint rule plugin to check for dependencies

What I don't like is to have a custom hook for handling abortability, and not totally fan of my state shape or returned api. This is not bad but probably could be better.

thanks @dai-shi

About the composability without loosing the cancellability, couldn't we allow users to plug this from the outside?

I suspect there's a common misconception about abort controller, for example most people think code like this is safe:

const result = await fetch(url,{signal})
await delay(1000);
return result.json();

If the cancel signal is triggered during the delay, people probably assume the async process will be canceled while it actually won't at all.

I think your library is interesting as it handle cancellation without letting the user falling in such trap (I guess?), but I wonder if it couldn't be simpler and if the task abstraction is needed for that (maybe something like https://www.npmjs.com/package/p-cancelable or similar)

Also, like @ghengeveld seems to think, couldn't we implement this task abstraction as a plugin, and keep the core more simple (just handling a single async fn)?

Not totally sure how you compare your project to Redux saga. I know sagas quite well and actually helped design and spread the library, but for me it's really 2 separate things.

One of the questions I have is, given the domain of "async" is larger than that of "data fetching", what would be async use cases other than data fetching? Or is it equal?

I use this regularly on RN apps to read about async things not related to fetch. For example, checking if current app is granted permission to access user camera is an async function. This kind of lib makes sense in a lot of cases that are totally unrelated to fetching data for me.

dai-shi commented 5 years ago

couldn't we allow users to plug this from the outside?

I think I'm doing this... It allows a single async function without using useAsyncCombine*, which are plugin hooks.

task abstraction

I would like to confirm if we are on the same stage.

https://github.com/dai-shi/react-hooks-async/blob/a08a7b5e01acdf9ecea91d4484bacdf0daae3fd6/src/index.d.ts#L1-L21

https://github.com/slorber/react-async-hook/blob/bfda97f4e3213ce756e7092f97ad3fde6eb4b497/src/index.ts#L213-L223

The fundamental difference I see for composability is only abort and the fact the hook doesn't execute an async func immediately, which is in your case useAsyncCallback.

keep the core more simple (just handling a single async fn)?

So, only if we added abort, this should holds true.

I might be misunderstanding something, so feel free to ask/correct.

it's really 2 separate things.

I wouldn't disagree. Let me phrase it differently: In some cases, people overuse redux-saga or redux-observable for their use cases which only require cancellation (so, no saga power). Such use cases could be covered by a simpler hook-based solution.

checking if current app is granted permission to access user camera

Oh, I see. Thanks for the example.

ghengeveld commented 5 years ago

I think we should at least handle promise cancellation in order to prevent race conditions. AbortController is optional, as not every browser supports it. Nevertheless I think this should be built-in because otherwise it will be forgotten by most developers, which is a shame. I would be okay with leveraging the AbortController API to handle cancellation, as an alternative to checking if the promise reference is still actual or checking against a counter. In that case we have to polyfill the AbortController to handle our usage.

Working with Promises instead of fetch has always been the basis of React Async, because it allows for composability simply by composing promises (e.g. with Promise.all). It's not as flexible as a task-based interface, but it saves us from having to build and maintain complex task scheduling logic. In practice I think not many people will need it, especially when you use Suspense to model the async state tree (and thus interdependencies between async operations). @dai-shi perhaps you can elaborate on the use cases you can cover with the task based approach? I always prefer to speak in terms of real-life use cases when discussing technical abstractions.

@slorber I think those are very valid points which we have to keep in mind when designing a consolidated API. It should be very hard to shoot yourself in the foot, and typechecking is a good way to ensure that. I like your point about the eslint rule, it would be really nice if we could leverage that.

On a final note: one of my goals is for the async process to become inspectable and controllable through DevTools (as a Chrome extension, eventually). I think this will give developers much better tooling to deal with HTTP requests and other async operations as compared to the Network tab. For example we can offer pausing, delaying and replaying for each individual operation, rather than throttling the entire network or having to refresh the page to replay a request. I consider this part of the core library, so we can potentially release @async-library/fetch as an alternative to native fetch which would leverage this DevTools integration.

dai-shi commented 5 years ago

For example we can offer pausing, delaying and replaying for each individual operation

That's very neat!

simply by composing promises (e.g. with Promise.all).

Yeah, that should be more JS centric. My only concern is if people can properly handle cancellation in composed promises.

In practice I think not many people will need it

Agreed.

especially when you use Suspense

In the case of Suspense, we need to execute async func in render, correct? That mental model is totally different from the current implementation.

the use cases you can cover with the task based approach

Only the realistic use case I have is the typeahead example. https://codesandbox.io/s/github/dai-shi/react-hooks-async/tree/master/examples/04_typeahead

The other use case I see from the user feedback is something like this:

  1. login as a user
  2. get the user information

We could obviously implement this as a single async function. If we split it into two async tasks, we can show successful login state before starting 2. I think this is also possible without the task based approach.

dai-shi commented 5 years ago

Quoting the previous comment:

I'm also not really keen on the task abstraction in react-hooks-async, I think it doesn't really fit the mental model of most developers.

I think I'm proposing a new paradigm in the async + hooks world. This may or may not be the mainstream. So, your comment is totally valid and maybe true.

By the way, this is probably something related with @dagda1 's comment in https://github.com/slorber/react-async-hook/pull/15#issuecomment-544258656.

I got a lot of feedback on this post and a lot of people are saying that you would not need the execute function that is returned from useAsync. Most are saying you can do this with state changes but I disagree. Others say that calling a callback in a hook is not what hooks are about as they are lifecycle only.

I don't disagree with him. Execute in useEffect and execute in callback are different. I do understand why many people say so, and probably the pattern is what I call useEffect chaining.

ghengeveld commented 5 years ago

CC @phryneas @cristovao-trevisan

cristovao-trevisan commented 5 years ago

Hello, I have a few more points to consider:

Type inference (Typescript)

This is a must in my opinion. When using javascript it is a pain to use jsdoc to declare types. The problem is that we can't put everything into an Object or Map because we lose the types doing this. I suggest an API that returns an object (or class instance) which is to be exported and used by the user, much like redux actions. For example:

import { createResource } from 'async-library'

export const userInfoResource = createResource(...)
export const userEventsResource = createResource(...)

Performance (Tree shaking, extensibility and bundle split)

JS code is costly for initial render and performance, so we should ship as little code as possible. Two ways of doing this:

I suggest we do both. The example above is a good way of doing this, because (unlike redux which usually declares the whole store at the index) the code will only be bundled into the chunks that need it. We can also declare middleware for things like caching, pooling and retrying. We can also use helpers to build fn, like useFetch, useAbortableFetch, useGraphQL or usePaginatedFetch, this way the user will get only the code needed for the api he is using

Separation of API's

I see most comments are thinking of a hook-like solution. I've working with svelte the past few months and I think it's solutions are very elegant. The way stores work (check the docs and this example) are very simple and effective. I think we should consider having 3 (2 + 1) separated API's (I will name any endpoint as a resource):

Only the Framework API here should use the hooks logic (and not for all frameworks)

Example

I wrote a gist as an example for this, it has the API declaration (like any .d.ts) and an example use case

Missing

The only thing I think I'm missing is the subscription (websocket), did not think about that yet

cristovao-trevisan commented 5 years ago

I also like the way rest-hooks work. The only thing I disagree is that the Resource class comes with everything (get, update, delete, lists, ...) already bundled, so it loses on performance because of that (I known 9kb gzipped might look lightweight, but consider that a page built with svelte may be only 3kb gzipped).

ghengeveld commented 5 years ago

I came across resourcerer which has some pretty interesting ideas. Particularly critical vs. noncritical resources, the way it handles dependencies between requests and built-in support for prefetching. Not things we want to have in the core but good use cases to consider.

phryneas commented 5 years ago

Also, React Suspense is going into preview right now. I hope to play around with it in the near future, maybe that gives us some additional ideas: https://reactjs.org/docs/concurrent-mode-suspense.html

ghengeveld commented 5 years ago

Are you at React Conf? Keep us in the loop on relevant updates 👍

ghengeveld commented 5 years ago

Okay so I spent some hours building an initial version of the core state machine: https://codesandbox.io/s/async-library-state-machine-084sw (use the Tests tab)

Let me know what y'all think. So far it does:

It doesn't actually deal with async functions yet. This is just pure synchronous vanilla JS.

phryneas commented 5 years ago

Are you at React Conf? Keep us in the loop on relevant updates +1

Nah, I'd love to be, but I just was at IJS in Munich the last few days. Plugged React-Async and Async-Library in my Talk though ;)

But my twitter is full of ReactConf right now, I guess it's the next best thing :D

cristovao-trevisan commented 5 years ago

I also spent some hours experimenting on the idea I described above for the core API. Check out the repo:

https://github.com/cristovao-trevisan/async-library-experimentation

It has just a few tests, so it may be hard to understand for now. The types got a little more complicated than I intended to, so I will explain here:

Usage example:

  const middleware = {
    resolved: jest.fn((options) => options.state),
    willLoad: jest.fn(),
    willRequest: jest.fn(),
  }
  const userInfoResource = createResource({ fn: getUser }, [() => middleware])
  userInfoResource.subscribe(state =>{
    console.log('got a new state: ', state)
  })

  userInfoResource.run({ id: 1 }) // run

There is also an example middleware for caching (I will translate to js for those not familiar with ts):

export const inMemoryCache = () => (/* IResource param */) => {
  const cache = {}
  return {
    cache: (args, next) => {
      const value = cache[args.options.hash]
      if (value === undefined) return next(args)
      return next({
        ...args,
        state: value,
      })
    },
    // save value to cache
    resolved ({ options: { hash, state } }) => {
      cache[hash] = state.data
    },
  }
}
cristovao-trevisan commented 5 years ago

Okay so I spent some hours building an initial version of the core state machine: https://codesandbox.io/s/async-library-state-machine-084sw (use the Tests tab)

Let me know what y'all think. So far it does:

* Handle race conditions

* Metadata

* Optimistic updates

It doesn't actually deal with async functions yet. This is just pure synchronous vanilla JS.

Nice. Really like how the metadata (and thus the state) can be extended. If you want I can fit your state machine into my middleware system (just got to find a way to do thisin typescript)

One question: what do you mean by "race conditions"? Since JS is single threaded, I'm probably misunderstanding something

phryneas commented 5 years ago

I like both those approaches (and certainly am thinking of a few ideas of my own), but while they work great with a given mindset, I'm not 100% sure if they will work for suspense if that should be a major use case for async-library.

From what I've seen so far, the idea of suspense seems to be not to restart actions, but to start completely new actions without any context of possible old actions (although cancellation certainly plays a valid role there). Race conditions don't seem to play any role because state is set once at the start, and never at the finish of an asynchronous action.

So that seems to make it very different from out mindset at the current time. I'll try to wrap my head around those news idea this weekend, and I'd suggest before we dig deeper, that you also try to play with those ideas (see link above) - this might be a complete gamechanger and we might need to do some serious rethinking to get both mindsets into one model.

cristovao-trevisan commented 5 years ago

Hi @phryneas , I think it should work with Suspense. We can even implement Suspense for svelte and vue. We only have to provide a useSuspense(userInfoResource) hook, that will return the suspense object instead of a traditional { pending, data, error } object. Should look like this:

const useSuspense = (resource) => ({
  read() {
    // returns a promise that uses the resource api to fetch data
  }
})

All we have to do is to add to the resource API so that this is possible

ghengeveld commented 5 years ago

The way Suspense works isn't much more than throwing a promise, alongside having a way to synchronously return data (from a cache) if it's already loaded. React Async supports this already, simply by setting the suspense flag. That will make it throw the promise if it's still pending. Suspense does come with a bunch of extras such as the SuspenseList and hooks, so most of the challenge is in finding the right patterns that these tools enable.

I think our main challenge is to offer a compelling API that works fluently with Suspense. We can look at Relay for inspiration. In fact it may be a good idea to just copy it for the most part. We should all get acquainted with the various things Suspense has to offer and concurrent UI patterns.

phryneas commented 5 years ago

Yeah, I have no doubt that whatever we'll think of, in the end it will work with concurrent mode/suspense.

But just reading half of the suspense docs yesterday, I have so many new ideas on how this will actually be used in the end and so many patterns I want to consider, that I just would try to get acquainted with those patterns before designing the core that we'll stick with for a long time. Just like the idea of saving a resource with a promise-returning function instead of saving the return value after the promise resolves eliminates all possible race conditions without any extra cancellation logic - that's just a new pattern for me. And I believe there's much more inspiration there somehow - maybe even stuff that we want to bring from suspense to non-suspense stuff.

dai-shi commented 5 years ago

Please check out my first attempt: https://github.com/dai-shi/react-hooks-fetch

ghengeveld commented 5 years ago

That's interesting! Why a Proxy?

Personally I'm not a fan of the Resource abstraction by the way. I think in terms of data and actions/events, not resources and CRUD.

dai-shi commented 5 years ago

To avoid data() calling style. But I’m going to change proxies to getters.

Not sure if I understand the data and action abstraction, but I was hoping resources sound more declarative.

ghengeveld commented 5 years ago

Resources have a very REST sound to it, which is not good because REST APIs are a lie. Most of them are not RESTful at all, and the ones that are are bloated and unwieldy. GraphQL has a way better abstraction with Queries and Mutations, but essentially it's just RPC. My intention is to support more than just HTTP data fetching (think websockets, workers, native APIs), in which case RPC is a better model than REST. If we want something that's familiar to webdevs, we should go with query/mutation/subscription.

dai-shi commented 5 years ago

Until I heard that, I hadn’t thought about REST. That’s understandable. My intention is not to relate with REST. At the same time, I’m worried if query/result mental model fits with Suspense...

(Note my comments are all about React. I know this issue is more than that though.)

dai-shi commented 5 years ago

I've got another feedback, so trying to rename APIs... https://github.com/dai-shi/react-hooks-fetch What I've got so far: createAsync, createStatic,,, and useAsync. OK, I know it's confusing... Naming is hard.

(edit) Updated the API, which should be much nicer.

import { createFetch, useFetch } from 'react-hooks-fetch';

const fetchFunc = async (userId: string) => (await fetch(`https://reqres.in/api/users/${userId}?delay=3`)).json();
const initialId = '1';
const initialResult = createFetch<
  { data: { first_name: string } },
  string
>(fetchFunc, initialId);

const Item: React.FC = () => {
  const [id, setId] = useState(initialId);
  const result = useFetch(initialResult);
  return (
    <div>
      User ID: <input value={id} onChange={e => setId(e.target.value)} />
      <DisplayData id={id} result={result} />
    </div>
  );
};

(edit2) The above is obsolete. Check out the repo for the latest status.

slorber commented 5 years ago

Hey all

Read the docs about ConcurrentMode etc and asked some stuff to Dan/Sebastian. What I understand from the answers if that if we want to take advantage of the transition feature, and avoid the request waterfall, current approaches/API are probably wrong.

https://twitter.com/sebmarkbage/status/1188832461351817216

As far as I understand the answers, deriving a resource (or throwing a promise) at render time will make Suspense work, but not transitions. For transitions to work we need to store a resource outside of React itself.

rostero1 commented 5 years ago

I had to look up the request waterfall. It's when you have a fetch in Parent and a fetch in Child, but Parent only renders Child when Parent's fetch has succeed. As a result, Child will have to wait to fetch, even when Child's fetch does not depend on anything from Parent.

ghengeveld commented 5 years ago

This just landed: https://twitter.com/zeithq/status/1188911002626097157 The way it does dependent fetching is particularly interesting: https://swr.now.sh/#dependent-fetching

ghengeveld commented 5 years ago

I spent a little more time on the core reducer and dispatcher. I pushed the code here: https://github.com/async-library/future/tree/core

I followed the Flux pattern to define the dispatcher. What's great about that is it doesn't tie you in to a specific store like Redux does, and it's very lightweight and flexible. Check out the integration test for an idea of what integration with 3rd party libs will look like.

Current features:

ghengeveld commented 5 years ago

Another interesting approach at data fetching in React: https://github.com/jamesknelson/react-zen

ghengeveld commented 5 years ago

I've updated the prototype again, adding support for subscriptions and setting data based on old data. I'm not sure yet about the API for subscriptions. Right now it's reusing the existing fn prop, but calls it with different arguments. Overloading the deferFn API wasn't a very good design choice in React Async so I think we may want to consider an alternative. Any suggestions?

For reference, the current subscription API looks like this:

// Normally `fn` is invoked with params and state.
// For subscriptions it's invoked with fulfill and reject.
const fn = (fulfill, reject) => {
  // do your subscription thing, calling fulfill/reject multiple times
}

// `createApp` is just a local name, not a core API.
// This is what would be `useAsync` in a React integration.
const app = createApp({ fn }) 

// Runs `fn` (once), passing the fulfill and reject callbacks (bound action creators)
app.subscribe()

One thing to consider is if we want to support mutations (deferFn) alongside subscriptions. That would be tricky in the current setup because fn is overloaded.

Setup and teardown of the subscription would be done through lifecycle callbacks (onInit / onDestroy), which aren't implemented yet.

What's left to do:

cristovao-trevisan commented 5 years ago

@ghengeveld , your code is not "type safe" because of the following function:

export const compose = (...reducers) => (state, action) =>
  reducers.reduce((state, reducer) => reducer(state, action), state)

reducers is an array with multiple types, which you can't do with generics (template). You can still use any and cast the result type to include the types added by each reducer in a single interface, but I don't think it's worth it. Maybe think of a more "typescript way" of composing reducers.

Anyway, I think we should have a few more things in the core, and think a bit more about the API instead of the implementation. I have a few questions over your code first:

Sorry for being so critical, just trying to get the best result out of this discussion. Let me know if I was too rough :grimacing:

ghengeveld commented 5 years ago

@ghengeveld , your code is not "type safe" because of the following function:

export const compose = (...reducers) => (state, action) =>
  reducers.reduce((state, reducer) => reducer(state, action), state)

reducers is an array with multiple types, which you can't do with generics (template). You can still use any and cast the result type to include the types added by each reducer in a single interface, but I don't think it's worth it. Maybe think of a more "typescript way" of composing reducers.

Thanks for pointing that out. My TS experience is very limited, so this isn't immediately apparent to me yet. I see why this is a problem now. Do you have a suggestion for making the reducer (and dispatcher for that matter) composable? One way I can think of is through thunks which basically make reducer composition unnecessary, instead you would probably compose/decorate the action creators. In fact I want to move the special handling of "start" to the start action creator using a thunk.

Anyway, I think we should have a few more things in the core, and think a bit more about the API instead of the implementation.

Definitely! This is the time to iterate on it before settling on something for the foreseeable future.

  • I see the callbacks are global (src/dispatcher.js), are you thinking of a single state holding all data for all calls?

The whole thing is bring-your-own-state by design. So you could put all state in one place, or keep it in local component state. Whatever works for you. However I did consider the need for a way to keep track of ALL instances of Async Library in one central place: the DevTools. The way I plan to do that is have the DevTools listen to all dispatched actions, and keep its own copy of state by running the actions through the reducer another time. It has to hook into the dispatcher if it wants to intercept actions before they are sent to the reducer (e.g. to delay or rerun it).

  • src/reducer.js exports the composition of the reducers you created. Should this be an api so the user can extend the state without modifying the core? If not, what is the advantage of using the reducer approach instead of a simple function for each action?

Yes, I designed this to be extendable by whoever develops the integration (so not the end user, typically). For example AbortController might not make sense for a Node.js or React Native integration (not sure though). I've not designed the API for it yet. I'm also not sure if this is the best way to do it.

Sorry for being so critical, just trying to get the best result out of this discussion. Let me know if I was too rough 😬

Not at all! Please be critical of my work, I don't have all the answers and I probably made some silly choices in some places.

ghengeveld commented 5 years ago

Another one just landed: https://twitter.com/tannerlinsley/status/1191472293643350021?s=20

Interesting detail is that it uses a key to determine same-ness: https://twitter.com/tannerlinsley/status/1191591012981805059?s=20