-
Notifications
You must be signed in to change notification settings - Fork 558
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
RFC: First class support for promises and async/await #229
base: main
Are you sure you want to change the base?
Conversation
55f181c
to
604dd9b
Compare
What would be the naming convention for functions only with const customFunctionThatCanBeCalledConditionallyButOnlyInRender = (p, c) => {
return [use(p), use(c)];
}; It can't be |
(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: 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: |
604dd9b
to
36685e0
Compare
Ah, I missed that part. So, we can't hide |
|
||
If a promise passed to `use` hasn't finished loading, `use` suspends the component's execution by throwing an exception. When the promise finally resolves, React will _replay_ the component's render. During this subsequent attempt, the `use` call will return the fulfilled value of the promise. | ||
|
||
Unlike async/await or a generator function, a suspended component does not resume from the same point that it last left off — the runtime must re-execute all of the code in between the beginning of the component and where it suspended. Replaying relies on the property that React components are required to be idempotent — they contain no external side effects during rendering, and return the same output for a given set of inputs (props, state, and context). As a performance optimization, the React runtime can memoize some of this computation. It's conceptually similar to how components are reexecuted during a state update. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If I have a call like use(getData()), my getData function will be called when React replays the component. What is the intended way to prevent this, if for example performance was a problem?
Also, the component is supposed to be deterministic but doesn't calling a function that almost certainly uses IO introduce non-determinism?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You could probably do something like
const promise = useMemo(() => getData())
comst result = use(promise)
Also, the component is supposed to be deterministic but doesn't calling a function that almost certainly uses IO introduce non-determinism?
Those functions should probably not generally blindly make new function calls, but be part of a caching solution that checks if there is already a request going on (or more broadly: if one is necessary) and only then make the request.
Generally: React components should render towards a result, but they are not side-effect free. Something like that could have a side effect like reading data from a server, but if you have a side effect that actively changes data somewhere, that should always be part of a user interaction and not part of the component render.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think this is where the cache
will come into play that's mentioned throughout the document. You should get more info on this, once its RFC drops.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Also, the component is supposed to be deterministic but doesn't calling a function that almost certainly uses IO introduce non-determinism?
I think this is where the cache will come into play that's mentioned throughout the document. You should get more info on this, once its RFC drops.
Yeah you're meant to cache all IO. The cache
RFC will cover this in detail — the basic idea is there will be a built-in, drop-in API you can use to cache any async/IO operation. I'll update this proposal once that RFC is ready.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It seems like this could be made more explicit (and eliminate the explicit caching burden on the developer) with an API change and a dependency array similar to useEffect
:
use(() => myAPI.fetch(id), [id])
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 We also have helpers that allow us to fetch data in a typical 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 Given that, a few questions / concerns from reading this:
|
Yeah you would return the promise, then the caller would unwrap it with
Yeah the |
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. |
36685e0
to
0ceadb5
Compare
React does this by adding additional properties to the promise object: | ||
|
||
- The **`status`** field is set to one of `"pending"`, `"fulfilled"`, or `"rejected"`. | ||
- When a promise is fulfilled, its **`value`** field is set to the fulfilled value. | ||
- When a promise is rejected, its **`reason`** field is set to the rejection reason (which in practice is usually an error object). |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Would React consider naming those fields differently – or maybe putting those fields behind a symbol?
// Just an example, not necessarily these names:
promise._reactState.status
promise._reactState.value
promise._reactState.reason
// Or using a symbol:
promise[React.PromiseState].status
promise[React.PromiseState].value
promise[React.PromiseState].reason
Otherwise,
...if JavaScript were to ever adopt a standard API for synchronously inspecting the value of a promise...
five years down the road, that API might be limited by choices React made today. (See e.g. [].flatten()
that had to be renamed to [].flat()
, etc.)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah, we're aware of the flat
debacle. I will say it's a little different from what happened with MooTools, though, because we're not modifying the global prototype. Though you're right that it does have implications for future standardization.
The reason we don't want to use private properties, symbols, or React-specific fields is because we want non-React libraries to be able to access these fields, too. It's better for everyone if we all align on the same convention.
Ideally, yes, this would be part of the language. That would be so nice! Would save us so much implementation complexity.
One could argue this is a mildly provocative stance for us to take, but we're hoping if there's enough traction this will motivate the standards bodies to pick up the feature — there seems to have been no traction, despite many proposals over the years, going back to at least 2016 when @sebmarkbage proposed this to TC39.
I also think there's a reasonable way for the standards bodies to workaround this if it becomes an issue: use a reflection-based API like Promise.inspect
, instead of direct properties. That's the design I would probably propose anyway, since this is really meant to be used by framework or library code, not in your regular application logic.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
React-specific fields is because we want non-React libraries to be able to access these fields, too. It's better for everyone if we all align on the same convention.
I am confused about the non-React libraries, can you provide some examples? Wouldn't those libraries be aware of that they are processing something coming from React, despite being a non-React library? If so, can't they just import your Symbol to access these new things?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Would you consider using Symbol.for
to avoid potential collisions with "normal" JS, while still making the value available to libraries that don't depend on React?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The reason we don't want to use private properties, symbols, or React-specific fields is because we want non-React libraries to be able to access these fields, too. It's better for everyone if we all align on the same convention.
This confuses me a little. Isn't the use
primitive even described in the RFC as still a hook requiring a react runtime (to quote "it's still a Hook because it only works when React is rendering." and while allowing conditional suspension inside (i.e. if
) blocks, this would still be essentially unusable outside of the react architecture?
Or are you simply referring to the fact, that you want library authors to be able to access the unwrapped "use-mutated" Promise
primitive?
|
||
## Why can't Client Components be async functions? | ||
|
||
We strongly considered supporting not only async Server Components, but async Client Components, too. It's technically possible, but there are enough pitfalls and caveats involved that, as of now, we aren't comfortable with the pattern as a general recommendation. The plan is to implement support for async Client Components in the runtime, but log a warning during development. The documentation will also discourage their use. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The plan is to implement support for async Client Components in the runtime, but log a warning during development.
This sounds like you want to discourage async Client Components but not outright prohibit them. (Otherwise, you’d throw an error instead of logging a warning.)
Why this choice? (Just curious.)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Does the explanation below make sense?
The main reason we're discouraging async Client Components is because it's too easy for a single prop flow through the component and invalidate its memoization, triggering the microtask dance described in an earlier section. It's not so much that it introduces new performance caveats, but it makes all the performance caveats described above much more likely.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nah, saw that :(
Let me try rephrasing the question. If you want to discourage async client-side components, why not prohibit them completely? (By throwing an error if you encounter an async client component, for example. Right now, as the RFC says, you’re just logging a warning during development.)
Is that because you still want people to experiment with async client components? Some other reason?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah it's so frameworks like React Router or Next.js could experiment with them:
There is one pattern where we think async Client Components make sense: if you structure them in such a way that they're guaranteed to only update during navigations. But the only realistic way to guarantee this in practice is by integrating support directly into your application's router.
The warning won't fire during a transition, so if your framework can guarantee that, it will work.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Wouldn't it be possible to move async Client Components to a worker thread that could just stream the jsx back to main thread?
Instead of having two different set of components (server and client), we could just write it one way and have react delegate those components to a thread when it's not running in a server.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@Noriller That wouldn't have the semantics I'd expect from an async client component. You can only transfer values that are structural cloneable over the worker boundary, which means no functions for example.
Having said that, it is possible to run server components in a worker today but you of course keep all of the server component semantics.
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/ |
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 |
0ceadb5
to
3a9e9da
Compare
@martinheidegger #229 (comment)
There is an experimental version of this we implemented behind a flag, but it's more related to the |
With suspend status will never be "waiting". Just success/error. Error can be thrown. Can we have ErrorBoundary in hooks API?
|
3a9e9da
to
f2508eb
Compare
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.
Libraries like React Query can essentially work the same as they do today. They can either 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 If |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.
Fortunately, Replay's use case is able to use module-level caching for most of our Suspense data, so we sidestep a lot of complexity.
|
||
Because async/await is a syntactical feature, it's a good candidate for compiler-driven optimizations. In the future, we could compile async Server Components to a lower-level, generator-like form to reduce the runtime overhead of things like microtasks, without affecting the outward behavior of the component. And although `use` is not technically a syntactical construct in JavaScript, it effectively acts as syntax in the context of React applications (we will use a linter to enforce correct usage) so we can apply similar compiler optimizations on the client, too. | ||
|
||
We've taken care to consider this throughout the design. For example, in the current version of React, an unstable mechanism allows arbitrary functions to suspend during render by throwing a promise. We will be removing this in future releases in favor of `use`. This means only Hooks will be allowed to suspend. An [auto-memoizing compiler](https://reactjs.org/blog/2022/06/15/react-labs-what-we-have-been-working-on-june-2022.html#react-compiler) can take advantage of this knowledge to prevent arbitrary function calls from unnecessarily invalidating a memoized computation. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This means only Hooks will be allowed to suspend.
At first I was confused and concerned about the "only Hooks" statement, until I saw:
The only requirement is that the parent function must be a React component or Hook
😄
} | ||
``` | ||
|
||
The rules regarding where `use` can be called in a React component correspond to where `await` can be invoked in an async function, or `yield` in a generator function. `use` can be called from within any control flow construct including blocks, switch statements, and loops. The only requirement is that the parent function must be a React component or Hook. So, for example, `use` can be called inside a for loop, but it cannot be called inside a closure passed to a `map` method call: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why? This seems like it could be pretty awkward in practice. (Easy to forget about.)
Is this constraint being imposed to increase the likelihood of being able to port to a more native approach in the future?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah that's one reason. It makes it easier for a compiler to optimize this. Note that it's the same restriction that applies to await
in async functions and yield
in generators.
The other benefit is that we think it'd be too hard to track whether you're inside a non-React function without a solid naming convention:
If we allowed use to be called in regular functions, it would be up to the developer to keep track of whether it was being in the right context, since there's no way to enforce this in the type systems of today. That was one of the reasons we created the "use"- naming convention in the first place, to distinguish between React functions and non-React functions.
[...]
We could introduce a separate naming convention that's different from hooks, but it doesn't seem worth adding yet-another type of React function for only this case. In practice, we don't think it will be a big issue, in the same way that not being able to call Hooks conditionally isn't a big deal.
|
||
Here's where our tricks start getting more complicated. | ||
|
||
What we can do in this case is rely on the fact that the promise returned from `fetchTodo` will resolve in a microtask. Rather than suspend, React will wait for the microtask queue to flush. If the promise resolves within that period, React can immediately replay the component and resume rendering, without triggering a Suspense fallback. Otherwise, React must assume that fresh data was requested, and will suspend like normal. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Rather than suspend, React will wait for the microtask queue to flush.
If the promise resolves within that period, React can immediately replay the component and resume rendering, without triggering a Suspense fallback.
How do inputs invalidate the cached promise?
If props/state change, those aren't guaranted to be ones that impact the cached promise, but they might.
Why not use ane explicit deps array?
Nevermind. This is clever.
The upcoming cache
API looks nice too (at least what you've hinted at)!
|
||
Although this convention is not part of the JavaScript specification, we think it's a reasonable way to track a promise's result. The ideal is that the lifetime of the resolved value corresponds to the lifetime of the promise object. The most straightforward way to implement this is by adding a property directly to the promise. | ||
|
||
An alternative would be to use a WeakMap, which offers similar benefits. The advantage of using a property instead of a WeakMap is that other frameworks besides React can access these fields, too. For example, a data framework can set the `status` and `value` fields on a promise preemptively, before passing to React, so that React can unwrap it without waiting a microtask. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
How do you plan to handle versioning for these non-standard, convention fields?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Versioning for React? I think the plan is that we don't ever change them :) Our hope is that other libraries adopt the same convention, and then that convinces the standards bodies to address with a built-in JavaScript version. Something like Promise.inspect
. But we're aware this is mildly provocative.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Considering you are the most popular UI framework out there by a long margin and most probably one of top ten NPM packages downloaded for a really long time by now; properties you add to promises might have huge ramificafions. Even if you dont pollute the Promise prototype and even if it's only your promises. At the end of the day TC39 wont want to break every React application out there when it comes to updating things in the future.
I dont know; I currently dont see any reason against proceeding with a Symbol.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why not create a dedicated React-agnostic package using symbols instead of attributes? The API would be stable, feature-complete, and usable by other libraries, and you could even improve typing support with an InspectablePromise
type
|
||
To mitigate confusion, the intent is that `use` will be the _only_ Hook that will _ever_ support conditional execution — instead of having to learn about a handful of Hooks that are exempted from the typical rules, developers will only have to remember one. | ||
|
||
Though it's not strictly within the scope of this proposal, `use` will eventually support additional types besides promises. For example, the first non-promise type to be supported will be Context — `use(Context)` will behave identically to `useContext(Context)`, except that it can be called conditionally. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Cool!
@tobias-tengler #229 (comment)
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(...);
The hope is that once |
@acdlite I know this is really starting to get over into the territory of the 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 How would you propose a library expose that same general functionality in a non-hook form? |
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 |
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 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. |
That's my point and question, in two ways:
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? |
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.
Yeah I would probably add a separate API. Similar to when Redux introduced The two APIs can share internal implementation but the public interface remains clean (e.g. types, as you pointed out). |
To avoid confusion. We are patching `fetch`, and only `fetch`, for a small fix scoped to react renders elsewhere, but this code is not it. This code was for the strategy used in the original [React Server Components demo](https://github.com/reactjs/server-components-demo). Which [we announced](https://reactjs.org/blog/2022/06/15/react-labs-what-we-have-been-working-on-june-2022.html) that we're moving away from in favor of [First class support for promises and async/await](reactjs/rfcs#229). We might explore using these package for other instrumentation in the future but not now and not like this.
Just want to echo this again to make it critical. "use" is an extremely common term in daily searching. Even from the existing
when we say This can unintentionally pollute the existing well-established It will be indeed a nightmare to distinguish the technical meaning and the daily literal meaning. |
It's a valid point right here, seems like this hook was named so without even considering the new developers who are still learning React. Only experienced React developers will understand the |
Not sure if this was asked before, but is it expected that I noticed this currently throws an error in Next.js: vercel/next.js#44778. Note that import {use} from 'react';
// ✅ Works
export default function ServerComponent() {
return use(Promise.resolve('Hello'));
}
// ❌ Breaks
export default async function ServerComponent() {
return use(Promise.resolve('Hello'));
} I understand that The RFC also mentions:
That would be a nice side effect for my use case since a large number of components will reuse the resolved value. |
As I know, nextjs version 13's server component can't be used with hooks. Isn't that error pointing that? |
Hmm, which hooks are you referring to? E.g. |
With @acdlite leaving Meta, does that mean this pull request will be deferred until after NextJS 13 and the server components release is really wrapped and released? A while back @gaearon said here vercel/swr#1906 (comment) that "server components will release before client side suspense and data fetching will happen in non relay data libraries (or at least that is how I interpret it). Would it be a correct assumption to assume that Suspense support on the client side with a hook like "use()" in this proposal is not coming out until after Next.js Server components have shipped? |
I can't tell you anything about release timeline, but I've noticed this demo is already consuming a |
|
||
# Unresolved questions | ||
|
||
The main piece missing from this proposal is `cache`, a API for caching async requests. This will be addressed in a companion RFC soon, at which point we'll update this proposal to include references where appropriate. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
is it ready now? I can't wait to see the cache
part. 😄
Since it took me a bit to find it the above experimental repo, here is the link to where |
Absolutely right. based on above, |
I'd like to reiterate the concern on cancellation: #229 (comment) I think this is a pretty big concern, since It's actually one of the reasons why currently "the render function must be kept side-effect free". |
Apologies for asking, but is there any update on this RFC? It was introduced nearly a year ago, and yet it hasn't been merged and/or included in a stable version of React. |
Note that while this hasn't been merged into the RFCs yet, it has already been implemented in React canaries (though you'll likely want a meta framework like Next). |
To avoid confusion. We are patching `fetch`, and only `fetch`, for a small fix scoped to react renders elsewhere, but this code is not it. This code was for the strategy used in the original [React Server Components demo](https://github.com/reactjs/server-components-demo). Which [we announced](https://reactjs.org/blog/2022/06/15/react-labs-what-we-have-been-working-on-june-2022.html) that we're moving away from in favor of [First class support for promises and async/await](reactjs/rfcs#229). We might explore using these package for other instrumentation in the future but not now and not like this.
Adds first class support for reading the result of a JavaScript Promise using Suspense:
await
syntax by defining your component as an async function.use
Hook. Likeawait
,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