Mirror of upstream PR #34295 #343
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
Differential Revision: D77531947 Pull Request resolved: facebook#33672
Original commit changeset: 65c4dec This was removed by dead code removal. Adding back the TODO with commented out code.
…ents (facebook#33675) We generally treat these types of fields as optional on ReactDebugInfo and should on ReactElement too. That way we can consume prod payloads from third parties.
`react-stack-bottom-frame` -> `react_stack_bottom_frame`. This survives `@babel/plugin-transform-function-name`, but now frames will be displayed as `at Object.react_stack_bottom_frame (...)` in V8. Checks that were relying on exact function name match were updated to use either `.indexOf()` or `.includes()` For backwards compatibility, both React DevTools and Flight Client will look for both options. I am not so sure about the latter and if React version is locked.
) No longer used after facebook#33648
…meline protocol (facebook#33501) The React API is just that we now accept this protocol as an alternative to a native `AnimationTimeline` to be passed to `startGestureTransition`. This is specifically the DOM version. ```js interface CustomTimeline { currentTime: number; animate(animation: Animation): void | (() => void); } ``` Instead, of passing this to the `Animation` that we start to control the View Transition keyframes, we instead inverse the control and pass the `Animation` to this one. It lets any custom implementation drive the updates. It can do so by updating the time every frame or letting it run a time based animation (such as momentum scroll). In this case I added a basic polyfill for `ScrollTimeline` in the example but we'll need a better one.
…ook#33576) View Transitions has this annoying quirk where it adds `width` and `height` to keyframes automatically when generating keyframes even when it's not needed. This causes them to deopt from running on the compositor thread in both Chrome and Safari. @bramus has a [good article on it](https://www.bram.us/2025/02/07/view-transitions-applied-more-performant-view-transition-group-animations/). In React we can automatically rewrite the keyframes when we're starting a View Transition to drop the `width` and `height` from the keyframes when they have the same value and the same value as the pseudo element. To compare it against the pseudo element we first apply the new keyframes without the width/height and then read it back to see if it has changed. For gestures, we have already cancelled the previous animation so we can just read out from that.
…33658) <img width="634" alt="Screenshot 2025-06-27 at 1 13 20 PM" src="https://github.com/user-attachments/assets/dc8c488b-4a23-453f-918f-36b245364934" /> We have to be careful with performance in DEV. It can slow down DX since these are ran whether you're currently running a performance trace or not. It can also show up as misleading since these add time to the "Remaining Effects" entry. I'm not adding all props to the entries. Instead, I'm only adding the changed props after diffing and none for initial mount. I'm trying to as much as possible pick a fast path when possible. I'm also only logging this for the "render" entries and not the effects. If we did something for effects, it would be more like checking with dep changed. This could still have a negative effect on dev performance since we're now also using the slower `performance.measure` API when there's a diff.
…cebook#33659) Stacked on facebook#33658. Unfortunately `console.timeStamp` has the same bug that `performance.measure` used to have where equal start/end times stack in call order instead of reverse call-order. We rely on that in general so we should really switch back all. But there is one case in particular where we always add the same start/time and that's for the "triggers" - Mount/Unmount/Reconnect/Disconnect. Switching to `console.timeStamp` broke this because they now showed below the thing that mounted. After: <img width="726" alt="Screenshot 2025-06-27 at 3 31 16 PM" src="https://github.com/user-attachments/assets/422341c8-bef6-4909-9403-933d76b71508" /> Also fixed a bug where clamped update times could end up logging zero width entries that stacked up on top of each other causing a two row scheduler lane which should always be one row.
…the Promise value (facebook#33662) It's useful to be able to distinguish between different invocations of common helper libraries (like fetch) without having to click through each one. This adds a heuristic to extract a useful description of I/O from the Promise value. We try to find things like getUser(id) -> User where User.id is the id or fetch(url) -> Response where Response.url is the url. For urls we use the filename (or hostname if there is none) as the short name if it can fit. The full url is in the tooltip. <img width="845" alt="Screenshot 2025-06-27 at 7 58 20 PM" src="https://github.com/user-attachments/assets/95f10c08-13a8-449e-97e8-52f0083a65dc" />
Stacked on facebook#33501. This disables the use of ScrollTimeline when detected in Safari in the recommended SwipeRecognizer approach. I'm instead using a polyfill using touch events on iOS. Safari seems set to [release ScrollTimeline soon](https://webkit.org/blog/16993/news-from-wwdc25-web-technology-coming-this-fall-in-safari-26-beta/). Unfortunately it's not really what you'd expect. First of all, [it's not running in sync with the scroll](https://bugs.webkit.org/show_bug.cgi?id=288402) which is kind of its main point. Instead, it is running at 60fps and out of sync with the scroll just like JS. In fact, it is worse than JS because with JS you can at least spawn CSS animations that run at 120fps. So our polyfill can respond to touches at 60fps while gesturing and then run at 120fps upon release. That's better than with ScrollTimeline. Second, [there's a bug which interrupts scrolling if you start a ViewTransition](https://bugs.webkit.org/show_bug.cgi?id=288795) when the element is being removed as part of that. The element can still respond to touches so in a polyfill this isn't an issue. But it essentially makes it useless to use ScrollTimeline with swipe-away gestures. So we're better off in every scenario by not using it. The UA detection is a bit unfortunate. Not sure if there's something more specific but we also had to do a UA detection for Chrome for View Transitions. Those are the only two we have in all of React. 
…nce Track (facebook#33660) Stacked on facebook#33658 and facebook#33659. If we detect that a component is receiving only deeply equal objects, then we highlight it as potentially problematic and worth looking into. <img width="1055" alt="Screenshot 2025-06-27 at 4 15 28 PM" src="https://github.com/user-attachments/assets/e96c6a05-7fff-4fd7-b59a-36ed79f8e609" /> It's fairly conservative and can bail out for a number of reasons: - We only log it on the first parent that triggered this case since other children could be indirect causes. - If children has changed then we bail out since this component will rerender anyway. This means that it won't warn for a lot of cases that receive plain DOM children since the DOM children won't themselves get logged. - If the component's total render time including children is 100ms or less then we skip warning because rerendering might not be a big deal. - We don't warn if you have shallow equality but could memoize the JSX element itself since we don't typically recommend that and React Compiler doesn't do that. It only warns if you have nested objects too. - If the depth of the objects is deeper than like the 3 levels that we print diffs for then we wouldn't warn since we don't know if they were equal (although we might still warn on a child). - If the component had any updates scheduled on itself (e.g. setState) then we don't warn since it would rerender anyway. This should really consider Context updates too but we don't do that atm. Technically you should still memoize the incoming props even if you also had unrelated updates since it could apply to deeper bailouts.
…arty (facebook#33685) If a FlightClient runs inside a FlightServer like fetching from a third party and that logs, then we currently double badge them since we just add on another badge. The issue is that this might be unnecessarily noisy but we also transfer the original format of the current server into the second badge. This extracts our own badge and then adds the environment name as structured data which lets the client decide how to format it. Before: <img width="599" alt="Screenshot 2025-07-02 at 2 30 07 PM" src="https://github.com/user-attachments/assets/4bf26a29-b3a8-4024-8eb9-a3f90dbff97a" /> After: <img width="590" alt="Screenshot 2025-07-02 at 2 32 56 PM" src="https://github.com/user-attachments/assets/f06bbb6d-fbb1-4ae6-b0e3-775849fe3c53" />
…n name (facebook#33697) Follow-up to facebook#33680. Turns out `.getFunctionName` not always returns string.
## Summary Follow-up to facebook#33517. With facebook#33517, we now preserve at least some minimal indent. This actually doesn't work with the current setup, because we don't allow the container to overflow, so basically deeply nested elements will go off the screen. With these changes, we completely change the approach: - The layout will be static and it will have a constant indentation that will always be preserved. - The container will allow overflows, so users will be able to scroll horizontally and vertically. - We will implement automatic horizontal and vertical scrolls, if selected element is not in a viewport. - New: added vertical delimiter that can be used for simpler visual navigation. ## Demo ### Current public release https://github.com/user-attachments/assets/58645d42-c6b8-408b-b76f-95fb272f2e1e ### With facebook#33517 https://github.com/user-attachments/assets/845285c8-5a01-4739-bcd7-ffc089e771bf ### This PR https://github.com/user-attachments/assets/72086b84-8d84-4626-94b3-e22e114e028e
Changes from 6.1.3: * feat: static Components panel layout ([hoxyq](https://github.com/hoxyq) in [facebook#33696](facebook#33696)) * fix: support optionality of structured stack trace function name ([hoxyq](https://github.com/hoxyq) in [facebook#33697](facebook#33697)) * fix: rename bottom stack frame ([hoxyq](https://github.com/hoxyq) in [facebook#33680](facebook#33680))
…acebook#33700) Discovered while testing with Hermes.
…33701) Follow-up to facebook#33652. Don't know how the other were missed. Double-checked that Profiler works in dev mode. Now all hooks start with `!isProfiling` check and return, if true.
Same as 6.1.4, but with 2 hotfixes: * fix: check if profiling for all profiling hooks ([hoxyq](https://github.com/hoxyq) in [facebook#33701](facebook#33701)) * fix: fallback to reading string stack trace when failed ([hoxyq](https://github.com/hoxyq) in [facebook#33700](facebook#33700))
…ok#33708) This delays the abort by splitting the abort into a first step that just flags a task as abort and tracks the time that we aborted. This first step also invokes the `cacheSignal()` abort handler. Then in a macrotask do we finish flushing the abort (or halt). This ensures that any microtasks after the abort signal can finish flushing which may emit rejections or fulfill (e.g. if you try/catch the abort or if it was allSettled). These rejections are themselves signals for which promise was blocked on what promise which forms a graph that we can use for debug info. Notably this doesn't include any additional data in the output since we don't include any data produced after the abort. It just uses the additional execution to collect more debug info. The abort itself might not have been spawned from I/O but it's still interesting to mark Promises that aborted as interesting since they may have been blocked on I/O. So we take the inner most Promise that resolved after the end time (presumably due to the abort signal but also could've just finished after but that's still after the abort). Since the microtasks can spawn new Promises after the ones that reject we ignore any of those that started after the abort.
…ned by then callback (facebook#33713) When a `.then()` callback returns another Promise, there's effectively another "await" on that Promise that happens in the internals but that was not modeled. In effect the Promise returned by `.then()` is blocked on both the original Promise AND the promise returned by the callback. This models that by cloning the original node and treat that as the await on the original Promise. Then we use the existing Node to await the new Promise but its "previous" points to the clone. That way we have a forked node that awaits both. --------- Co-authored-by: Sebastian Sebbie Silbermann <[email protected]>
…ore listed (facebook#33714) These are part of the internals of Promises and async functions even if anonymous functions are otherwise not ignore listed.
If I/O is not awaited in user space in a "previous" path we used to just drop it on the floor. There's a few strategies we could apply here. My first commit just emits it without an await but that would mean we don't have an await stack when there's no I/O in a follow up. I went with a strategy where the "previous" I/O is used only if the "next" didn't have I/O. This may still drop I/O on the floor if there's two back to back within internals for example. It would only log the first one even though the outer await may have started earlier. It may also log deeper in the "next" path if that had user space stacks and then the outer await will appear as if it awaited after. So it's not perfect.
…facebook#33718) When we have a debug channel open that can ask for more objects. That doesn't close until all lazy objects have been explicitly asked for. If you GC an object before the lazy references inside of it before asking for or releasing the objects, then it'll never close. This ensures that if there are no more PendingChunk and no more ResolvedModelChunk then we can close the connection. There's two sources of retaining the Response object. On one side we have a handle to it from the stream coming from the server. On the other side we have a handle to it from ResolvedModelChunk to ask for more data when we lazily parse a model. This PR makes a weak handle from the stream to the Response. However, it keeps a strong reference alive whenever we're waiting on a pending chunk because then the stream might be the root if the only listeners are the callbacks passed to the promise and no references to the promise itself. The pending chunks count can end up being zero even if we might get more data because the references might be inside lazy chunks. In this case the lazy chunks keeps the Response alive. When the lazy chunk gets parsed it can find more chunks that then end up pending to keep the response strongly alive until they resolve.
Same as facebook#33716 but without the separate close signal. We'll need the ref count for separate debug channel anyway but I'm not sure we'll need the separate close signal.
…acebook#33719) Stacked on facebook#33718. Alternative to facebook#33716. The issue with flushing the Server Components track in its current form is that we need to decide how long to wait before flushing whatever we have. That's because the root's end time will be determined by the end time of that last child. However, if a child isn't actually used then we don't necessarily need to include it in the Server Components track since it wasn't blocking the initial render. This waits for 100ms after the last pending chunk is resolved and if nothing is invoking any more lazy initializers after that then we log the Server Components track with the information we have at that point. We also don't eagerly initialize any chunks that wasn't already initialized so if nothing was rendered, then nothing will be logged. This is somewhat an artifact of the current visualization. If we did another transposed form we wouldn't necessarily need to wait until the end and can log things as they're discovered.
Content in Suspense fallbacks are really not considered part of the Suspense but since it does have some behavior it should be marked somehow separately from the Suspense content. A follow up would be to do the same in Fiber.
) Because we sync built artifacts into Meta, we can't support edits from inside www/fbsource to be synced back into OSS as it would cause merge conflicts for future OSS PRs. We have a workflow that should automatically catch and close these PRs, but it looks like this one was missing one permission.
Catching up Flow versions. Since there's plenty new errors, I'm taking each version with breaking changes as a new PR.
`$Call` was removed.
This update remove support for `%checks`. Thanks @SamChou19815 for finding a close replacement that works.
Looks like these versions didn't require changes, so easy fast forward.
…xtension (facebook#34235) This fixes the displaying of "rendered by" section if owner stacks contained any native frames. This regressed after facebook#34185, where we added the Suspense boundary for the StackTraceView. This fails because the Promise that is responsible for symbolication of the source is never getting resolved or rejected. Previously, we would just throw an Error without sending a corresponding message to the `main` script, and it would just cache a Promise that is never resolved, hence the Suspense boundary for "rendered by" section is never resolved. In a separate change, I think we need to update StackTraceView component to display `native` as location, instead of `:0`: <img width="712" height="118" alt="Screenshot 2025-08-20 at 00 20 42" src="https://github.com/user-attachments/assets/c79735c9-fdd2-467c-96cd-2bc29d38c4e0" />
After an easy couple version with facebook#34252, this version is less flexible (and safer) on inferring exported types mainly. We require to annotate some exported types to differentiate between `boolean` and literal `true` types, etc.
Minor new suppressions only.
- 0.261 required to pull out a constant to preserve refinement - 0.259 needed some updated suppressions for hacky stuff
…#34176) NOTE: this is a merged version of @mofeiZ's original PR along with my edits per offline discussion. The description is updated to reflect the latest approach. The key problem we're trying to solve with this PR is to allow developers more control over the compiler's various validations. The idea is to have a number of rules targeting a specific category of issues, such as enforcing immutability of props/state/etc or disallowing access to refs during render. We don't want to have to run the compiler again for every single rule, though, so @mofeiZ added an LRU cache that caches the full compilation output of N most recent files. The first rule to run on a given file will cause it to get cached, and then subsequent rules can pull from the cache, with each rule filtering down to its specific category of errors. For the categories, I went through and assigned a category roughly 1:1 to existing validations, and then used my judgement on some places that felt distinct enough to warrant a separate error. Every error in the compiler now has to supply both a severity (for legacy reasons) and a category (for ESLint). Each category corresponds 1:1 to a ESLint rule definition, so that the set of rules is automatically populated based on the defined categories. Categories include a flag for whether they should be in the recommended set or not. Note that as with the original version of this PR, only eslint-plugin-react-compiler is changed. We still have to update the main lint rule. ## Test Plan * Created a sample project using ESLint v9 and verified that the plugin can be configured correctly and detects errors * Edited `fixtures/eslint-v9` and introduced errors, verified that the w latest config changes in that fixture it correctly detects the errors * In the sample project, confirmed that the LRU caching is correctly caching compiler output, ie compiling files just once. Co-authored-by: Mofei Zhang <[email protected]>
Co-authored-by: Sebastian Sebbie Silbermann <[email protected]>
Co-authored-by: Abdulwahab Omira <[email protected]> Co-authored-by: Sebastian Sebbie Silbermann <[email protected]>
This update was a bit more involved. - `React$Component` was removed, I replaced it with Flow component types. - Flow removed shipping the standard library. This adds the environment libraries back from `flow-typed` which seemed to have changed slightly (probably got more precise and less `any`s). Suppresses some new type errors.
The docs site is in a separate repo, but this gives us a semi-automated way to update the docs about our lint rules. The script generates markdown files from the rule definitions which we can then manually copy/paste into the docs site somewhere. In the future we can automate this fully.
Looks like this version removed `Object.prototype` although I didn't see that in the changelog. This is fine for this code here.
- replace `$ElementType` and `$PropertyType` with `T[K]` accesses. - Use component types
Changes to type inference require some more annotations.
This is the last version before "Natural Inference" change to Flow that will require more changes, so doing a quick fast-forward PR here. - Disabled a new Flow lint against unsafe `Object.assign`.
This version introduces "Natural Inference" which requires a couple more type annotations to make Flow pass.
An exported needed explicit typing as it was inferred incorrectly.
Multiple of these version upgrades required minor additional annotations.
A Flow upgrade removed the bundled library definitinos for SynthaticEvent and we probably want to use our internal definitions. Those are not properly typed at this point yet, but we can look into that as a followup.
…controlled field detection for form reset functionalityAdd controlled field detection for form reset functionalityUpdate ReactDOMFormActions.js This commit enhances the form reset functionality in ReactDOMFormActions.js by: 1. Added isControlledField() helper function that detects if a form field is controlled (has value or checked props from React) 2. Added resetFormFieldsAfterAction() function that resets form fields after successful form action with logic to: - Skip resetting controlled fields (preserving React state management) - Reset uncontrolled fields as before 3. Modified requestFormReset() to use the new controlled/uncontrolled detection logic 4. Exported helper functions for potential external use This ensures that controlled components maintain their state while uncontrolled components are properly reset after form actions.
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#34295