Aside of `promiseFn`, also provide just a `promise` property, to be able to run with existing promise

[slawomir-brzezinski-at-clarksons]

Hi. Thanks for this good work!

TL/DR: the requested feature would allow to simplify this

{searchResultPromise && (
          <Async promiseFn={() => searchResultPromise}>
            <Async.Loading>{<ProgressBar />}</Async.Loading>
            <Async.Resolved>
              {searchResult => (<ul>...</ul>)}
            </Async.Resolved>
          </Async>
        )}

Into this:

<Async promise={searchResultPromise}>
  <Async.Loading>{<ProgressBar />}</Async.Loading>
  <Async.Resolved>
    {searchResult => (<ul>...</ul>)}
  </Async.Resolved>
</Async>

Rationale:
Using the promise-based UI patterns, a realm of a module like this one, I find myself quite often in need of having the code for controlling when to start the promise (the logic that would normally go to promiseFn) outside of the 'present the loader or the results' component. Sometimes it is enforced on me due to pre-existent decomposition of the code (separate component with the form for triggering the action).

I found that a good way to tackle this in a utility like this module, is to allow it to work with an existent promise. It would use 'existence of the promise' as an indicator that the run has started. 'lack of a promise', in turn, would be an indicator that the whole component shouldn't show at all yet, because the user hasn't started the action yet (NOTE: this second requirement is also a must for this feature to be useful for these frequent use cases).

Notably, I also found that a feature like that is already offered by many of the 'promise pending/resolved/errored' NPM modules, (including a popular one - the react-promise), so I do feel it is a proven common pattern.

[ghengeveld]

To recap, you're suggesting the following:

• Accept a promise prop which takes an optional Promise instance which is already started.
• If promise is undefined and there's no promiseFn either, the Async state should be pending (as it is with deferFn).
• Watch for changes to this prop so when promise becomes an actual Promise, the Async state becomes loading, resolved or rejected, depending on the state of the Promise.

I'll look into it. There might be a few scenarios where this doesn't hold up in the greater scheme of things, but in essence I agree with adding the feature. It does have a few limitations though, such as not being able to reload the promise, which is a break of contract which should be clearly documented.

[slawomir-brzezinski-at-clarksons]

Thanks for a quick response! 👍

Re:

• If promise is undefined and there's no promiseFn either, the Async state should be pending (as it is with deferFn).

No, when there is no promise, this should mean nothing gets rendered. This is consistent with the react-promise's treating of their promise property. This essentially enables the main code improvement from my second snippet above, namely removing the && check and a whole level of nesting over the component usage (the remaining one, getting rid of the few characters of () => isn't really that big of a deal).

It does have a few limitations though, such as not being able to reload the promise, which is a break of contract which should be clearly documented

