Mirror of upstream PR #34126 #317
Open
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Technically the async call graph spans basically all the way back to the start of the app potentially, but we don't want to include everything. Similarly we don't want to include everything from previous components in every child component. So we need some heuristics for filtering out data. We roughly want to be able to inspect is what might contribute to a Suspense loading sequence even if it didn't this time e.g. due to a race condition. One flaw with the previous approach was that awaiting a cached promise in a sibling that happened to finish after another sibling would be excluded. However, in a different race condition that might end up being used so I wanted to include an empty "await" in that scenario to have some association from that component. However, for data that resolved fully before the request even started, it's a little different. This can be things that are part of the start up sequence of the app or externally cached data. We decided that this should be excluded because it doesn't contribute to the loading sequence in the expected scenario. I.e. if it's cached. Things that end up being cache misses would still be included. If you want to test externally cached data misses, then it's up to you or the framework to simulate those. E.g. by dropping the cache. This also helps free up some noise since static / cached data can be excluded in visualizations. I also apply this principle to forwarding debug info. If you reuse a cached RSC payload, then the Server Component render time and its awaits gets clamped to the caller as if it has zero render/await time. The I/O entry is still back dated but if it was fully resolved before we started then it's completely excluded.
…allow processing function props (facebook#32119) ## Summary In react-native props that are passed as function get converted to a boolean (`true`). This is the default pattern for event handlers in react-native. However, there are reasons for why you might want to opt-out of this behavior, and instead, pass along the actual function as the prop. Right now, there is no way to do this, and props that are functions always get set to `true`. The `ViewConfig` attributes already have the API for a `process` function. I simply moved the check for the process function up, so if a ViewConfig's prop attribute configured a process function this is always called first. This provides an API to opt out of the default behavior. This is the accompanied PR for react-native: - facebook/react-native#48777 ## 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 modified the code manually in a template react-native app and confirmed its working. This is a code path you only need in very special cases, thus it's a bit hard to provide a test for this. I recorded a video where you can see that the changes are active and the prop is being passed as native value. For this I created a custom native component with a view config that looked like this: ```js const viewConfig = { uiViewClassName: 'CustomView', bubblingEventTypes: {}, directEventTypes: {}, validAttributes: { nativeProp: { process: (nativeProp) => { // Identity function that simply returns the prop function callback // to opt out of this prop being set to `true` as its a function return nativeProp }, }, }, } ``` https://github.com/user-attachments/assets/493534b2-a508-4142-a760-0b1b24419e19 Additionally I made sure that this doesn't conflict with any existing view configs in react native. In general, this shouldn't be a breaking change, as for existing view configs it didn't made a difference if you simply set `myProp: true` or `myProp: { process: () => {...} }` because as soon as it was detected that the prop is a function the config wouldn't be used (which is what this PR fixes). Probably everyone, including the react-native core components use `myProp: true` for callback props, so this change should be fine.
…#33486) The prettier check for this file is currently failing on `main`, after facebook#32119 was merged.
…33450) Summary: useEffectEvent values are not meant to be added to the dep array
This adds some I/O to go get the third party thing to test how it overlaps. With facebook#33482, this is what it looks like. The await gets cut off when the third party component starts rendering. I.e. after the latency to start. <img width="735" alt="Screenshot 2025-06-08 at 5 42 46 PM" src="https://github.com/user-attachments/assets/f68d9a84-05a1-4125-b3f0-8f3e4eaaa5c1" /> This doesn't fully simulate everything because it should actually also simulate each chunk of the stream coming back too. We could wrap the ReadableStream to simulate that. In that scenario, it would probably get some awaits on the chunks at the end too.
…paces (facebook#33409) ## Summary Problem #1: Running the `link-compiler.sh` bash script via `"prebuild"` script fails if a developer has cloned the `react` repo into a folder that contains _any_ spaces. 3 tests fail because of this. <img width="1003" alt="fail-1" src="https://github.com/user-attachments/assets/1fbfa9ce-4f84-48d7-b49c-b6e967b8c7ca" /> <img width="1011" alt="fail-2" src="https://github.com/user-attachments/assets/0a8c6371-a2df-4276-af98-38f4784cf0da" /> <img width="1027" alt="fail-3" src="https://github.com/user-attachments/assets/1c4f4429-800c-4b44-b3da-a59ac85a16b9" /> For example, my current folder is: `/Users/wes/Development/Open Source Contributions/react` The link compiler error returns: `./scripts/react-compiler/link-compiler.sh: line 15: cd: /Users/wes/Development/Open: No such file or directory` Problem #2: 1 test in `ReactChildren-test.js` fails due the existing stack trace regex which should be lightly revised. `([^(\[\n]+)[^\n]*/g` is more robust for stack traces: it captures the function/class name (with dots) and does not break on spaces in file paths. `([\S]+)[^\n]*/g` is simpler but breaks if there are spaces and doesn't handle dotted names well. Additionally, we trim the whitespace off the name to resolve extra spaces breaking this test as well: ``` - in div (at **) + in div (at **) ``` <img width="987" alt="fail-4" src="https://github.com/user-attachments/assets/56a673bc-513f-4458-95b2-224129c77144" /> All of the above tests pass if I hyphenate my local folder: `/Users/wes/Development/Open-Source-Contributions/react` I selfishly want to keep spaces in my folder names. 🫣 ## How did you test this change? **npx yarn prebuild** Before: <img width="896" alt="Screenshot at Jun 01 11-42-56" src="https://github.com/user-attachments/assets/4692775c-1e5c-4851-9bd7-e12ed5455e47" /> After: <img width="420" alt="Screenshot at Jun 01 11-43-42" src="https://github.com/user-attachments/assets/4e303c00-02b7-4540-ba19-927b2d7034fb" /> **npx yarn test** **npx yarn test ./packages/react/src/\_\_tests\_\_/ReactChildren-test.js** **npx yarn test -r=xplat --env=development --variant=true --ci --shard=3/5** Before: <img width="438" alt="before" src="https://github.com/user-attachments/assets/f5eedb22-18c3-4124-a04b-daa95c0f7652" /> After: <img width="439" alt="after" src="https://github.com/user-attachments/assets/a94218ba-7c6a-4f08-85d3-57540e9d0029" /> <img width="650" alt="Screenshot at Jun 02 18-03-39" src="https://github.com/user-attachments/assets/3eae993c-a56b-46c8-ae02-d249cb053fe7" /> <img width="685" alt="Screenshot at Jun 03 12-53-47" src="https://github.com/user-attachments/assets/5b2caa33-d3dc-4804-981d-52cb10b6226f" />
… in shadow dom scnenario (facebook#33487) ## Summary Minor changes around css and styling of Settings dialog. 1. `:root` selector was updated to `:is(:root, :host)` to make css variables available on Shadow Root 2. CSS tweaks around Settings dialog: removed references to deleted styles, removed unused styles, ironed out styling for cases when input styles are enhanced by user agent stylesheet <!-- Explain the **motivation** for making this change. What existing problem does the pull request solve? --> ## How did you test this change? | Before | After | |--------|--------| |  |  | |  |  | |  |  | |  |  |
Includes facebook#31412. The issue is that `pushTreeFork` stores some global state when reconcile children. This gets popped by `popTreeContext` in `completeWork`. Normally `completeWork` returns its own `Fiber` again if it wants to do a second pass which will call `pushTreeFork` again in the next pass. However, `SuspenseList` doesn't return itself, it returns the next child to work on. The fix is to keep track of the count and push it again it when we return the next child to attempt. There are still some outstanding issues with hydration. Like the backwards test still has the wrong behavior in it because it hydrates backwards and so it picks up the DOM nodes in reverse order. `tail="hidden"` also doesn't work correctly. There's also another issue with `useId` and `AsyncIterable` in SuspenseList when there's an unknown number of children. We don't support those showing one at a time yet though so it's not an issue yet. To fix it we need to add variable total count to the `useId` algorithm. E.g. by falling back to varint encoding. --------- Co-authored-by: Rick Hanlon <[email protected]> Co-authored-by: Ricky <[email protected]>
facebook#33482) Previously you weren't guaranteed to have only advancing time entries, you could jump back in time, but now it omits unnecessary duplicates and clamps automatically if you emit a previous time entry to enforce forwards order only. The reason I didn't do this originally is because `await` can jump in the order because we're trying to encode a graph into a flat timeline for simplicity of the protocol and consumers. ```js async function a() { await fetch1(); await fetch2(); } async function b() { await fetch3(); } async function foo() { const p = a(); await b(); return p; } ``` This can effectively create two parallel sequences: ``` --1.................----2.......-- ------3......--------------------- ``` This can now be flattened to either: ``` --1.................3---2.......-- ``` Or: ``` ------3......1......----2.......-- ``` Depending on which one we visit first. Regardless, information is lost. I'd say that the second one is worse encoding of this scenario because it pretends that we weren't waiting for part of the timespan that we were. To solve this I think we should probably make `emitAsyncSequence` create a temporary flat list and then sort it by start time before emitting. Although we weren't actually blocked since there was some CPU time that was able to proceed to get to 3. So maybe the second one is actually better. If we wanted that consistently we'd have to figure out what the intersection was. --------- Co-authored-by: Hendrik Liebau <[email protected]>
The flag is fully rolled out.
…book#33485) Stacked on facebook#33482. There's a flaw with getting information from the execution context of the ping. For the soft-deprecated "throw a promise" technique, this is a bit unreliable because you could in theory throw the same one multiple times. Similarly, a more fundamental flaw with that API is that it doesn't allow for tracking the information of Promises that are already synchronously able to resolve. This stops tracking the async debug info in the case of throwing a Promise and only when you render a Promise. That means some loss of data but we should just warn for throwing a Promise anyway. Instead, this also adds support for tracking `use()`d thenables and forwarding `_debugInfo` from then. This is done by extracting the info from the Promise after the fact instead of in the resolve so that it only happens once at the end after the pings are done. This also supports passing the same Promise in multiple places and tracking the debug info at each location, even if it was already instrumented with a synchronous value by the time of the second use.
…ebook#33507) This matches the change in React 19 to use `<SomeContext>` as the preferred way to provide a context.
…33508) This bug was reported via our wg and appears to only affect values created as a ref. Currently, postfix operators used in a callback gets compiled to: ```js modalId.current = modalId.current + 1; // 1 const id = modalId.current; // 1 return id; ``` which is semantically incorrect. The postfix increment operator should return the value before incrementing. In other words something like this should have been compiled instead: ```js const id = modalId.current; // 0 modalId.current = modalId.current + 1; // 1 return id; ``` This bug does not trigger when the incremented value is a plain primitive, instead there is a TODO bailout.
## Summary This PR adds a 'Layout' selector to the devtools shell main example, as well as a resizable split pane, allowing more realistic testing of how the devtools behaves when used in a vertical or horizontal layout and at different sizes (e.g. when resizing the Chrome Dev Tools pane). ## How did you test this change? https://github.com/user-attachments/assets/81179413-7b46-47a9-bc52-4f7ec414e8be
## Summary The devtools Components tab's component tree view currently has a behavior where the indentation of each level of the tree scales based on the available width of the view. If the view is narrow or component names are long, all indentation showing the hierarchy of the tree scales down with the view width until there is no indentation at all. This makes it impossible to see the nesting of the tree, making the tree view much less useful. With long component names and deep hierarchies this issue is particularly egregious. For comparison, the Chrome Dev Tools Elements panel uses a fixed indentation size, so it doesn't suffer from this issue. This PR adds a minimum pixel value for the indentation width, so that even when the window is narrow some indentation will still be visible, maintaining the visual representation of the component tree hierarchy. Alternatively, we could match the behavior of the Chrome Dev Tools and just use a constant indentation width. ## How did you test this change? - tests (yarn test-build-devtools) - tested in browser: - added an alternate left/right split pane layout to react-devtools-shell to test with (facebook#33516) - tested resizing the tree view in different layout modes ### before this change: https://github.com/user-attachments/assets/470991f1-dc05-473f-a2cb-4f7333f6bae4 with a long component name: https://github.com/user-attachments/assets/1568fc64-c7d7-4659-bfb1-9bfc9592fb9d ### after this change: https://github.com/user-attachments/assets/f60bd7fc-97f6-4680-9656-f0db3d155411 with a long component name: https://github.com/user-attachments/assets/6ac3f58c-42ea-4c5a-9a52-c3b397f37b45
facebook#33525) It may be useful at times to publish only specific packages as an experimental tag. For example, if we need to cherry pick some fixes for an old release, we can first do so by creating that as an experimental release just for that package to allow for quick testing by downstream projects. Similar to .github/workflows/runtime_releases_from_npm_manual.yml I added three options (`dry`, `only_packages`, `skip_packages`) to `runtime_prereleases.yml` which both the manual and nightly workflows reuse. I also added a discord notification when the manual workflow is run.
Previously the experimental workflow relied on the canary one running first to avoid race conditions. However, I didn't account for the fact that the canary one can now be skipped.
…33546) --- [//]: # (BEGIN SAPLING FOOTER) Stack created with [Sapling](https://sapling-scm.com). Best reviewed with [ReviewStack](https://reviewstack.dev/facebook/react/pull/33546). * facebook#33548 * __->__ facebook#33546
As discussed in chat, this is a simple fix to stop introducing labels inside expressions. The useMemo-with-optional test was added in facebook@d70b2c2 and crashes for the same reason- an unexpected label as a value block terminal. --- [//]: # (BEGIN SAPLING FOOTER) Stack created with [Sapling](https://sapling-scm.com). Best reviewed with [ReviewStack](https://reviewstack.dev/facebook/react/pull/33548). * __->__ facebook#33548 * facebook#33546
This was really meant to be there from the beginning. A `cache()`:ed
entry has a life time. On the server this ends when the render finishes.
On the client this ends when the cache of that scope gets refreshed.
When a cache is no longer needed, it should be possible to abort any
outstanding network requests or other resources. That's what
`cacheSignal()` gives you. It returns an `AbortSignal` which aborts when
the cache lifetime is done based on the same execution scope as a
`cache()`ed function - i.e. `AsyncLocalStorage` on the server or the
render scope on the client.
```js
import {cacheSignal} from 'react';
async function Component() {
await fetch(url, { signal: cacheSignal() });
}
```
For `fetch` in particular, a patch should really just do this
automatically for you. But it's useful for other resources like database
connections.
Another reason it's useful to have a `cacheSignal()` is to ignore any
errors that might have triggered from the act of being aborted. This is
just a general useful JavaScript pattern if you have access to a signal:
```js
async function getData(id, signal) {
try {
await queryDatabase(id, { signal });
} catch (x) {
if (!signal.aborted) {
logError(x); // only log if it's a real error and not due to cancellation
}
return null;
}
}
```
This just gets you a convenient way to get to it without drilling
through so a more idiomatic code in React might look something like.
```js
import {cacheSignal} from "react";
async function getData(id) {
try {
await queryDatabase(id);
} catch (x) {
if (!cacheSignal()?.aborted) {
logError(x);
}
return null;
}
}
```
If it's called outside of a React render, we normally treat any cached
functions as uncached. They're not an error call. They can still load
data. It's just not cached. This is not like an aborted signal because
then you couldn't issue any requests. It's also not like an infinite
abort signal because it's not actually cached forever. Therefore,
`cacheSignal()` returns `null` when called outside of a React render
scope.
Notably the `signal` option passed to `renderToReadableStream` in both
SSR (Fizz) and RSC (Flight Server) is not the same instance that comes
out of `cacheSignal()`. If you abort the `signal` passed in, then the
`cacheSignal()` is also aborted with the same reason. However, the
`cacheSignal()` can also get aborted if the render completes
successfully or fatally errors during render - allowing any outstanding
work that wasn't used to clean up. In the future we might also expand on
this to give different
[`TaskSignal`](https://developer.mozilla.org/en-US/docs/Web/API/TaskSignal)
to different scopes to pass different render or network priorities.
On the client version of `"react"` this exposes a noop (both for
Fiber/Fizz) due to `disableClientCache` flag but it's exposed so that
you can write shared code.
This is covered by iife-inline-ternary --- [//]: # (BEGIN SAPLING FOOTER) Stack created with [Sapling](https://sapling-scm.com). Best reviewed with [ReviewStack](https://reviewstack.dev/facebook/react/pull/33572). * facebook#33571 * facebook#33558 * facebook#33547 * facebook#33543 * facebook#33533 * facebook#33532 * facebook#33530 * facebook#33526 * facebook#33522 * facebook#33518 * facebook#33514 * facebook#33513 * facebook#33512 * facebook#33504 * facebook#33500 * facebook#33497 * facebook#33496 * facebook#33495 * facebook#33494 * __->__ facebook#33572
Squashed, review-friendly version of the stack from facebook#33488. This is new version of our mutability and inference model, designed to replace the core algorithm for determining the sets of instructions involved in constructing a given value or set of values. The new model replaces InferReferenceEffects, InferMutableRanges (and all of its subcomponents), and parts of AnalyzeFunctions. The new model does not use per-Place effect values, but in order to make this drop-in the end _result_ of the inference adds these per-Place effects. I'll write up a larger document on the model, first i'm doing some housekeeping to rebase the PR. --- [//]: # (BEGIN SAPLING FOOTER) Stack created with [Sapling](https://sapling-scm.com). Best reviewed with [ReviewStack](https://reviewstack.dev/facebook/react/pull/33494). * facebook#33571 * facebook#33558 * facebook#33547 * facebook#33543 * facebook#33533 * facebook#33532 * facebook#33530 * facebook#33526 * facebook#33522 * facebook#33518 * facebook#33514 * facebook#33513 * facebook#33512 * facebook#33504 * facebook#33500 * facebook#33497 * facebook#33496 * facebook#33495 * __->__ facebook#33494 * facebook#33572
--- [//]: # (BEGIN SAPLING FOOTER) Stack created with [Sapling](https://sapling-scm.com). Best reviewed with [ReviewStack](https://reviewstack.dev/facebook/react/pull/33495). * facebook#33571 * facebook#33558 * facebook#33547 * facebook#33543 * facebook#33533 * facebook#33532 * facebook#33530 * facebook#33526 * facebook#33522 * facebook#33518 * facebook#33514 * facebook#33513 * facebook#33512 * facebook#33504 * facebook#33500 * facebook#33497 * facebook#33496 * __->__ facebook#33495 * facebook#33494 * facebook#33572
--- [//]: # (BEGIN SAPLING FOOTER) Stack created with [Sapling](https://sapling-scm.com). Best reviewed with [ReviewStack](https://reviewstack.dev/facebook/react/pull/33496). * facebook#33571 * facebook#33558 * facebook#33547 * facebook#33543 * facebook#33533 * facebook#33532 * facebook#33530 * facebook#33526 * facebook#33522 * facebook#33518 * facebook#33514 * facebook#33513 * facebook#33512 * facebook#33504 * facebook#33500 * facebook#33497 * __->__ facebook#33496
--- [//]: # (BEGIN SAPLING FOOTER) Stack created with [Sapling](https://sapling-scm.com). Best reviewed with [ReviewStack](https://reviewstack.dev/facebook/react/pull/33497). * facebook#33571 * facebook#33558 * facebook#33547 * facebook#33543 * facebook#33533 * facebook#33532 * facebook#33530 * facebook#33526 * facebook#33522 * facebook#33518 * facebook#33514 * facebook#33513 * facebook#33512 * facebook#33504 * facebook#33500 * __->__ facebook#33497 * facebook#33496
…acebook#33500) AnalyzeFunctions had logic to reset the mutable ranges of context variables after visiting inner function expressions. However, there was a bug in that logic: InferReactiveScopeVariables makes all the identifiers in a scope point to the same mutable range instance. That meant that it was possible for a later function expression to indirectly cause an earlier function expressions' context variables to get a non-zero mutable range. The fix is to not just reset start/end of context var ranges, but assign a new range instance. Thanks for the help on debugging, @mofeiZ! --- [//]: # (BEGIN SAPLING FOOTER) Stack created with [Sapling](https://sapling-scm.com). Best reviewed with [ReviewStack](https://reviewstack.dev/facebook/react/pull/33500). * facebook#33571 * facebook#33558 * facebook#33547 * facebook#33543 * facebook#33533 * facebook#33532 * facebook#33530 * facebook#33526 * facebook#33522 * facebook#33518 * facebook#33514 * facebook#33513 * facebook#33512 * facebook#33504 * __->__ facebook#33500 * facebook#33497 * facebook#33496
…e request byteSize (facebook#34059) Stacked on facebook#34058 When tracking how large the shell is we currently only track the bytes of everything above Suspense boundaries. However since Boundaries that contribute to the preamble will always be inlined when the shell flushes they should also be considered as part of the request byteSize since they always flush alongside the shell. This change adds this tracking
) This is modeling Offscreen boundaries as the thing that unmounts a tree in the frontend. This will let us model this as a "hide" that preserves state instead in a follow up but not yet. By doing it this way, we don't have to special case suspended Suspense boundaries, at least not for the modern versions that use Offscreen as the internal node. It's still special cased for the old React versions. Instead, this is handled by the Offscreen fiber getting hidden. By giving this fiber an FilteredFiberInstance, we also have somewhere to store the children on (separately from the parent children set which can include other siblings too like the loading state). One consequence is that Activity boundary content now disappears when they're hidden which is probably a good thing since otherwise it would be confusing and noisy when it's used to render multiple pages at once.
This was a pretty glaring memory leak. 🙈 I forgot to clean up the VirtualInstances from the id map so the Server Component instances always leaked in DEV.
…rved words (facebook#34080) This currently throws an invariant which may be misleading. I checked the ecma262 spec and used the same list of reserved words in our check. To err on the side of being conservative, we also error when strict mode reserved words are used.
The `waitForReference` call for debug info can trigger inside a different object's initializingHandler. In that case, we can get confused by which one is the root object. We have this special case to detect if the initializing handler's object is `null` and we have an empty string key, then we should replace the root object's value with the resolved value. https://github.com/facebook/react/blob/52612a7cbdd8e1fee9599478247f78725869ebad/packages/react-client/src/ReactFlightClient.js#L1374 However, if the initializing handler actually should have the value `null` then we might get confused by this and replace it with the resolved value from a debug object. This fixes it by just using a non-empty string as the key for the waitForReference on debug value since we're not going to use it anyway. It used to be impossible to get into this state since a `null` value at the root couldn't have any reference inside itself but now the debug info for a `null` value can have outstanding references. However, a better fix might be using a placeholder marker object instead of null or better yet ensuring that we know which root we're initializing in the debug model.
…4047) Fixes remaining issue in facebook#32261, where passing a previously useMemo()-d value to `Object.entries()` makes the compiler think the value is mutated and fail validatePreserveExistingMemo. While I was there I added Object.keys() and Object.values() too. --- [//]: # (BEGIN SAPLING FOOTER) Stack created with [Sapling](https://sapling-scm.com). Best reviewed with [ReviewStack](https://reviewstack.dev/facebook/react/pull/34047). * facebook#34049 * __->__ facebook#34047 * facebook#34044
…ok#34049) We try to merge consecutive reactive scopes that will always invalidate together, but there's one common case that isn't handled. ```js const y = [[x]]; ``` Here we'll create two consecutive scopes for the inner and outer array expressions. Because the input to the second scope is a temporary, they'll merge into one scope. But if we name the inner array, the merging stops: ```js const array = [x]; const y = [array]; ``` This is because the merging logic checks if all the dependencies of the second scope are outputs of the first scope, but doesn't account for renaming due to LoadLocal/StoreLocal. The fix is to track these temporaries. --- [//]: # (BEGIN SAPLING FOOTER) Stack created with [Sapling](https://sapling-scm.com). Best reviewed with [ReviewStack](https://reviewstack.dev/facebook/react/pull/34049). * __->__ facebook#34049 * facebook#34047 * facebook#34044
…ook#33761) <!-- 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 Fixes `await`-ing and returning temporary references in `async` functions. These two operations invoke `.then()` under the hood if it is available, which currently results in an "Cannot access then on the server. You cannot dot into a temporary client reference..." error. This can easily be reproduced by returning a temporary reference from a server function. Fixes facebook#33534 ## How did you test this change? I added a test in a new test file. I wasn't sure where else to put it. <img width="771" height="138" alt="image" src="https://github.com/user-attachments/assets/09ffe6eb-271a-4842-a9fe-c68e17b3fb41" /> <!-- 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. -->
Fixes facebook#33534. `.then` method can be tested when you await a value that's not a Promise. For regular Client References we have a way to mark those as "async" and yield a reference to the unwrapped value in case it's a Promise on the Client. However, the realization is that we never serialize Promises as opaque when passed from the client to the server. If a Promise is passed, then it would've been deserialized as a Promise (while still registered as a temporary reference) and not one of these Proxy objects. Technically it could be a non-function value on the client which would be wrong but you're not supposed to dot into it in the first place. So we can just assume it's `undefined`.
When the element is pre-selected and the Tree component is mounted, right now we are only applying initial vertical offset, but not the horizontal one. Because of this, if the DOM element was selected on Elements panel and then user opens Components panel for the first time of the browser DevTools session, depending on the element's depth, it could be hidden. Similarly to vertical offset, apply horizontal one, but via ref setter. ### Before: https://github.com/user-attachments/assets/0ab3cca9-93c1-4e9e-8d23-88330d438912 ### After: https://github.com/user-attachments/assets/10de153a-1e55-4cf7-b1ff-4cc7cb35ba10
The only thing that uses `memoizedState` as a public API is ClassComponents. Everything else uses it as internals. We shouldn't ever show those internals. Before those internals showed up for example on a suspended Suspense boundary: <img width="436" height="370" alt="Screenshot 2025-08-03 at 8 13 37 PM" src="https://github.com/user-attachments/assets/7fe275a7-d5da-421d-a000-523825916630" />
…4095) This has been bothering me. You can click the arrow and the value to expand/collapse a KeyValue row but not the name. When the name is not editable it should be clickable. Such as when inspecting a Promise value.
…old (facebook#34096) We have two type of links that appear next to each other now. One type of link jumps to a Component instance in the DevTools. The other opens a source location - e.g. in your editor. This clarifies that something will jump to the Component instance by marking it as bold and using angle brackets around the name. This can be seen in the "rendered by" list of owner as well as in the async stack traces when the stack was in a different owner than the one currently selected. <img width="516" height="387" alt="Screenshot 2025-08-03 at 11 27 38 PM" src="https://github.com/user-attachments/assets/5da50262-1e74-4e46-a6f8-96b4c1e4db31" /> The idea is to connect this styling to the owner stacks using `createTask` where this same pattern occurs (albeit the task name is not clickable): <img width="454" height="188" alt="Screenshot 2025-08-03 at 11 23 45 PM" src="https://github.com/user-attachments/assets/81a55c8f-963a-4fda-846a-97f49ef0c469" /> In fact, I was going to add the stack traces to the "rendered by" list to give the ability to jump to the JSX location in the owner stack so that it becomes this same view.
We'll need complete parsing of stack traces for both owner stacks and async debug info so we need to expand the stack parsing capabilities a bit. This refactors the source location extraction to use some helpers we can use for other things too. This is a fork of `ReactFlightStackConfigV8` which also supports DevTools requirements like checking both `react_stack_bottom_frame` and `react-stack-bottom-frame` as well as supporting Firefox stacks. It also supports extracting the first frame of a component stack or the last frame of an owner stack for the source location.
Show the value as "fulfilled: Type" or "rejected: Type" immediately instead of having to expand it twice. We could show all the properties of the object immediately like we do in the Performance Track but it's not always particularly interesting data in the value that isn't already in the header. I also moved it to the end after the stack traces since I think the stack is more interesting but I'm also visually trying to connect the stack trace with the "name" since typically the "name" will come from part of the stack trace. Before: <img width="517" height="433" alt="Screenshot 2025-08-03 at 11 39 49 PM" src="https://github.com/user-attachments/assets/ad28d8a2-c149-4957-a393-20ff3932a819" /> After: <img width="520" height="476" alt="Screenshot 2025-08-03 at 11 58 35 PM" src="https://github.com/user-attachments/assets/53a755b0-bb68-4305-9d16-d6fac7ca4910" />
… out of a subtree (facebook#34082) This searches through the remaining children to see if any of them were children of the bailed out FiberInstance and if so we should reuse them in the new set. It's faster to do this than search through children of the FiberInstance for Suspense boundaries.
We received some bug reports about invariants reported by the compiler in their codebase. Adding them as repros. --- [//]: # (BEGIN SAPLING FOOTER) Stack created with [Sapling](https://sapling-scm.com). Best reviewed with [ReviewStack](https://reviewstack.dev/facebook/react/pull/34099). * facebook#34100 * __->__ facebook#34099
Redo of facebook#32285 which was created with ghstack and is tedious to rebase with sapling.
We moved this logic into InferTypes a long time ago and the PRs to clean it up keep getting lost in the shuffle.
facebook#34104) Instead, we just continue to collect the unfiltered children. --------- Co-authored-by: Sebastian Sebbie Silbermann <[email protected]>
…nstead of Unmounting and Mounting (facebook#34089) Stacked on facebook#34082. This keeps the DevToolsInstance children alive inside Offscreen trees while they're hidden. However, they're sent as unmounted to the front end store. This allows DevTools state to be preserved between these two states. Such as it keeps the "suspended by" set on the SuspenseNode alive since the children are still mounted. So now you when you resuspend, you can see what in the children was suspended. This is useful when you're simulating a suspense but can also be a bit misleading when something suspended for real since it'll only show the previous suspended set and not what is currently suspending it since that hasn't committed yet. SuspenseNodes inside resuspended trees are now kept alive too. That way they can contribute to the timeline even when resuspended. We can choose whether to keep them visible in the rects while hidden or not. In the future we'll also need to add more special cases around Activity. Because right now if SuspenseNodes are kept alive in the Suspense tab UI while hidden, then they're also alive inside Activity that are hidden which maybe we don't want. Maybe simplest would be that they both disappear from the Suspense tab UI but can be considered for the timeline. Another case is that when Activity goes hidden, Fiber will no longer cause its content to suspend the parent but that's not modeled here. So hidden Activity will show up as "suspended by" in a parent Suspense. When they disconnect, they should really be removed from the "suspended by" set of the parent (and perhaps be shown only on the Activity boundary itself).
… different owner (facebook#34101) Stacked on facebook#34094. This shows the I/O stack if available. If it's not available or if it has a different owner (like if it was passed in) then we show the `"awaited at:"` stack below it so you can see where it started and where it was awaited. If it's the same owner this tends to be unnecessary noise. We could maybe be smarter if the stacks are very different then you might want to show both even with the same owner. <img width="517" height="478" alt="Screenshot 2025-08-04 at 11 57 28 AM" src="https://github.com/user-attachments/assets/2dbfbed4-4671-4a5f-8e6e-ebec6fe8a1b7" /> Additionally, this adds an inferred await if there's no owner and no stack for the await. The inferred await of a function/class component is just the owner. No stack. Because the stack trace would be the return value. This will also be the case if you use throw-a-Promise. The inferred await in the child position of a built-in is the JSX location of that await like if you pass a promise to a child. This inference already happens when you pass a Promise from RSC so in this case it already has an await - so this is mainly for client promises.
or end time if they have the same start time. <img width="517" height="411" alt="Screenshot 2025-08-04 at 4 00 23 PM" src="https://github.com/user-attachments/assets/b99be67b-5727-4e24-98c0-ee064fb21e2f" /> They would typically appear in this order naturally but not always. Especially in Suspense boundaries where the order can also be depended on when the components are discovered.
…34094) Stacked on facebook#34093. Instead of using the original `ReactStackTrace` that has the call sites on the server, this parses the `Error` object which has the virtual call sites on the client. We'll need this technique for things stack traces suspending on the client anyway like `use()`. We can then use these callsites to source map in the front end. We currently don't source map function names but might be useful for this use case as well as getting original component names from prod. One thing this doesn't do yet is that it doesn't ignore list the stack traces on the client using the source map's ignore list setting. It's not super important since we expect to have already ignore listed on the server but this will become important for client stack traces like `use()`.
…le (facebook#34090) Stacked on facebook#34089. This measures the client rects of the direct children of Suspense boundaries as we reconcile. This will be used by the Suspense tab to visualize the boundaries given their outlines. We could ask for this more lazily just in case we're currently looking at the Suspense tab. We could also do something like monitor the sizes using a ResizeObserver to cover when they change. However, it should be pretty cheap to this in the reconciliation phase since we're already mostly visiting these nodes on the way down. We have also already done all the layouts at this point since it was part of the commit phase and paint already. So we're just reading cached values in this phase. We can also infer that things are expected to change when parents or sibling changes. Similar technique as ViewTransitions.
all credit on the Flood/ code goes to @mvitousek and @jbrown215, i'm just the one upstreaming it
If you have a ref that the compiler doesn't know is a ref (say, a value returned from a custom hook) and try to assign its `.current = ...`, we currently fail with a generic error that hook return values are not mutable. However, an assignment to `.current` specifically is a very strong hint that the value is likely to be a ref. So in this PR, we track the reason for the mutation and if it ends up being an error, we use it to show an additional hint to the user. See the fixture for an example of the message.
Hints are meant as additional information to present to the developer about an error. The first use-case here is for the suggestion to name refs with "-Ref" if we encounter a mutation that looks like it might be a ref. The original error printing used a second error detail which printed the source code twice, a hint with just extra text is less noisy.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Mirrored from facebook/react PR facebook#34126