rfcs icon indicating copy to clipboard operation
rfcs copied to clipboard

RFC: First class support for promises and async/await

Open acdlite opened this issue 1 year ago • 129 comments

Adds first class support for reading the result of a JavaScript Promise using Suspense:

  • Introduces support for async/await in Server Components. Write Server Components using standard JavaScript await syntax by defining your component as an async function.
  • Introduces the use Hook. Like await, use unwraps the value of a promise, but it can be used inside normal components and Hooks, including on the client.

This enables React developers to access arbitrary asynchronous data sources with Suspense via a stable API.

View the rendered text

acdlite avatar Oct 13 '22 12:10 acdlite

What would be the naming convention for functions only with use?

const customFunctionThatCanBeCalledConditionallyButOnlyInRender = (p, c) => {
  return [use(p), use(c)];
};

It can't be useFoo as eslint rule check will be false positive. So, no naming convention?

dai-shi avatar Oct 13 '22 14:10 dai-shi

What would be the naming convention for functions only with use?

use is the only Hook that's allowed to be called conditionally. And you can only call use from inside another Hook (or component).

(There was a sentence in the RFC that I think made this a bit ambiguous; I tweaked it so it's hopefully clearer.)

Calling from a non-Hook will "work" in the runtime, but the linter will forbid it. It's the same restriction as other Hooks: use is only permitted inside components and other custom Hooks.

It's an intentional decision that this proposal would effectively prevent arbitrary function calls from being allowed to suspend. There are a few reasons for this, one of them being that it allows an auto-memoizing compiler to more effectively reuse computations.

But arguably the primary reason is the concern you just raised: it makes it easier to tell which functions are allowed to call it.

Relevant sections of RFC:

acdlite avatar Oct 13 '22 14:10 acdlite

Ah, I missed that part. So, we can't hide use within another function. It's a very unfortunate limitation from the runtime solution perspective, but maybe required for the compiler.

dai-shi avatar Oct 13 '22 14:10 dai-shi

Brian Vaughn joined Replay ( https://replay.io ) a few months ago, and we've been intentionally using pre-alpha/experimental Suspense behavior for data fetching. The pattern Brian introduced is a "Suspense cache", which primarily uses a Wakeable type (basically a sync promise) and then throws it to trigger Suspense behavior:

We also have helpers that allow us to fetch data in a typical async function, such as from a Redux thunk.

Typical usage looks like:

// Suspense in a component
function ColumnBreakpoints(props) {
  // actually fetch and suspend
  
  const breakpoints = getBreakpointPositionsSuspense(replayClient, focusedSourceId, visibleLines);

  // rendering
}

// In an async function such as a thunk
export function selectNode(nodeId: string, reason?: SelectionReason): UIThunkAction {
  return async (dispatch, getState, { ThreadFront, replayClient, protocolClient }) => {

    const nodes = await getNodeDataAsync(
      protocolClient,
      replayClient,
      ThreadFront.sessionId!,
      ThreadFront.currentPause!.pauseId!,
      { type: "parentNodes", nodeId }
    );

    // more logic
  }
}

Again, to be clear, we're very knowingly using pre-pre-alpha / experimental behavior here :) and we've come up with a couple custom abstractions like this createGenericCache API for our own use.

Given that, a few questions / concerns from reading this:

  • The React team has previously talked about this "Promise cache" pattern along the lines of SomeDataCache.read(id), where that cache would itself throw the promise if the data doesn't exist yet and it starts a request. I know this RFC talks about a future cache API, but doesn't go into details yet. If use is the replacement for the existing-but-now-deprecated "throw a promise" behavior, is the SomeDataCache.read(id) pattern no longer a goal or planned usage approach? Or is it that the "Promise cache" would exist and just return the promise instead of throwing it?
  • We've definitely got plenty of cases in our codebase where logic outside of React needs to trigger data fetching, which may then tie into Suspense rendering. Any direction on how that would be done here?
  • The restriction on "no use calls in other functions" feels very restrictive at first glance, although I don't have concrete counter-examples off the top of my head

markerikson avatar Oct 13 '22 15:10 markerikson

@markerikson https://github.com/reactjs/rfcs/pull/229#issuecomment-1277757239

The React team has previously talked about this "Promise cache" pattern along the lines of SomeDataCache.read(id), where that cache would itself throw the promise if the data doesn't exist yet and it starts a request. I know this RFC talks about a future cache API, but doesn't go into details yet. If use is the replacement for the existing-but-now-deprecated "throw a promise" behavior, is the SomeDataCache.read(id) pattern no longer a goal or planned usage approach?

cache will be the built-in cache API — it's an evolution of the experimental <Cache /> API that is currently in the experimental channel. It has different implementation details but if you're currently using <Cache /> then cache should fit your needs just as well.

Or is it that the "Promise cache" would exist and just return the promise instead of throwing it?

Yeah you would return the promise, then the caller would unwrap it with use. It might feel a bit strange at first if you're used to the old Suspense pattern, but it's not that different from how you would write code in the async/await paradigm.

We've definitely got plenty of cases in our codebase where logic outside of React needs to trigger data fetching, which may then tie into Suspense rendering. Any direction on how that would be done here?

Yeah the cache RFC will cover this, too. You'll be able to call it from outside React (inside regular async/await code) and then reuse the cached response during rendering.

acdlite avatar Oct 13 '22 15:10 acdlite

@dai-shi https://github.com/reactjs/rfcs/pull/229#issuecomment-1277722422

Ah, I missed that part. So, we can't hide use within another function. It's a very unfortunate limitation from the runtime solution perspective, but maybe required for the compiler.

It would also just be really hard to keep track of which functions are only called inside React functions, without a solid naming convention. We could introduce a new naming convention that's different from hooks but it doesn't seem worth adding yet-another special type of function for only this case.

In practice I don't think it will feel that limiting, just as it's usually not a big deal that custom Hooks can't be conditional.

acdlite avatar Oct 13 '22 15:10 acdlite

Some reactions over in the Reddit /r/reactjs thread:

https://www.reddit.com/r/reactjs/comments/y30uga/react_rfc_first_class_support_for_promises_and/

markerikson avatar Oct 13 '22 16:10 markerikson

For proper support of async / await in the client I would assume first-class support of AbortSignals:

// if a function is passed instead of a promise, it will be called with a signal.
// upon "unrendering" of the component the signal is aborted. 
const note = use(({ signal }) => fetchNote(id, { signal }));

Also, with the proposed API I am wondering how to distinguish an empty result form a finished result:

const note = use(Promise.resolve(undefined))

maybe a different API would prevent future head-aches and workarounds?

const { state, result, error } = use(Promise.resolve(undefined))
if (state === 0) // waiting
if (state === 1) // successful
if (state === -1) // errored

martinheidegger avatar Oct 13 '22 16:10 martinheidegger

@martinheidegger https://github.com/reactjs/rfcs/pull/229#issuecomment-1277865338

For proper support of async / await in the client I would assume first-class support of AbortSignals

There is an experimental version of this we implemented behind a flag, but it's more related to the cache proposal that we're working on. (I do apologize that proposal isn't ready yet — we're aware that a lot of the questions that are coming up are hard to answer without more knowledge of how caching works.) We do see the value but we'd need to figure out what the lifetime of the signal is — for example, does it live for as long as the response is cached, or only until it finishes?

acdlite avatar Oct 13 '22 16:10 acdlite

With suspend status will never be "waiting". Just success/error. Error can be thrown. Can we have ErrorBoundary in hooks API?

react-query does a great job with promises.

eugeneo avatar Oct 13 '22 16:10 eugeneo

@eugeneo https://github.com/reactjs/rfcs/pull/229#issuecomment-1277873541

With suspend status will never be "waiting". Just success/error. Error can be thrown. Can we have ErrorBoundary in hooks API?

The status for that is "pending" — if you read a promise that's still pending, it will suspend and trigger the nearest Suspense fallback.

If the status is "error" it will trigger the nearest error boundary.

react-query does a great job with promises.

Libraries like React Query can essentially work the same as they do today. They can either use use internally, or they can return a promise to the caller, and then the caller would unwrap the result of the promise with use.

acdlite avatar Oct 13 '22 16:10 acdlite

Libraries like React Query can essentially work the same as they do today. They can either use use internally, or they can return a promise to the caller, and then the caller would unwrap the result of the promise with use.

const promise = useQuery(...);
const data = use(promise);

feels like a very clunky user-facing API, so I don't think it will catch on.

Just to get this right though: The ability to call use conditionally will be lost, once exposed as part of a custom hook? A useQuery hook for example, built on top of use, won't be callable conditionally, right? I understand use and cache very much as primitives that regular users shouldn't or shouldn't have to interact with. Most of the time they will be an implementation detail of some 3rd-party solution or data fetching abstraction. I feel like use's ability to be called conditionally would be used as an escape hatch to the established hooks behavior: "Would be great if I could just do this fetch here conditionally, but our established useData hook won't allow me to" "Ah right, use can be called conditionally, maybe I can just use it for this one special case" ... Time passes, system gets out-of-sync, yadda yadda yadda...

If use's ability to be called conditionally is lost in 90% of real world use cases, where it is abstracted, why even divert from established patterns? What are your thoughts?

tobias-tengler avatar Oct 13 '22 17:10 tobias-tengler

@tobias-tengler https://github.com/reactjs/rfcs/pull/229#issuecomment-1277948794

const promise = useQuery(...); const data = use(promise);

feels like a very clunky user-facing API, so I don't think it will catch on.

In practice we expect it would look more like this:

const data = use(fetchQuery(...));

which you could also call from a non-React function like this:

const data = await fetchQuery(...);

If use's ability to be called conditionally is lost in 90% of real world use cases, where it is abstracted, why even divert from established patterns? What are your thoughts?

The hope is that once use exists, libraries will adapt their APIs to support conditional suspending. I know it seems weird compared to what we're used to but I do think the comparison to async/await is apt. It's basically the same pattern. One day we might literally be able to use async/await directly in Client Components, just as we're currently proposing for Server Components.

acdlite avatar Oct 13 '22 18:10 acdlite

@acdlite I know this is really starting to get over into the territory of the cache API RFC that doesn't exist yet, but what would that sort of "adapted library API" look like?

I'm specifically thinking of the major existing data fetching libs here like Apollo, SWR, React Query, and RTK Query. All of them have settled on a fairly similar useQuery()-style API that returns something like { data, isFetching }.

How would you propose a library expose that same general functionality in a non-hook form?

markerikson avatar Oct 13 '22 18:10 markerikson

@markerikson https://github.com/reactjs/rfcs/pull/229#issuecomment-1278030280

All of them have settled on a fairly similar useQuery()-style API that returns something like { data, isFetching }.

use isn't going to be useful for those APIs anyway, because it requires Suspense. Libraries or APIs that aren't interested in integrating with Suspense can keep doing what they're already doing with their Hook-based APIs.

But presumably some of those libraries have landed on that convention because Suspense for data fetching hasn't been stable until now, and once it is stable (with use) they'll switch to Suspense.

acdlite avatar Oct 13 '22 19:10 acdlite

Also note that it's totally possible to do this:

const promise = useQuery(...);
if (condition) {
  const data = use(promise);
}

maybe with like an opt in flag to return a promise instead of suspending directly. Then you don't have to adopt the cache API, you can just do whatever you're already doing.

It looks clunkier compared to the version that suspends internally but the nice thing is that the user has the option to conditionally suspend based on some other value.

acdlite avatar Oct 13 '22 19:10 acdlite

use isn't going to be useful for those APIs anyway, because it requires Suspense. Libraries or APIs that aren't interested in integrating with Suspense can keep doing what they're already doing with their Hook-based APIs.

That's my point and question, in two ways:

  • There's a ton of code in the ecosystem that is using these hooks. Is the expectation that libraries would provide an alternate API, and that the intended path is that ecosystem would stop using useQuery() hooks? That's asking an awful lot and implies a ton of migration.
  • Give that these hooks already have a defined result of "some object like {data, isFetching}, returning a Promise from the query hook would be a complete breaking change. I do think a {suspense: true} type option might be more feasible, but even there that would start to wreak havoc with TS types. I could more imagine a useQuery hook calling use() internally.

But this goes back to what I was asking a minute ago.

Given that these hooks exist and have broad adoption, how would you envision an alternate "fetch this data" API for the same library looking like?

markerikson avatar Oct 13 '22 19:10 markerikson

@markerikson https://github.com/reactjs/rfcs/pull/229#issuecomment-1278112957

There's a ton of code in the ecosystem that is using these hooks. Is the expectation that libraries would provide an alternate API, and that the intended path is that ecosystem would stop using useQuery() hooks? That's asking an awful lot and implies a ton of migration.

Well yeah but there's nothing forcing them to migrate immediately. It'll be incremental, like when we introduced Hooks — we didn't delete class components from the universe, but it turned out that enough people liked the new pattern that most of the community eventually switched over.

Ultimately it comes down to whether the community at large finds Suspense compelling enough. If they don't, they can stay with the existing APIs. If they do, then here's a new set of functionality that is unlocked by that.

Given that these hooks exist and have broad adoption, how would you envision an alternate "fetch this data" API for the same library looking like?

Yeah I would probably add a separate API. Similar to when Redux introduced useSelector. That's also the strategy Relay used when they started migrating to Suspense a few years ago.

The two APIs can share internal implementation but the public interface remains clean (e.g. types, as you pointed out).

acdlite avatar Oct 13 '22 20:10 acdlite

Suggestion: not only allow

use(promise)

but also

use({
  promise
})

or

use({
  [useSymbol]: promise
})

(of course while keeping use(promise) the obvious api)

So useQuery could just add that promise or [promiseSymbol] property and React could latch on that.

That way, use(useQuery()) would be possible without a complete api change.

phryneas avatar Oct 13 '22 20:10 phryneas

@phryneas https://github.com/reactjs/rfcs/pull/229#issuecomment-1278124600

So useQuery could just add that promise or [promiseSymbol] property and React could latch on that.

It actually works with any thenable object. So if you add a then method that implements the Promise interface, that'll work.

acdlite avatar Oct 13 '22 20:10 acdlite

However, I'm not really sure what that solves because if the data is already part of that object then what's the point of suspending with use?

@acdlite the data might not already be part - it could still be in a pending status at that point - the current implementation does not suspend (we have read the changelog and know that's experimental so we did not include that in the official api).

A .then would probably work although I'm a bit afraid that people might start to use that manually and I can't imagine what kind of weird stuff they would build on that. (We want that api to be as declarative as possible, so also have no "finished" callbacks on component level.)

phryneas avatar Oct 13 '22 20:10 phryneas

@phryneas https://github.com/reactjs/rfcs/pull/229#issuecomment-1278145114

@acdlite the data might not already be part - it could still be in a pending status at that point - the current implementation does not suspend (we have read the changelog and know that's experimental so we did not include that in the official api).

Yeah sorry I edited my comment right after sending it when I realized that :D

acdlite avatar Oct 13 '22 20:10 acdlite

I also edited the second half into my answer ^^

Any thoughts on that? If possible, we'd love to expose that promise to React, but not to the user.

phryneas avatar Oct 13 '22 20:10 phryneas

It's something to consider, at least. My inclination would be to make it more like a [ReactUseSymbol] and then you could call other usable things in there, like context. But probably should be a separate RFC.

acdlite avatar Oct 13 '22 20:10 acdlite

@acdlite https://github.com/reactjs/rfcs/pull/229#issuecomment-1277808487

In practice I don't think it will feel that limiting, just as it's usually not a big deal that custom Hooks can't be conditional.

I think it's very limiting if we further explore use only solutions, because we can't extract reusable logic into functions. Off the top of my head, I'd have some promises in a context, and use the context and use one of promises. And that will be my library code.

We will be having more reusable patterns when we have more use(usable) patterns in the future.

Maybe it will be mitigated with the cache RFC.

(I have another real use case in practice, but it's probably a rare case.)

dai-shi avatar Oct 14 '22 00:10 dai-shi

Well yeah but there's nothing forcing them to migrate immediately. It'll be incremental, like when we introduced Hooks — we didn't delete class components from the universe, but it turned out that enough people liked the new pattern that most of the community eventually switched over.

Ultimately it comes down to whether the community at large finds Suspense compelling enough. If they don't, they can stay with the existing APIs. If they do, then here's a new set of functionality that is unlocked by that.

This is comparison is a bit confusing. The way I am interpreting it is that the new use API enables Suspense and a new set of functionality that is unlocked by it… however Suspense is already supported by APIs like Relay’s hooks and they already achieve the functionality of Suspense. My initial interpretation of the RFC is that use is an attempt to bridge the gap between the land of asynchronous JavaScript and React for use cases that can’t rely on data fetching libraries like Relay. Not that this is a replacement for Suspense-powered hooks, but this comment makes it sound like it is meant as a replacement for those hooks (of course a gradual one)

Mathspy avatar Oct 14 '22 00:10 Mathspy

@dai-shi You can call async functions and use their Promises. It's the same principle for why async functions are annotated and have await.

sebmarkbage avatar Oct 14 '22 00:10 sebmarkbage

@dai-shi You can call async functions and use their Promises. It's the same principle for why async functions are annotated and have await.

I might still be missing something, but can we use use(context) in those async functions?


Maybe we can focus on use(context) only. I can imagine creating a function that uses multiple contexts and does some logic.

dai-shi avatar Oct 14 '22 00:10 dai-shi

Yea, no that's a limitation. They have to be Hooks or you have to read it on the outside and pass it in. If Hooks could be made conditional and used in some loops, then it becomes less limiting though.

sebmarkbage avatar Oct 14 '22 00:10 sebmarkbage

Is there a way we could also support async iterables? From what I understand, you couldn't use them with use because they're not necessarily generator functions, and you couldn't use them from server components because they're not async functions.

nickmccurdy avatar Oct 14 '22 04:10 nickmccurdy