The fact that one cannot reload the promise comes with the fact that user has shared just a Promise object (which does represent just one piece of work that has already started, and doesn't have a reload in its contract), so it shouldn't come as a surprise. Importantly, this is not a concern for the use cases I'm describing - if user fires another action (in whichever outer component this is done) it will simply exchange the promise, causing a re-render only then (together with the graceful UI feedback of loading state).

[ghengeveld]

If you use <Async.Resolved>, it will not render while the state is pending, which is what you want. This does add one level of nesting though.

Having <Async> itself render nothing while promise is undefined is not consistent with the rest of the API and would mean a breaking change and inconsistent behavior which I'm not willing to do. What if a user is passing promiseFn, but for some reason it's temporarily undefined, would that suddenly cause <Async> not to render anything anymore? What about <Async.Resolved> which might be present within its children. There's no decent way to know it exists, and it might in turn also be rendered conditionally. All in all this would complicate things greatly, for too little benefit.

Please don't take this as criticism. I'm looking for a way to make this possible without breaking existing contracts.

[slawomir-brzezinski-at-clarksons]

Having <Async> itself render nothing while promise is undefined is not consistent with the rest of the API and would mean a breaking change and inconsistent behavior which I'm not willing to do.

It wouldn't be a breaking change, since you're not exposing the property so far. I appreciate it introduces a new way of behaving, but I really think it wouldn't surprise people using the promise property.

Perhaps I haven't seen enough, but in quite a long time, I have never come across a sensible code written for simplicity, where the behavior would not be desired. I think the fact that react-promise (which is what seems to be the only other popular module for this responsibility) has a large happy user-base supports my claim.

Having said that, if you are concerned by people not knowing the consequence, this could come as an extra boolean flag, and then you could make it your preferred default that you think would not surprise users.

[slawomir-brzezinski-at-clarksons]

I'm looking for a way to make this possible without breaking existing contracts.

As per my suggestion, I think it is possible without breaking existent contracts.

What if a user is passing promiseFn, but for some reason it's temporarily undefined, would that suddenly cause <Async> not to render anything anymore? What about <Async.Resolved> which might be present within its children. There's no decent way to know it exists, and it might in turn also be rendered conditionally. All in all this would complicate things greatly, for too little benefit.

I suppose you're pointing at difficulties of implementing the the contract I'm describing. I wouldn't want to suggest I know better how to do this, but I can sense that if you want to introduce this via a flag like hideWhenNoPromise, then you should be free to represent lack of promise by lack of promiseFn, but raise an error when you find promiseFn === undefined only when the flag is false, to keep the old behavior. Still, there are other ways of achieving it and this is only one way that might minimize code refactorings, depending on how the code looks like (some that come to mind: always route promiseFn through promise and just check the flag to decide how to behave on undefined).

[ghengeveld]

I'm currently thinking of using promise={false} as a way to indicate that we're going to deal with a Promise directly, it just isn't there yet. That way we have a clean way to trigger the behavior that's specific to using promise. I'll try some things to see what works best.

Perhaps I haven't seen enough, but in quite a long time, I have never come across a sensible code written for simplicity, where the behavior would not be desired. I think the fact that react-promise (which is what seems to be the only other popular module for this responsibility) has a large happy user-base supports my claim.

I agree with that, in fact it's why there's also a useFetch hook, just to make that particular (but very common) use case easier to deal with. There's a fine balance between a minimal, pure API that works the same in all scenarios and one that's easy to work with in specific scenarios. In principle I prefer the former, but in practice adding more convenient APIs is what can trigger people to use the library in the first place.

[slawomir-brzezinski-at-clarksons]

I'm currently thinking of using promise={false}

Just some criticism of naming:
If that's because you think of passing an instance of a Promise into promiseFn, then the Fn becomes misleading in promiseFn. Also the promise={true} confuses as it doesn't suggest it takes a boolean.
You could make it isPromise={true} and promise={resultPromise}, but really then the isPromise={true} becomes tautological, because you can derive it from the fact that what was specified was the promise, not the promiseFn.

[slawomir-brzezinski-at-clarksons]

in practice adding more convenient APIs is what can trigger people to use the library in the first place.

Very much agreed. Because this isn't rocket science (not to say maintaining an npm module isn't hard work), people often resort to write things like this themselves to minimize repetitive code, and keep it as DRY as possible.

[ghengeveld]

Let's cycle back here and focus on the API. Would the following work for you?

const myPromise = Promise.resolve("whatever") // or undefined, or rejected

<Async promise={myPromise}>
  <Async.Pending>
    `promise` is undefined (not a thenable)
  </Async.Pending>
  <Async.Loading>
    `promise` is defined but unresolved
  </Async.Loading>
  <Async.Resolved>
    `promise` is defined and fulfilled
  </Async.Resolved>
  <Async.Rejected>
    `promise` is defined and rejected
  </Async.Rejected>
</Async>

That seems to be what you initially suggested, and I think this is totally feasible within the current contract.

[slawomir-brzezinski-at-clarksons]

I think Pending is pretty confusing with Loading. Many people use these interchangeably.

But I see that this is what you already have :)

If you insist on a child item instead of attribute, then my two cents is that I'd consider making a backwards compatible change by adding an alias of Async.Unstarted or something similar (to me, Pending should then be phased out). Otherwise, I think you have all the key information so I might as well leave the final word to you.

[ghengeveld]

I'm already implementing it as we speak. As for Pending vs Unstarted (or whatever), you're not the first to bring it up. I agree the naming is not perfect, but I haven't found a decent alternative. Unstarted doesn't sound quite right either. I haven't found it to be worth the trouble of changing it.

[slawomir-brzezinski-at-clarksons]

I haven't found it to be worth the trouble of changing it.

(Cheeky remark warning) People might find it worth the trouble to choose a different, unconfusing library ;)

I feel an alias is a reasonable strategy, and the exact name was just one idea. Anyways, thanks for your hard work and time.

[ghengeveld]

Thanks for helping out.

[slawomir-brzezinski-at-clarksons]

Cheers 👍

Another option came to mind, perhaps sounding more 'right': BeforeStart

[dhurlburtusa]

What about <Async.Undefined> to replace <Async.Pending>?