Similar to https://github.com/facebook/react/pull/30768 we want to
schedule work during prerendering in microtasks both for the root task
and pings. We continue to schedule flushes as Tasks to allow as much
work to be batched up as possible.
The unbundled form is just a way to show case a prototype for how an
unbundled version of RSC can work. It's not really intended for every
bundler combination to provide such a configuration.
There's no configuration of Turbopack that supports this mode atm and
possibly never will be since it's more of an integrated server/client
experience.
This removes the unbundled form and node register/loaders from the
turbopack build.
Follow up to #30741.
This is just for the reference Webpack implementation.
If there is a source map associated with a Node ESM loader, we generate
new source map entries for every `registerServerReference` call.
To avoid messing too much with it, this doesn't rewrite the original
mappings. It just reads them while finding each of the exports in the
original mappings. We need to read all since whatever we append at the
end is relative. Then we just generate new appended entries at the end.
For the location I picked the location of the local name identifier.
Since that's the name of the function and that gives us a source map
name index. It means it jumps to the name rather than the beginning of
the function declaration. It could be made more clever like finding a
local function definition if it is reexported. We could also point to
the line/column of the function declaration rather than the identifier
but point to the name index of the identifier name.
Now jumping to definition works in the fixture.
<img width="574" alt="Screenshot 2024-08-20 at 2 49 07 PM"
src="https://github.com/user-attachments/assets/7710f0e6-2cee-4aad-8d4c-ae985f8289eb">
Unfortunately this technique doesn't seem to work in Firefox nor Safari.
They don't apply the source map for jumping to the definition.
In https://github.com/facebook/react/pull/29491 I updated the work
scheduler for Flight to use microtasks to perform work when something
pings. This is useful but it does have some downsides in terms of our
ability to do task prioritization. Additionally the initial work is not
instantiated using a microtask which is inconsistent with how pings
work.
In this change I update the scheduling logic to use microtasks
consistently for prerenders and use regular tasks for renders both for
the initial work and pings.
Shortcut for the common case where only a single flag is checked. Same
as `gate(flags => flags.enableFeatureFlag)`.
Normally I don't care about these types of conveniences but I'm about to
add a lot more inline flag checks these all over our tests and it gets
noisy. This helps a bit.
When we introduced prerendering for flight we modeled an abort of a
flight prerender as having unfinished rows. This is similar to how
postpone was already implemented when you postponed from "within" a
prerender using React.unstable_postpone. However when aborting with a
postponed instance every boundary would be eagerly marked for client
rendering which is more akin to prerendering and then resuming with an
aborted signal.
The insight with the flight work was that it's not so much the postpone
that describes the intended semantics but the abort combined with a
prerender. So like in flight when you abort a prerender and enableHalt
is enabled boundaries and the shell won't error for any reason. Fizz
will still call onPostpone and onError according to the abort reason but
the consuemr of the prerender should expect to resume it before trying
to use it.
When aborting a prerender we should leave references unfulfilled, not
share a common unfullfilled reference. functionally today this doesn't
matter because we don't have resuming but the semantic is that the row
was not available when the abort happened and in a resume the row should
fill in. But by pointing each task to a common unfulfilled chunk we lose
the ability for these references to resolves to distinct values on
resume.
When aborting with a postpone value boundaries are put into client
rendered mode even during prerenders. This doesn't follow the postpoen
semantics of the rest of fizz where during a prerender a postpone is
tracked and it will leave holes in tracked postpone state that can be
resumed. This change updates this behavior to match the postpones
semantics between aborts and imperative postpones.
Noticed this from #30707. This was vestigial from from circleci and now
that we're on GH actions I think we should be able to remove this option
altogether.
ghstack-source-id: 78e8b0243b
Pull Request resolved: https://github.com/facebook/react/pull/30753
stacked on: #30731
We've refined the model of halting a prerender. Now when you abort
during a prerender we simply omit the rows that would complete the
flight render. This is analagous to prerendering in Fizz where you must
resume the prerender to actually result in errors propagating in the
postponed holes. We don't have a resume yet for flight and it's not
entirely clear how that will work however the key insight here is that
deciding whether the never resolving rows are an error or not should
really be done on the consuming side rather than in the producer.
This PR also reintroduces the logs for the abort error/postpone when
prerendering which will give you some indication that something wasn't
finished when the prerender was aborted.
Stacked on #30731.
When logging a Promise we emit it as an infinite promise instead of
blocking the replay on it.
This models that as a halted row instead. No need for this special case.
I unflag the receiving side since now it's used to replace a feature
that's already unflagged so it's used.
When printing these in DevTools they show up as the name of the
constructor so then you pass a Promise to the client it logs as "Chunk"
which is confusing.
Ideally we'd probably just name this Promise but 1) there's a slight
difference in the .then method atm 2) it's a bit tricky to name a
variable and get it from the global in the same scope. Closure compiler
doesn't let us just name a function because it removes it and just uses
the variable name.
using infinitely suspending promises isn't right because this will parse
as a promise which is only appropriate if the value we're halting at is
a promise. Instead we need to have a special marker type that says this
reference will never resolve. Additionally flight client needs to not
error any halted references when the stream closes because they will
otherwise appear as an error
addresses:
https://github.com/facebook/react/pull/30705#discussion_r1720479974
Per comments on the new validation pass, this disallows creating JSX (expression/fragment) within a try statement. Developers sometimes use this pattern thinking that they can catch errors during the rendering of the element, without realizing that rendering is lazy. The validation allows us to teach developers about the error boundary pattern.
ghstack-source-id: 0bc722aeaed426ddd40e075c008f0ff2576e0c33
Pull Request resolved: https://github.com/facebook/react/pull/30725
This uses a similar technique to what we use to generate fake stack
frames for server components. This generates an eval:ed wrapper function
around the Server Reference proxy we create on the client. This wrapper
function gets the original `name` of the action on the server and I also
add a source map if `findSourceMapURL` is defined that points back to
the source of the server function.
For `"use server"` on the server, there's no new API. It just uses the
callsite of `registerServerReference()` on the Server. We can infer the
function name from the actual function on the server and we already have
the `findSourceMapURL` on the client receiving it.
For `"use server"` imported from the client, there's two new options
added to `createServerReference()` (in addition to the optional
[`encodeFormAction`](#27563)). These are only used in DEV mode. The
[`findSourceMapURL`](#29708) option is the same one added in #29708. We
need to pass this these references aren't created in the context of any
specific request but globally. The other weird thing about this case is
that this is actually a case where the compiled environment is the
client so any source maps are the same as for the client layer, so the
environment name here is just `"Client"`.
```diff
createServerReference(
id: string,
callServer: CallServerCallback,
encodeFormAction?: EncodeFormActionCallback,
+ findSourceMapURL?: FindSourceMapURLCallback, // DEV-only
+ functionName?: string, // DEV-only
)
```
The key is that we use the location of the
`registerServerReference()`/`createServerReference()` call as the
location of the function. A compiler can either emit those at the same
locations as the original functions or use source maps to have those
segments refer to the original location of the function (or in the case
of a re-export the original location of the re-export is also a fine
approximate). The compiled output must call these directly without a
wrapper function because the wrapper adds a stack frame. I decided
against complicated and fragile dev-only options to skip n number of
frames that would just end up in prod code. The implementation just
skips one frame - our own. Otherwise it'll just point all source mapping
to the wrapper.
We don't have a `"use server"` imported from the client implementation
in the reference implementation/fixture so it's a bit tricky to test
that. In the case of CJS on the server, we just use a runtime instead of
compiler so it's tricky to source map those appropriately. We can
implement it for ESM on the server which is the main thing we're testing
in the fixture. It's easier in a real implementation where all the
compilation is just one pass. It's a little tricky since we have to
parse and append to other source maps but I'd like to do that as a
follow up. Or maybe that's just an exercise for the reader.
You can right click an action and click "Go to Definition".
<img width="1323" alt="Screenshot 2024-08-17 at 6 04 27 PM"
src="https://github.com/user-attachments/assets/94d379b3-8871-4671-a20d-cbf9cfbc2c6e">
For now they simply don't point to the right place but you can still
jump to the right file in the fixture:
<img width="1512" alt="Screenshot 2024-08-17 at 5 58 40 PM"
src="https://github.com/user-attachments/assets/1ea5d665-e25a-44ca-9515-481dd3c5c2fe">
In Firefox/Safari given that the location doesn't exist in the source
map yet, the browser refuses to open the file. Where as Chrome does
nearest (last) line.
It is possible to throw after aborting during a render and we were not
properly tracking this. We use an AbortSigil to mark whether a rendering
task needs to abort but the throw interrupts that and we end up handling
an error on the error pathway instead.
This change reworks the abort-while-rendering support to be robust to
throws after calling abort
Addresses a todo from a while back. We now validate environment options when parsing the plugin options, which means we can stop re-parsing/validating in later phases.
ghstack-source-id: b19806e843e1254716705b33dcf86afb7223f6c7
Pull Request resolved: https://github.com/facebook/react/pull/30726
This enables finding Server Components on the owner path. Server
Components aren't stateful so there's not actually one specific owner
that it necessarily matches. So it can't be a global look up. E.g. the
same Server Component can be rendered in two places or even nested
inside each other.
Therefore we need to find an appropriate instance using a heuristic. We
can do that by traversing the parent path since the owner is likely also
a parent. Not always but almost always.
To simplify things we can also do the same for Fibers. That brings us
one step closer to being able to get rid of the global
fiberToFiberInstance map since we can just use the shadow tree to find
this information.
This does mean that we can't find owners that aren't parents which is
usually ok. However, there is a test case that's interesting where you
have a React ART tree inside a DOM tree. In that case the owners
actually span multiple renderers and roots so the owner is not on the
parent stack. Usually this is fine since you'd just care about the
owners within React ART but ideally we'd support this. However, I think
that really the fix to this is that the React ART tree itself should
actually show up inside the DOM tree in DevTools and in the virtual
shadow tree because that's conceptually where it belongs. That would
then solve this particular issue. We'd just need some way to associate
the root with a DOM parent when it gets mounted.
This was a pet peeve where our playground could only compile top level
FunctionDeclarations. Just synthesize a fake identifier if it doesn't
have one.
ghstack-source-id: 882483c79c
Pull Request resolved: https://github.com/facebook/react/pull/30729
This PR updates the eslint plugin to report unused opt out directives.
One of the downsides of the opt out directive is that it opts the
component/hook out of compilation forever, even if the underlying issue
was fixed in product code or fixed in the compiler.
ghstack-source-id: 81deb5c11b
Pull Request resolved: https://github.com/facebook/react/pull/30721
This PR updates the babel plugin to continue the compilation pipeline as
normal on components/hooks that have been opted out using a directive.
Instead, we no longer emit the compiled function when the directive is
present.
Previously, we would skip over the entire pipeline. By continuing to
enter the pipeline, we'll be able to detect if there are unused
directives.
The end result is:
- (no change) 'use forget' will always opt into compilation
- (new) 'use no forget' will opt out of compilation but continue to log
errors without throwing them. This means that a Program containing
multiple functions (some of which are opted out) will continue to
compile correctly
ghstack-source-id: 5bd85df2f8
Pull Request resolved: https://github.com/facebook/react/pull/30720
enableHalt turns on a mode for flight prerenders where aborts are
treated like infinitely stalled outcomes while still completing the
prerender. For regular tasks we simply serialize the slot as a promise
that never settles. For ReadableStream, Blob, and Async Iterators we
just never advance the serialization so they remain unfinished when
consumed on the client.
When enableHalt is turned on aborts of prerenders will halt rather than
error. The abort reason is forwarded to the upstream produces of the
aforementioned async iterators, blobs, and ReadableStreams. In the
future if we expose a signal that you can consume from within a render
to cancel additional work the abort reason will also be forwarded there
Summary:
The change earlier in this stack makes it less safe to have ref enforcement disabled. This diff enables it by default.
ghstack-source-id: d3ab5f1b28b7aed0f0d6d69547bb638a1e326b66
Pull Request resolved: https://github.com/facebook/react/pull/30716
Summary:
We previously were excessively strict about preventing functions that access refs from being returned--doing so is potentially valid for hooks, because the return value may only be used in an event or effect.
ghstack-source-id: cfa8bb1b54e8eb365f2de50d051bd09e09162d7b
Pull Request resolved: https://github.com/facebook/react/pull/30724
Summary:
Since we want to make ref-in-render errors enabled by default, we should position those errors at the location of the read. Not only will this be a better experience, but it also aligns the behavior of Forget and Flow.
This PR also cleans up the resulting error messages to not emit implementation details about place values.
ghstack-source-id: 1d1131706867a6fc88efddd631c4d16d2181e592
Pull Request resolved: https://github.com/facebook/react/pull/30723
Test Plan:
Documents that useCallback calls interfere with it being ok for refs to escape as part of functions into jsx
ghstack-source-id: a5df427981ca32406fb2325e583b64bbe26b1cdd
Pull Request resolved: https://github.com/facebook/react/pull/30714
Summary:
Refs, as stable values that the rules of react around mutability do not apply to, currently are treated as having mutable ranges, and through aliasing, this can extend the mutable range for other values and disrupt good memoization for those values. This PR excludes refs and their .current values from having mutable ranges.
Note that this is unsafe if ref access is allowed in render: if a mutable value is assigned to ref.current and then ref.current is mutated later, we won't realize that the original mutable value's range extends.
ghstack-source-id: e8f36ac25e2c9aadb0bf13bd8142e4593ee9f984
Pull Request resolved: https://github.com/facebook/react/pull/30713
## Summary
Flow will eventually remove the specific `React.Element` type. For most
of the code, it can be replaced with `React.MixedElement` or
`React.Node`.
When specific react elements are required, it needs to be replaced with
either `React$Element` which will trigger a `internal-type` lint error
that can be disabled project-wide, or use
`ExactReactElement_DEPRECATED`.
Fortunately in this case, this one can be replaced with just
`React.MixedElement`.
## How did you test this change?
`flow`
Prerendering in flight is similar to prerendering in Fizz. Instead of
receiving a result (the stream) immediately a promise is returned which
resolves to the stream when the prerender is complete. The promise will
reject if the flight render fatally errors otherwise it will resolve
when the render is completed or is aborted.
Test Plan:
Builds support for a.x++ and friends. Similar to a.x += y, emits it as an assignment expression.
ghstack-source-id: 8f3979913aad561cdba70464c3cc5f0ee95887b5
Pull Request resolved: https://github.com/facebook/react/pull/30697
Summary:
It doesn't seem as though this invariant was necessary
ghstack-source-id: b27e76525911d5cfc1991b5cfdb7b2074c039e21
Pull Request resolved: https://github.com/facebook/react/pull/30699
Supports showing the key in DevTools on the Server Component that the
key was applied to. We can also use this to reconcile to preserve
instance equality when they're reordered.
One thing that's a bit weird about this is that if you provide an
explicit key on a Server Component that alone doesn't have any
semantics. It's because we pass the key down and let the nearest child
inherit the key or get prefixed by the key.
So you might see the same key as a prefix on the child of the Server
Component too which might be a bit confusing. We could remove the prefix
from children but that might also be a bit confusing if they collide.
The div in this case doesn't have a key explicitly specified. It gets it
from the Server Component parent.
<img width="1107" alt="Screenshot 2024-08-14 at 10 06 36 PM"
src="https://github.com/user-attachments/assets/cfc517cc-e737-44c3-a1be-050049267ee2">
Overall keys get a bit confusing when you apply filter. Especially since
it's so common to actually apply the key on a Host Instance. So you
often don't see the key.
Per discussion today, adds validation against calling setState "during" passive effects. Basically, it's fine to _schedule_ setState to be called (via a timeout, listener, etc) but generally not recommended to call setState during the effect since that will trigger a cascading render.
This validation is off by default, i'm putting this up for discussion and to experiment with it internally.
ghstack-source-id: 5f385ddab59561ec3939ae5ece265dfee4f2cb56
Pull Request resolved: https://github.com/facebook/react/pull/30685
This commit updates the file locations and bulid configurations for
flight in preparation for new static entrypoints. This follows a
structure similar to Fizz which has a unified build but exports methods
from different top level entrypoints. This PR doesn't actually add the
new top level entrypoints however, that will arrive in a later update.
During local development it's common to add or remove code which may
change the cache size between renders. Add a failing test to show that
currently (without the compiled fast refresh check) this issues a
warning and reuses the cache which may have stale values.
ghstack-source-id: efdcb017ba3bdadd88b1f8bb5523b1a9f6217eb5
Pull Request resolved: https://github.com/facebook/react/pull/30662
Summary:
UseTransition is a builtin hook that returns a stable value, like useState. This PR represents that in Forget, and marks the startTransition function as stable.
ghstack-source-id: 0e76a64f2d0c86a4eb55c620922b4698250bb5c3
Pull Request resolved: https://github.com/facebook/react/pull/30681
Summary:
In theory, as I understand it, the result of a useRef will never change between renders, because we'll always provide the same ref value consistently. That means that memoization that depends on a ref value will never re-compute, so I think we could not infer it as a dependency in Forget. This diff, however, doesn't do that: it instead allows the validatePreserveExistingMemoizationGuarantees analysis to admit mismatches between explicit dependencies and implicit ones when the implicit dependency is a ref that doesn't exist in source.
ghstack-source-id: 685d859d1eed5d1e19dbbbfadc75be3875ddb6ea
Pull Request resolved: https://github.com/facebook/react/pull/30679
This adds VirtualInstances to the tree. Each Fiber has a list of its
parent Server Components in `_debugInfo`. The algorithm is that when we
enter a set of fibers, we actually traverse level 0 of all the
`_debugInfo` in each fiber. Then level 1 of each `_debugInfo` and so on.
It would be simpler if `_debugInfo` only contained Server Component
since then we could just look at the index in the array but it actually
contains other data as well which leads to multiple passes but we don't
expect it to have a lot of levels before hitting a reified fiber.
Finally when we hit the end a traverse the fiber itself.
This lets us match consecutive `ReactComponentInfo` that are all the
same at the same level. This creates a single VirtualInstance for each
sequence. This lets the same Server Component instance that's a parent
to multiple children appear as a single Instance instead of one per
Fiber.
Since a Server Component's result can be rendered in more than one place
there's not a 1:1 mapping though. If it is in different parents or if
the sequence is interrupted, then it gets split into two different
instances with the same `ReactComponentInfo` data.
The real interesting case is what happens during updates because this
algorithm means that a Fiber can become reparented during an update to
end up in a different VirtualInstance. The ideal would maybe be that the
frontend could deal with this reparenting but instead I basically just
unmount the previous instance (and its children) and mount a new
instance which leads to some interesting scenarios. This is inline with
the strategy I was intending to pursue anyway where instances are
reconciled against the previous children of the same parent instead of
the `fiberToFiberInstance` map - which would let us get rid of that map.
In that case the model is resilient to Fiber being in more than one
place at a time.
However this unmount/remount does mean that we can lose selection when
this happens. We could maybe do something like using the tracked path
like I did for component filters. Ideally it's a weird edge case though
because you'd typically not have it. The main case that it happens now
is for reorders of list of server components. In that case basically all
the children move between server components while the server components
themselves stay in place. We should really include the key in server
components so that we can reconcile them using the key to handle
reorders which would solve the common case anyway.
I convert the name to the `Env(Name)` pattern which allows the
Environment Name to be used as a badge.
<img width="1105" alt="Screenshot 2024-08-13 at 9 55 29 PM"
src="https://github.com/user-attachments/assets/323c20ba-b655-4ee8-84fa-8233f55d2999">
(Screenshot is with #30667. I haven't tried it with the alternative
fix.)
---------
Co-authored-by: Ruslan Lesiutin <rdlesyutin@gmail.com>
Alternative to https://github.com/facebook/react/pull/30667.
Basically wrap every section in a `div` with the same class, and only
apply `border-bottom` for every instance, except for the last child. We
are paying some cost by having more divs, but thats more explicit.
When synchronously aborting in a non-async Function Component if you
throw after aborting the task would error rather than abort because
React never observed the AbortSignal.
Using a sigil to throw after aborting during render isn't effective b/c
the user code itself could throw so insteead we just read the request
status. This is ok b/c we don't expect any tasks to still be pending
after the currently running task finishes.
However I found one instance where that wasn't true related to
serializing thenables which I've fixed so we may find other cases. If we
do, though it's almost certainly a bug in our task bookkeeping so we
should just fix it if it comes up.
I also updated `abort` to not set the status to ABORTING unless the
status was OPEN. we don't want to ever leave CLOSED or CLOSING status
