Commit Graph
5862 Commits
Author SHA1 Message Date
Sebastian Markbåge 0123d7c19f [DevTools] Track root instances in a root Map (#30875)
The FiberRoot is a stateful node that can be tracked this way.

This is another step that will let us remove the
`fiberToFiberInstanceMap`.
2024-09-04 16:15:53 -04:00
Andrew Clark e10e868182 Schedule prerender after something suspends (#30800)
Adds the concept of a "prerender". These special renders are spawned
whenever something suspends (and we're not already prerendering).

The purpose is to move speculative rendering work into a separate phase
that does not block the UI from updating. For example, during a
transition, if something suspends, we should not speculatively prerender
siblings that will be replaced by a fallback in the UI until *after* the
fallback has been shown to the user.
2024-09-04 13:55:29 -04:00
Andrew Clark 8b4c54c00f Temporarily disable suspending during work loop (#30762)
### Based on

- #30761 
- #30759 

---

`use` has an optimization where in some cases it can suspend the work
loop during the render phase until the data has resolved, rather than
unwind the stack and lose context. However, the current implementation
is not compatible with sibling prerendering. So I've temporarily
disabled it until the sibling prerendering has been refactored. We will
add it back in a later step.
2024-09-04 12:38:36 -04:00
Sebastian Markbåge d1afcb43fd [DevTools] Track all public HostInstances in a Map (#30831)
This lets us get from a HostInstance to the nearest DevToolsInstance
without relying on `findFiberByHostInstance` and
`fiberToDevToolsInstanceMap`. We already did the equivalent of this for
Resources in HostHoistables.

One issue before was that we'd ideally get away from the
`fiberToDevToolsInstanceMap` map in general since we should ideally not
treat Fibers as stateful but they could be replaced by something else
stateful in principle.

This PR also addresses Virtual Instances. Now you can select a DOM node
and have it select a Virtual Instance if that's the nearest parent since
the parent doesn't have to be a Fiber anymore.

However, the other reason for this change is that I'd like to get rid of
the need for the `findFiberByHostInstance` from being injected. A
renderer should not need to store a reference back from its instance to
a Fiber. Without the Synthetic Event system this wouldn't be needed by
the renderer so we should be able to remove it. We also don't really
need it since we have all the information by just walking the commit to
collect the nodes if we just maintain our own Map.

There's one subtle nuance that the different renderers do. Typically a
HostInstance is the same thing as a PublicInstance in React but
technically in Fabric they're not the same. So we need to translate
between PublicInstance and HostInstance. I just hardcoded the Fabric
implementation of this since it's the only known one that does this but
could feature detect other ones too if necessary. On one hand it's more
resilient to refactors to not rely on injected helpers and on hand it
doesn't follow changes to things like this.

For the conflict resolution I added in #30494 I had to make that
specific to DOM so we can move the DOM traversal to the backend instead
of the injected helper.
2024-09-03 17:28:05 -04:00
Sebastian Markbåge 8d68da3f73 [Fiber] Stash ThenableState on the Dependencies Object for Use By DevTools (#30866)
This lets us track what a Component might suspend on from DevTools. We
could already collect this by replaying a component's Hooks but that
would be expensive to collect from a whole tree.

The thenables themselves might contain useful information but mainly
we'd want access to the `_debugInfo` on the thenables which might
contain additional information from the server.


https://github.com/facebook/react/blob/19bd26beb689e554fceb0b929dc5199be8cba594/packages/shared/ReactTypes.js#L114

In a follow up we should really do something similar in Flight to
transfer `use()` on the debugInfo of that Server Component.
2024-09-03 16:04:24 -04:00
Sebastian Markbåge e0a07e9738 [DevTools] Support VirtualInstances in findAllCurrentHostInstances (#30853)
This lets us highlight Server Components.

However, there is a problem with this because if the actual nearest
Fiber is filtered, there's no FiberInstance and so we might skip past it
and maybe never find a child while walking the whole tree. This is very
common in the case where you have just Server Components and Host
Components which are filtered by default.

Note how the DOM nodes that are just plain host instances without client
component wrappers are not highlighted here:

<img width="1102" alt="Screenshot 2024-08-30 at 4 33 55 PM"
src="https://github.com/user-attachments/assets/c9a7b91e-5faf-4c60-99a8-1195539ff8b5">

Fixing that needs a separate refactor though and related to several
other features that already have a similar issue without
VirtualInstances like Suspense/Error Boundaries (triggering
suspense/error on a filtered Suspense/ErrorBoundary doesn't work
correctly). So this first PR just adds the feature for the common case
where there's at least some Fibers.
2024-09-03 12:29:59 -04:00
Sebastian Markbåge 04ec50efa9 [DevTools] Add Filtering of Environment Names (#30850)
Stacked on #30842.

This adds a filter to be able to exclude Components from a certain
environment. Default to Client or Server.

The available options are computed into a dropdown based on the names
that are currently used on the page (or an option that were previously
used). In addition to the hardcoded "Client". Meaning that if you have
Server Components on the page you see "Server" or "Client" as possible
options but it can be anything if there are multiple RSC environments on
the page.

"Client" in this case means Function and Class Components in Fiber -
excluding built-ins.

If a Server Component has two environments (primary and secondary) then
both have to be filtered to exclude it.

We don't show the option at all if there are no Server Components used
in the page to avoid confusing existing users that are just using Client
Components and wouldn't know the difference between Server vs Client.

<img width="815" alt="Screenshot 2024-08-30 at 12 56 42 AM"
src="https://github.com/user-attachments/assets/e06b225a-e85d-4cdc-8707-d4630fede19e">
2024-09-03 12:29:15 -04:00
Sebastian Markbåge 4f60494156 [Flight] Ship DEV-only enableServerComponentLogs flag in Stable/Canary (#30847)
To recap. This only affects DEV and RSC. It patches console on the
server in DEV (similar to how React DevTools already does and what we
did for the double logging). Then replays those logs with a `[Server]`
badge on the client so you don't need a server terminal open.

This has been on for over 6 months now in our experimental channel and
we've had a lot of coverage in Next.js due to various experimental flags
like taint and ppr.

It's non-invasive in that even if something throws we just serialize
that as an unknown value.

The main feedback we've gotten was:

- The serialization depth wasn't deep enough which I addressed in #30294
and haven't really had any issues since. This could still be an issue or
the inverse that you serialize too many logs that are also too deep.
This is not so much an issue with intentional logging and things like
accidental errors don't typically have unbounded arguments (e.g. React
errors are always string arguments). The ideal would be some way to
retain objects and then load them on-demand but that needs more
plumbing. Which can be later.
- The other was that double logging on the server is annoying if the
same terminal does both the RSC render and SSR render which was
addressed in #30207. It is now off by default in node/edge-builds of the
client, on by default in browser builds. With the `replayConsole` option
to either opt-in or out.

We've reached a good spot now I think.

These are better with `enableOwnerStacks` but that's a separate track
and not needed.

The only thing to document here, other than maybe that we're doing it,
is the `replayConsole` option but that's part of the RSC renderers that
themselves are not documented so nowhere to document it.
2024-08-30 15:11:57 -04:00
Sebastian Markbåge e56f4ae38d [DevTools] Support secondary environment name when it changes (#30842)
We currently support the Environment Name change within a Component.
#29867

If this happens, we give it two HoCs. The problem with this is that we
only show one followed by `+1` in the list.

Before:
<img width="529" alt="Screenshot 2024-08-28 at 6 50 31 PM"
src="https://github.com/user-attachments/assets/c080be72-c254-4d4d-89b6-d1b7f9a9ada8">

After:
<img width="1101" alt="Screenshot 2024-08-28 at 7 16 21 PM"
src="https://github.com/user-attachments/assets/04718674-164b-4255-9cf6-dec9198f12b7">

I could potentially instead badge this case as `A/B` in a single badge.
2024-08-30 10:05:19 -04:00
Ruslan Lesiutin 8308d2f1fe fix[react-devtools/ReactDebugHooks]: support unstable prefixes in hooks and useContextWithBailout (#30837)
Related - https://github.com/facebook/react/pull/30407.

This is experimental-only and FB-only hook. Without these changes,
inspecting an element that is using this hook will throw an error,
because this hook is missing in Dispatcher implementation from React
DevTools, which overrides the original one to build the hook tree.

![Screenshot 2024-08-28 at 18 42
55](https://github.com/user-attachments/assets/e3bccb92-74fb-4e4a-8181-03d13f8512c0)

One nice thing from it is that in case of any potential regressions
related to this experiment, we can quickly triage which implementation
of `useContext` is used by inspecting an element in React DevTools.

Ideally, I should've added some component that is using this hook to
`react-devtools-shell`, so it can be manually tested, but I can't do it
without rewriting the infra for it. This is because this hook is only
available from fb-www builds, and not experimental.
2024-08-30 10:34:52 +01:00
Rune Botten 394e75d9a9 [DevTools] Increase max payload for websocket in standalone app (#30848)
<!--
  Thanks for submitting a pull request!
We appreciate you spending the time to work on these changes. Please
provide enough information so that others can review your pull request.
The three fields below are mandatory.

Before submitting a pull request, please make sure the following is
done:

1. Fork [the repository](https://github.com/facebook/react) and create
your branch from `main`.
  2. Run `yarn` in the repository root.
3. If you've fixed a bug or added code that should be tested, add tests!
4. Ensure the test suite passes (`yarn test`). Tip: `yarn test --watch
TestName` is helpful in development.
5. Run `yarn test --prod` to test in the production environment. It
supports the same options as `yarn test`.
6. If you need a debugger, run `yarn test --debug --watch TestName`,
open `chrome://inspect`, and press "Inspect".
7. Format your code with
[prettier](https://github.com/prettier/prettier) (`yarn prettier`).
8. Make sure your code lints (`yarn lint`). Tip: `yarn linc` to only
check changed files.
  9. Run the [Flow](https://flowtype.org/) type checks (`yarn flow`).
  10. If you haven't already, complete the CLA.

Learn more about contributing:
https://reactjs.org/docs/how-to-contribute.html
-->

## Summary

<!--
Explain the **motivation** for making this change. What existing problem
does the pull request solve?
-->

When debugging applications that are experiencing runaway re-rendering,
it is helpful to profile them in the React Developer Tools.
Unfortunately there is a size limit on the captured profile which can
make them impossible to inspect or save. The limitations I have found
are in `postMessage` for the Chrome extension and in the `ws` websocket
server for the standalone app.

Profiling an app that produces a large profile artifact will simply show
that no profiling data was captured and output an error in the console,
here shown for the standalone app:

```text
standalone.js:92 [React DevTools] Error with websocket connection i {target: H, type: 'error', message: 'Max payload size exceeded', error: RangeError: Max payload size exceeded
    at e.exports.haveLength (/Users/rune/.npm/_npx/8ea6ac5c50…}error: RangeError: Max payload size exceeded
```

This change simply increases the max payload of the websocket server in
the standalone app so that larger profiles may be captured and
inspected.

## How did you test this change?

<!--
Demonstrate the code is solid. Example: The exact commands you ran and
their output, screenshots / videos if the pull request changes the user
interface.
How exactly did you verify that your PR solves the issue you wanted to
solve?
  If you leave this empty, your PR will very likely be closed.
-->

I verified that I could capture and inspect profiling data that
previously exceeded the default limitation for a particular app
2024-08-30 10:34:27 +01:00
Sebastian Markbåge 61739a8a0f [DevTools] Filter Server Components (#30839)
Support filtering Virtual Instances with existing filters.

Server Components are considered "Functions".

In a follow up I'll a new filter for "Environment" which will let you
filter by Client vs Server (and more).
2024-08-29 12:49:30 -04:00
Sebastian Markbåge e33a7233a7 [DevTools] Track virtual instances on the tracked path for selections (#30802)
This appends a (filtered) virtual instance path at the end of the fiber
path. If a virtual instance is selected inside the fiber.

The main part of the path is still just the fiber path since that's the
semantically stateful part. Then we just tack on a few virtual path
frames at the end if we're currently selecting a specific Server
Component within the nearest Fiber.

I also took the opportunity to fix a bug which caused selections inside
Suspense boundaries to not be tracked.
2024-08-29 12:45:03 -04:00
Sebastian Markbåge 18bf7bf500 [DevTools] Remove displayName from inspected data (#30841)
This just clarifies that this is actually unused in the front end. We
use the name from the original instance as the canonical name.
2024-08-29 12:44:48 -04:00
Ruslan Lesiutin a19a8ab44f chore[react-devtools/hook]: remove unused native values (#30827)
Stacked on https://github.com/facebook/react/pull/30826. See [this
commit](https://github.com/facebook/react/pull/30827/commits/ec0e48ed7a47dbbdafb5e2530ccba1f2e5b17bad).

This is unused.
2024-08-29 11:34:31 +01:00
Ruslan Lesiutin 233d63c497 chore[react-devtools/extensions]: remove unused storage permission (#30826)
Stacked on https://github.com/facebook/react/pull/30825. See [this
commit](https://github.com/facebook/react/pull/30826/commits/b2130701cf6b25d7a96c1e92b44f41affa56bb35).

We are not using `storage` anywhere yet, but will be soon. This
permission is not needed.
2024-08-29 11:32:18 +01:00
Ruslan Lesiutin 795b3207ce fix[react-devtools/extensions]: fixed tabs API calls and displaying restricted access popup (#30825)
Stacked on https://github.com/facebook/react/pull/30824. See [this
commit](https://github.com/facebook/react/pull/30825/commits/c9830d64749cf8fd592ea30a1cd65842cf83f6df).

Turns out we should be listing `tabs` in our permissions, if we want to
be able to receive tab url, once its updated.
This also fixes `chrome.tabs.onCreated` event subscription, because [it
should receive only tab
object](https://developer.chrome.com/docs/extensions/reference/api/tabs#event-onCreated),
and not 3 arguments, as expected in the previous implementation.
2024-08-29 11:31:43 +01:00
Ruslan Lesiutin 537c74e16a feat[react-devtools]: support Manifest v3 for Firefox extension (#30824)
Firefox [finally supports
`ExecutionWorld.MAIN`](https://bugzilla.mozilla.org/show_bug.cgi?id=1736575)
in content scripts, which means we can migrate the browser extension to
Manifest V3.

This PR also removes a bunch of no longer required explicit branching
for Firefox case, when we are using Manifest V3-only APIs.

We are also removing XMLHttpRequest injection, which is no longer needed
and restricted in Manifest V3. The new standardized approach (same as in
Chromium) doesn't violate CSP rules, which means that extension can
finally be used for apps running in production mode.
2024-08-29 11:28:35 +01:00
Sebastian Markbåge 96aca5f4f3 Spawn new task if we hit stack overflow (#30419)
If we see the "Maximum call stack size exceeded" error we know we've hit
stack overflow. We can recover from this by spawning a new task and
trying again. Effectively a zero-cost trampoline in the normal case. The
new task will have a clean stack. If you have a lot of siblings at the
same depth that hits the limit you can end up hitting this once for each
sibling but within that new sibling you're unlikely to hit this again.
So it's not too expensive.

If it errors again in the retryTask pass, the other error handling takes
over which causes this to be able to still not infinitely stall. E.g.
when the component itself throws an error like this.

It's still better to increase the stack limit for performance if you
have a really deep tree but it doesn't really hurt to be able to recover
since it's zero cost when it doesn't happen.

We could do the same thing for Flight. Those trees don't tend to be as
deep but could happen.
2024-08-27 13:10:37 -04:00
Sebastian Markbåge f90a6bcc4c [DevTools] Reconcile Fibers Against Previous Children Instances (#30822)
This loops over the remainingReconcilingChildren to find existing
FiberInstances that match the updated Fiber. This is the same thing we
already do for virtual instances. This avoids the need for a
`fiberToFiberInstanceMap`.

This loop is fast but there is a downside when the children set is very
large and gets reordered with keys since we might have to loop over the
set multiple times to get to the instances in the bottom. If that
becomes a problem we can optimize it the same way ReactChildFiber does
which is to create a temporary Map only when the children don't line up
properly. That way everything except the first pass can use the Map but
there's no need to create it eagerly.

Now that we have the loop we don't need the previousSibling field so we
can save some memory there.
2024-08-27 12:05:47 -04:00
Sebastian Markbåge 9690b9ad74 [DevTools] Remove findCurrentFiberUsingSlowPathByFiberInstance (#30818)
We always track the last committed Fiber on `FiberInstance.data`.


https://github.com/facebook/react/blob/dcae56f8b72f625d8affe5729ca9991b31a492ac/packages/react-devtools-shared/src/backend/fiber/renderer.js#L3068

So we can now remove this complex slow path to get the current fiber.
2024-08-27 12:05:24 -04:00
Sebastian Markbåge 1a8f92a869 [DevTools] Track Tree Base Duration of Virtual Instances (#30817)
These don't have their own time since they don't take up any time to
render but they show up in the tree for context. However they never
render themselves. Their base tree time is the base time of their
children. This way they take up the same space as their combined
children in the Profiler tree. (Instead of leaving a blank line which
they did before this PR.)

The frontend doesn't track the difference between a virtual instance and
a Fiber that didn't render this update. This might be a bit confusing as
to why it didn't render. I add the word "client" to make it a bit
clearer and works for both. We should probably have different verbiage
here based on it is a Server Component or something else.

<img width="1103" alt="Screenshot 2024-08-26 at 5 00 47 PM"
src="https://github.com/user-attachments/assets/87b811d4-7024-466a-845d-542493ed3ca2">

I also took the opportunity to remove idToTreeBaseDurationMap and
idToRootMap maps. Cloning the Map isn't really all that super fast
anyway and it means we have to maintain the map continuously as we
render. Instead, we can track it on the instances and then walk the
instances to create a snapshot when starting to profile. This isn't as
fast but really fast too and requires less bookkeeping while rendering
instead which is more sensitive than that one snapshot in the beginning.
2024-08-27 12:05:10 -04:00
Benoit Girard 246d7bfeb0 Enable suspenseCallback on React Native (#29210)
## Summary

suspenseCallback feature has proved to be useful for FB Web. Let's look
at enabling the feature for the React Native build.


## How did you test this change?

Will sync the react changes with a React Native build and will verify
that performance logging is correctly notified of suspense promises
during the suspense callback.
2024-08-26 22:06:02 -04:00
Sebastian Markbåge e44685e4f1 [DevTools] Use Owner Stacks to Implement View Source of a Server Component (#30798)
We don't have the source location of Server Components on the client
because we don't want to eagerly do the throw trick for all Server
Components just in case. Unfortunately Node.js doesn't expose V8's API
to get a source location of a function.

We do have the owner stacks of the JSX that created it though and at
some point we'll also show that location in DevTools.

However, the realization is that if a Server Component is the owner of
any child. The owner stack of that child will have the owner component's
source location as its bottom stack frame.

The technique I'm implementing here is to track whenever a child mounts
we already have its owner. We track the first discovered owned child's
stack on the owner. Then when we ask for a Source location of the owner
do we parse that stack and extract the location of the bottom frame.
This doesn't give us a location necessarily in the top of the function
but somewhere in the function.

In this case the first owned child is the Container:

<img width="1107" alt="Screenshot 2024-08-22 at 10 24 42 PM"
src="https://github.com/user-attachments/assets/95f32850-24a5-4151-8ce6-b7b89db68aee">
<img width="648" alt="Screenshot 2024-08-22 at 10 24 20 PM"
src="https://github.com/user-attachments/assets/4bcba033-866f-4684-9beb-de09d189deff">

We can even use this technique for Fibers too. Currently I use this as a
fallback in case the error technique didn't work. This covers a case
where nothing errors but you still render a child. This case is actually
quite common:

```
function Foo() {
  return <Bar />;
}
```

However, for Fibers we could really just use the `inspect(function)`
technique which works for all cases. At least in Chrome.

Unfortunately, this technique doesn't work if a Component doesn't create
any new JSX but just renders its children. It also currently doesn't
work if the child is filtered since I only look up the owner if an
instance is not filtered. This means that the container in the fixture
can't view source by default since the host component is filtered:

```
export default function Container({children}) {
  return <div>{children}</div>;
}
```

<img width="1107" alt="Screenshot 2024-08-22 at 10 24 35 PM"
src="https://github.com/user-attachments/assets/c3f8f9c5-5add-4d35-9290-3a5079e82adc">
2024-08-26 20:50:43 -04:00
Sebastian Markbåge f65ac7bd4a [DevTools] Make function inspection instant (#30786)
I noticed that there is a delay due to the inspection being split into
one part that gets the attribute and another eval that does the
inspection. This is a bit hacky and uses temporary global names that are
leaky. The timeout was presumably to ensure that the first step had
fully propagated but it's slow. As we've learned, it can be throttled,
and it isn't a guarantee either way.

Instead, we can just consolidate these into a single operation that
by-passes the bridge and goes straight to the renderer interface from
the eval.

I did the same for the viewElementSource helper even though that's not
currently in use since #28471 but I think we probably should return to
that technique when it's available since it's more reliable than the
throw - at least in Chrome. I'm not sure about the status of React
Native here. In Firefox, inspecting a function with source maps doesn't
seem to work. It doesn't jump to original code.
2024-08-26 11:53:17 -04:00
Andrew Clark ee7f6757c4 Fix: Synchronous popstate transitions (#30759)
This is a refactor of the fix in #27505.

When a transition update is scheduled by a popstate event, (i.e. a back/
forward navigation) we attempt to render it synchronously even though
it's a transition, since it's likely the previous page's data is cached.

In #27505, I changed the implementation so that it only "upgrades" the
priority of the transition for a single attempt. If the attempt
suspends, say because the data is not cached after all, from then on it
should be treated as a normal transition.

But it turns out #27505 did not work as intended, because it relied on
marking the root with pending synchronous work (root.pendingLanes),
which was never cleared until the popstate update completed.

The test scenarios I wrote accidentally worked for a different reason
related to suspending the work loop, which I'm currently in the middle
of refactoring.
2024-08-23 12:30:08 -04:00
Sebastian Markbåge eb7570bc16 [Flight] Preserve location of bound server actions (#30778)
Follow up to #30741.

We need to keep the location of the action when it's bound so we can
still jump to it.
2024-08-22 12:36:05 -04:00
Sebastian Markbåge 36c04348d7 [DevTools] Make Functions Clickable to Jump to Definition (#30769)
Currently you can jump to definition of a function by right clicking
through the context menu. However, it's pretty difficult to discover.
This makes the functions clickable to jump to definition - like links.

This uses the same styling as we do for links (which are btw only
clickable if they're not editable). Including cursor: pointer.

I added a background on hover which follows the same pattern as the
owners list.

I also dropped the ƒ prefix when displaying functions. This is a cute
short cut and there's precedence in how Chrome prints functions in the
console *if* the function's toString would've had a function prefix like
if it was a function declaration or expression. It does not do this for
arrow functions or object methods.

Elsewhere in the JS ecosystem this isn't really used anywhere. It
invites more questions than it answers.

The parenthesis and curlies are enough. There's no ambiguity here since
strings have quotations. It looks better with just its object method
form. Keeping it simple seems best. To my eyes this flows better because
I'm used to looking at function syntax but not weird "f"s.

Before:

<img width="433" alt="Screenshot 2024-08-20 at 11 55 09 PM"
src="https://github.com/user-attachments/assets/9dd50da6-598f-4291-9e24-1cdc7200dc9e">


After:
<img width="388" alt="Screenshot 2024-08-20 at 11 46 01 PM"
src="https://github.com/user-attachments/assets/dd988e14-412e-4deb-8c8c-26a54be8331f">


After (Hover):
<img width="389" alt="Screenshot 2024-08-20 at 11 46 31 PM"
src="https://github.com/user-attachments/assets/6fb4ebed-5dc1-448a-8e4d-b6d4f3903329">
2024-08-22 12:35:49 -04:00
Sebastian Markbåge 97e2ce6a00 [Flight] Enable Server Action Source Maps in flight-esm Fixture (#30763)
Stacked on #30758 and #30755.

This is copy paste from #30755 into the ESM package. We use the
`webpack-sources` package for the source map utility but it's not
actually dependent on Webpack itself. Could probably inline it in the
build.
2024-08-22 12:35:16 -04:00
Andrew Clark eb3ad065a1 Feature flag: enableSiblingPrerendering (#30761)
Adds a new feature flag for an upcoming experiment.

No implementation yet.
2024-08-22 10:17:19 -04:00
Sebastian Markbåge 985747f810 [DevTools] Support REACT_LEGACY_ELEMENT_TYPE for formatting JSX (#30779)
DevTools shouldn't use react-is since that's versioned to one version of
React. We don't need to since we use all the symbols from
shared/ReactSymbols anyway and have a fork of typeOf that can cover
both.

Now JSX of old React versions show up with proper JSX formatting when
inspecting.
2024-08-21 18:17:29 -04:00
Josh Story ab24f643d0 [Fizz] use microtasks rather than tasks when scheduling work while prerendering (#30770)
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.
2024-08-21 07:55:56 -07:00
Sebastian Markbåge 1228a28398 Remove turbopack unbundled/register/loader (#30756)
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.
2024-08-21 09:58:31 -04:00
Sebastian Markbåge dd9117e313 [Flight] Source Map Actions in Reference Node Loader Transforms (#30755)
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.
2024-08-21 09:52:17 -04:00
Josh Story dc32c7f35e [Flight] use microtask for scheduling during prerenders (#30768)
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.
2024-08-20 21:43:21 -07:00
Josh Story 85180b8cf8 [Fizz][Static] when aborting a prerender halt unfinished boundaries instead of erroring (#30732)
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.
2024-08-20 13:30:51 -07:00
Josh Story 92d26c8e93 [Flight] When halting omit any reference rather than refer to a shared missing chunk (#30750)
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.
2024-08-20 10:22:39 -07:00
Josh Story 2505bf9b34 [Fizz] track postpones when aborting boundaries with a postpone (#30751)
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.
2024-08-20 09:49:41 -07:00
Josh Story a960b92cb9 [Flight] model halting as never delivered chunks (#30740)
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.
2024-08-19 19:34:20 -07:00
Sebastian Markbåge 0fa9476b9b [Flight] Revert Emit Infinite Promise as a Halted Row (#30746) (#30748)
This reverts commit 52c9c43735.

Just kidding. We realized we probably don't want to do the halted row
thing after all.
2024-08-19 16:34:38 -04:00
Sebastian Markbåge 52c9c43735 [Flight] Emit Infinite Promise as a Halted Row (#30746)
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.
2024-08-19 15:02:41 -04:00
Sebastian Markbåge 591adfa40d [Flight] Rename Chunk constructor to ReactPromise (#30747)
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.
2024-08-19 14:51:22 -04:00
Josh Story 9d082b5500 [Flight] model halted references explicitly (#30731)
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
2024-08-19 11:24:41 -07:00
Sebastian Markbåge 6ebfd5b082 [Flight] Source Map Server Actions to their Server Location (#30741)
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.
2024-08-18 12:31:45 -04:00
Josh Story 7954db9398 [Fizz] handle throwing after abort during render (#30730)
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
2024-08-16 18:29:18 -07:00
Sebastian Markbåge 13ddf1084b [DevTools] Find owners from the parent path that matches the Fiber or ReactComponentInfo (#30717)
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.
2024-08-16 19:52:11 -04:00
Josh Story 7b41cdc093 [Flight][Static] Implement halting a prerender behind enableHalt (#30705)
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
2024-08-16 14:21:57 -07:00
Sam Zhou 85fb95cdff [flow] Eliminate a few React.Element type that will be synced to react-native (#30719)
## 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`
2024-08-16 12:53:52 -04:00
Josh Story fa6eab5854 [Flight] Implement prerender (#30686)
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.
2024-08-15 14:28:28 -07:00
Sebastian Markbåge 19bd26beb6 [Flight/DevTools] Pass the Server Component's "key" as Part of the ReactComponentInfo (#30703)
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.
2024-08-15 11:04:53 -04:00