[pull] main from facebook:main #1100
Open
+127,701
−58,677
Conversation
This file contains 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
![pull[bot]](https://avatars.githubusercontent.com/in/12910?s=60&v=4){kind=small}
JSX inlining is a prod-only optimization. We want to enforce this while maintaining the same compiler output in DEV and PROD. Here we add a conditional to the transform that only replaces JSX with object literals outside of DEV. Then a later build step can handle DCE based on the value of `__DEV__`
The flag is fully rolled out.
This setting breaks `toMatchInlineSnapshot` by removing whitespace in snapshots.
There's no real reason to keep this around anymore now that the compiler beta is released and we have validated that react-compiler-runtime is [usable by libraries](https://www.npmjs.com/package/react-compiler-runtime?activeTab=dependents). Let's clean this up for now.
We don't actually want the source mapped version of `.stack` from errors because that would cause us to not be able to associate it with a source map in the UIs that need it. The strategy in browsers is more correct where the display is responsible for source maps. That's why we disable any custom `prepareStackTrace` like the ones added by `source-map`. We reset it to `undefined`. However, when running node with `--enable-source-maps` the default for `prepareStackTrace` which is a V8 feature (but may exist elsewhere too like Bun) is a source mapped version of the stack. In those environments we need to reset it to a default implementation that doesn't apply source maps. We already did this in Flight using the `ReactFlightStackConfigV8.js` config. However, we need this more generally in the `shared/ReactComponentStackFrame` implementation. We could always set it to the default implementation instead of `undefined` but that's unnecessary code in browser builds and it might lead to slightly different results. For safety and code size, this PR does it with a fork instead. All builds specific to `node` or `edge` (or `markup` which is a server feature) gets the default implementation where as everything else (e.g. browsers) get `undefined` since it's expected that this is not source mapped. We don't have to do anything about the equivalent in React DevTools since React DevTools doesn't run on the server.
Extends #31066 to ObjectMethods (somehow missed this before). ' --- [//]: # (BEGIN SAPLING FOOTER) Stack created with [Sapling](https://sapling-scm.com). Best reviewed with [ReviewStack](https://reviewstack.dev/facebook/react/pull/31197). * #31204 * #31202 * #31203 * #31201 * #31200 * #31346 * #31199 * #31431 * #31345 * __->__ #31197
`PropertyPathRegistry` is responsible for uniqueing identifier and property paths. This is necessary for the hoistability CFG merging logic which takes unions and intersections of these nodes to determine a basic block's hoistable reads, as a function of its neighbors. We also depend on this to merge optional chained and non-optional chained property paths This fixes a small bug in #31066 in which we create a new registry for nested functions. Now, we use the same registry for a component / hook and all its inner functions ' --- [//]: # (BEGIN SAPLING FOOTER) Stack created with [Sapling](https://sapling-scm.com). Best reviewed with [ReviewStack](https://reviewstack.dev/facebook/react/pull/31345). * #31204 * #31202 * #31203 * #31201 * #31200 * #31346 * #31199 * #31431 * __->__ #31345 * #31197
This bugfix is needed to land #31199 PropagateScopeDepsHIR infers scope declarations for the `inline-jsx-transform` test fixture (the non-hir version does not). These declarations must get the rewritten phi identifiers --- [//]: # (BEGIN SAPLING FOOTER) Stack created with [Sapling](https://sapling-scm.com). Best reviewed with [ReviewStack](https://reviewstack.dev/facebook/react/pull/31431). * #31204 * #31202 * #31203 * #31201 * #31200 * #31346 * #31199 * __->__ #31431 * #31345 * #31197
This is a followup to 6c0f37f to unblock the job.
We were bailing out on complex computed-key syntax (prior to #31344) as we assumed that this caused bugs (due to inferring computed key rvalues to have `freeze` effects). This fixture shows that this bailout is unrelated to the underlying bug
Repro for bug reported by @alexmckenley
Qol improvement. Currently, typescript lints treat test fixtures without an export as a 'global script' (see [docs](https://www.typescriptlang.org/docs/handbook/2/modules.html#how-javascript-modules-are-defined)). This gives confusing lints for duplicate declarations (in the global scope)
Move environment config parsing for `inlineJsxTransform`, `lowerContextAccess`, and some dev-only options out of snap (test fixture). These should now be available for playground via `@inlineJsxTransform` and `lowerContextAccess`. Other small change: Changed zod fields from `nullish()` -> `nullable().default(null)`. [`nullish`](https://zod.dev/?id=nullish) fields accept `null | undefined` and default to `undefined`. We don't distinguish between null and undefined for any of these options, so let's only accept null + default to null. This also makes EnvironmentConfig in the playground more accurate. Previously, some fields just didn't show up as `prettyFormat({field: undefined})` does not print `field`.
…#31362) All dependencies and declarations of a reactive scope can be reordered to scope start/end. i.e. generated code does not depend on conditional short-circuiting logic as dependencies are inferred to have no side effects. Sorting these by name helps us get higher signal compilation snapshot diffs when upgrading the compiler and testing PRs
`enablePropagateScopeDepsHIR` is now used extensively in Meta. This has been tested for over two weeks in our e2e tests and production. The rest of this stack deletes `LoweredFunction.dependencies`, which the non-hir version of `PropagateScopeDeps` depends on. To avoid a more forked HIR (non-hir with dependencies and hir with no dependencies), let's go ahead and clean up the non-hir version of PropagateScopeDepsHIR. Note that all fixture changes in this PR were previously reviewed when they were copied to `propagate-scope-deps-hir-fork`. Will clean up / merge these duplicate fixtures in a later PR ' --- [//]: # (BEGIN SAPLING FOOTER) Stack created with [Sapling](https://sapling-scm.com). Best reviewed with [ReviewStack](https://reviewstack.dev/facebook/react/pull/31199). * #31202 * #31203 * #31201 * #31200 * #31346 * __->__ #31199
…ns (#31346) Recursively collect identifier / property loads and optional chains from inner functions. This PR is in preparation for #31200 Previously, we only did this in `collectHoistablePropertyLoads` to understand hoistable property loads from inner functions. 1. collectTemporariesSidemap 2. collectOptionalChainSidemap 3. collectHoistablePropertyLoads - ^ this recursively calls `collectTemporariesSidemap`, `collectOptionalChainSidemap`, and `collectOptionalChainSidemap` on inner functions 4. collectDependencies Now, we have 1. collectTemporariesSidemap - recursively record identifiers in inner functions. Note that we track all temporaries in the same map as `IdentifierIds` are currently unique across functions 2. collectOptionalChainSidemap - recursively records optional chain sidemaps in inner functions 3. collectHoistablePropertyLoads - (unchanged, except to remove recursive collection of temporaries) 4. collectDependencies - unchanged: to be modified to recursively collect dependencies in next PR ' --- [//]: # (BEGIN SAPLING FOOTER) Stack created with [Sapling](https://sapling-scm.com). Best reviewed with [ReviewStack](https://reviewstack.dev/facebook/react/pull/31346). * #31202 * #31203 * #31201 * #31200 * __->__ #31346 * #31199
We no longer need this production logging.
Previously, we'd directly store the original attributes from the jsx expressions. But this isn't enough as we want to rename duplicate attributes. This PR refactors the prop collection logic to store both the original and new names for jsx attributes in the newly outlined jsx expression. For now, both the new and old names are the same. In the future, they will be different when we add support for outlining expressions with duplicate attribute names.
Previously, we would skip outlining jsx expressions that had duplicate jsx attributes as we would not rename them causing incorrect compilation. In this PR, we add outlining support for duplicate jsx attributes by renaming them.
Previously, we bailed out on outlining jsx that had children that were not part of the outlined jsx. Now, we add support for children by treating as attributes.
## Summary I'm working to get the main `react-native` package parsable by modern Flow tooling (both `flow-bundler`, `flow-api-translator`), and one blocker is legacy `module.exports` syntax. This diff updates files which are [synced to `react-native`](https://github.com/facebook/react-native/tree/main/packages/react-native/Libraries/Renderer/shims) from this repo. ## How did you test this change? Files were pasted into `react-native-github` under fbsource, where Flow validates ✅.
We've previously failed to land this change due to some internal apps seeing infinite render loops due to external store state updates during render. It turns out that since the `renderWasConcurrent` var was moved into the do block, the sync render triggered from the external store check was stuck with a `RootSuspended` `exitStatus`. So this is not unique to sibling prerendering but more generally related to how we handle update to a sync external store during render. We've tested this build against local repros which now render without crashes. We will try to add a unit test to cover the scenario as well. --------- Co-authored-by: Andrew Clark <[email protected]> Co-authored-by: Rick Hanlon <[email protected]>
Going to take some testing to get this right
The playground's compilation mode is currently set to 'all' along with reporting all errors. This tends to be misleading since people usually expect a 1:1 match between how the playground works with what the compiler does in their codebase, eg reactwg/react-compiler#51.
Super minor change to keep our naming scheme consistent for gh workflows
Based off: #31988 <img width="741" alt="Screenshot 2025-01-06 at 12 52 08 AM" src="https://github.com/user-attachments/assets/29b159ca-66d4-441f-8817-dd2db66d1edb" /> it is done
…32005) Disables warnings Webpack DevServer overlay, which is used by React DevTools shell. We are testing against `react-native-web` in this shell, and it installs older versions of the `react-dom` package, and there are some expected discrepancies between it and `react-dom` from source. Before:  After: 
Wait for me to merge, but this has landed everywhere and is ready to remove.
The public API has been deleted a long time ago so this should be unused unless it's used by hacks. It should be replaced with an effect/lifecycle that manually tracks this if you need it. The problem with this API is how the timing implemented because it requires Placement/Hydration flags to be cleared too early. In fact, that's why we also have a separate PlacementDEV flag that works differently. https://github.com/facebook/react/blob/main/packages/react-reconciler/src/ReactFiberCommitWork.js#L2157-L2165 We should be able to remove this code now.
This will provide the opt-in for using [View Transitions](https://developer.mozilla.org/en-US/docs/Web/API/View_Transition_API) in React. View Transitions only trigger for async updates like `startTransition`, `useDeferredValue`, Actions or `<Suspense>` revealing from fallback to content. Synchronous updates provide an opt-out but also guarantee that they commit immediately which View Transitions can't. There's no need to opt-in to View Transitions at the "cause" side like event handlers or actions. They don't know what UI will change and whether that has an animated transition described. Conceptually the `<ViewTransition>` component is like a DOM fragment that transitions its children in its own isolate/snapshot. The API works by wrapping a DOM node or inner component: ```js import {ViewTransition} from 'react'; <ViewTransition><Component /></ViewTransition> ``` The default is `name="auto"` which will automatically assign a `view-transition-name` to the inner DOM node. That way you can add a View Transition to a Component without controlling its DOM nodes styling otherwise. A difference between this and the browser's built-in `view-transition-name: auto` is that switching the DOM nodes within the `<ViewTransition>` component preserves the same name so this example cross-fades between the DOM nodes instead of causing an exit and enter: ```js <ViewTransition>{condition ? <ComponentA /> : <ComponentB />}</ViewTransition> ``` This becomes especially useful with `<Suspense>` as this example cross-fades between Skeleton and Content: ```js <ViewTransition> <Suspense fallback={<Skeleton />}> <Content /> </Suspense> </ViewTransition> ``` Where as this example triggers an exit of the Skeleton and an enter of the Content: ```js <Suspense fallback={<ViewTransition><Skeleton /></ViewTransition>}> <ViewTransition><Content /></ViewTransition> </Suspense> ``` Managing instances and keys becomes extra important. You can also specify an explicit `name` property for example for animating the same conceptual item from one page onto another. However, best practices is to property namespace these since they can easily collide. It's also useful to add an `id` to it if available. ```js <ViewTransition name="my-shared-view"> ``` The model in general is the same as plain `view-transition-name` except React manages a set of heuristics for when to apply it. A problem with the naive View Transitions model is that it overly opts in every boundary that *might* transition into transitioning. This is leads to unfortunate effects like things floating around when unrelated updates happen. This leads the whole document to animate which means that nothing is clickable in the meantime. It makes it not useful for smaller and more local transitions. Best practice is to add `view-transition-name` only right before you're about to need to animate the thing. This is tricky to manage globally on complex apps and is not compositional. Instead we let React manage when a `<ViewTransition>` "activates" and add/remove the `view-transition-name`. This is also when React calls `startViewTransition` behind the scenes while it mutates the DOM. I've come up with a number of heuristics that I think will make a lot easier to coordinate this. The principle is that only if something that updates that particular boundary do we activate it. I hope that one day maybe browsers will have something like these built-in and we can remove our implementation. A `<ViewTransition>` only activates if: - If a mounted Component renders a `<ViewTransition>` within it outside the first DOM node, and it is within the viewport, then that ViewTransition activates as an "enter" animation. This avoids inner "enter" animations trigger when the parent mounts. - If an unmounted Component had a `<ViewTransition>` within it outside the first DOM node, and it was within the viewport, then that ViewTransition activates as an "exit" animation. This avoids inner "exit" animations triggering when the parent unmounts. - If an explicitly named `<ViewTransition name="...">` is deep within an unmounted tree and one with the same name appears in a mounted tree at the same time, then both are activated as a pair, but only if they're both in the viewport. This avoids these triggering "enter" or "exit" animations when going between parents that don't have a pair. - If an already mounted `<ViewTransition>` is visible and a DOM mutation, that might affect how it's painted, happens within its children but outside any nested `<ViewTransition>`. This allows it to "cross-fade" between its updates. - If an already mounted `<ViewTransition>` resizes or moves as the result of direct DOM nodes siblings changing or moving around. This allows insertion, deletion and reorders into a list to animate all children. It is only within one DOM node though, to avoid unrelated changes in the parent to trigger this. If an item is outside the viewport before and after, then it's skipped to avoid things flying across the screen. - If a `<ViewTransition>` boundary changes size, due to a DOM mutation within it, then the parent activates (or the root document if there are no more parents). This ensures that the container can cross-fade to avoid abrupt relayout. This can be avoided by using absolutely positioned children. When this can avoid bubbling to the root document, whatever is not animating is still responsive to clicks during the transition. Conceptually each DOM node has its own default that activates the parent `<ViewTransition>` or no transition if the parent is the root. That means that if you add a DOM node like `<div><ViewTransition><Component /></ViewTransition></div>` this won't trigger an "enter" animation since it was the div that was added, not the ViewTransition. Instead, it might cause a cross-fade of the parent ViewTransition or no transition if it had no parent. This ensures that only explicit boundaries perform coarse animations instead of every single node which is really the benefit of the View Transitions model. This ends up working out well for simple cases like switching between two pages immediately while transitioning one floating item that appears on both pages. Because only the floating item transitions by default. Note that it's possible to add manual `view-transition-name` with CSS or `style={{ viewTransitionName: 'auto' }}` that always transitions as long as something else has a `<ViewTransition>` that activates. For example a `<ViewTransition>` can wrap a whole page for a cross-fade but inside of it an explicit name can be added to something to ensure it animates as a move when something relates else changes its layout. Instead of just cross-fading it along with the Page which would be the default. There's more PRs coming with some optimizations, fixes and expanded APIs. This first PR explores the above core heuristic. --------- Co-authored-by: Sebastian "Sebbie" Silbermann <[email protected]>
…lass (#31999) Stacked on #31975. This is the primary way we recommend styling your View Transitions since it allows for reusable styling such as a CSS library specializing in View Transitions in a way that's composable and without naming conflicts. E.g. ```js <ViewTransition className="enter-slide-in exit-fade-out update-cross-fade"> ``` This doesn't change the HTML `class` attribute. It's not a CSS class. Instead it assign the `view-transition-class` style prop of the underlying DOM node while it's transitioning. You can also just use `<div style={{viewTransitionClass: ...}}>` on the DOM node but it's convenient to control the Transition completely from the outside and conceptually we're transitioning the whole fragment. You can even make Transition components that just wraps existing components. `<RevealTransition><Component /></RevealTransition>` this way. Since you can also have multiple wrappers for different circumstances it allows React's heuristics to use different classes for different scenarios. We'll likely add more options like configuring different classes for different `types` or scenarios that can't be described by CSS alone. ## CSS Modules ```js import transitions from './transitions.module.css'; <ViewTransition className={transitions.bounceIn}>...</ViewTransition> ``` CSS Modules works well with this strategy because you can have globally unique namespaces and define your transitions in the CSS modules as a library that you can import. [As seen in the fixture here.](8b91b37#diff-b4d9854171ffdac4d2c01be92a5eff4f8e9e761e6af953094f99ca243b054a85R11) I did notice an unfortunate bug in how CSS Modules (at least in Webpack) generates class names. Sometimes the `+` character is used in the hash of the class name which is not valid for `view-transition-class` and so it breaks. I had to rename my class names until the hash yielded something different to work around it. Ideally that bug gets fixed soon. ## className, rly? `className` isn't exactly the most loved property name, however, I'm using `className` here too for consistency. Even though in this case there's no direct equivalent DOM property name. The CSS property is named `viewTransitionClass`, but the "viewTransition" prefix is implied by the Component it is on in this case. For most people the fact that this is actually a different namespace than other CSS classes doesn't matter. You'll most just use a CSS library anyway and conceptually you're just assigning classes the same way as `className` on a DOM node. But if we ever rename the `class` prop then we can do that for this one as well.
Stacked on #31975. We're going to recommend that the primary way you style a View Transition is using a View Transition Class (and/or Type). These are only available in the View Transitions v2 spec. When they're not available it's better to fallback to just not animating instead of animating with the wrong styling rules applied. This is already widely supported in Chrome and Safari 18.2. Safari 18.2 usage is still somewhat low but it's rolling out quickly as we speak. A way to detect this is by just passing the object form to `startViewTransition` which throws if it's an earlier version. The object form is required for `types` but luckily classes rolled out at the same time. Therefore we're only indirectly detecting class support. This means that in practice Safari 18.0 and 18.1 won't animate. We could try to only apply the feature detection if you're actually using classes or types, but that would create an unfortunate ecosystem burden to try to support names. It also leads to flaky effects when only some animations work. Better to just disable them all. Firefox has yet to ship anything. We'll have to look out for how the feature detection happens there and if they roll things out in different order but if you ship late, you deal with web compat as the ball lies.
…tion to finish (#32002) Stacked on #31975. View Transitions cannot handle interruptions in that if you start a new one before the previous one has finished, it just stops and then restarts. It doesn't seamlessly transition into the new transition. This is generally considered a bad thing but I actually think it's quite good for fire-and-forget animations (gestures is another story). There are too many examples of bad animations in fast interactions because the scenario wasn't predicted. Like overlapping toasts or stacked layers that look bad. The only case interrupts tend to work well is when you do a strict reversal of an animation like returning to the page you just left or exiting a modal just being opened. However, we're limited by the platform even in that regard. I think one reason interruptions have traditionally been seen as good is because it's hard if you have a synchronous framework to not interrupt since your application state has already moved on. We don't have that limitation since we can suspend commits. We can do all the work to prepare for the next commit by rendering while the animation is going but then delay the commit until the previous one finishes. Another technical limitation earlier animation libraries suffered from is only have the option to either interrupt or sequence animations since it's modeling just one change set. Like showing one toast at a time. That's bad. We don't have that limitation because we can interrupt a previously suspended commit and start working on a new one instead. That's what we do for suspended transitions in general. The net effect is that we batch the commits. Therefore if you get multiple toasts flying in fast, they can animate as a batch in together all at once instead of overlapping slightly or being staggered. Interruptions (often) bad. Staggered animations bad. Batched animations good. This PR stashes the currently active View Transition with an expando on the container that's animating (currently always document). This is similar to what we do with event handlers etc. We reason we do this with an expando is that if you have multiple Reacts on the same page they need to wait for each other. However, one of those might also be the SSR runtime. So this lets us wait for the SSR runtime's animations to finish before starting client ones. This could really be a more generic name since this should ideally be shared across frameworks. It's kind of strange that this property doesn't already exist in the DOM given that there can only be one. It would be useful to be able to coordinate this across libraries.
This adds navigation support to the View Transition fixture using both `history.pushState/popstate` and the Navigation API models. Because `popstate` does scroll restoration synchronously at the end of the event, but `startViewTransition` cannot start synchronously, it would observe the "old" state as after applying scroll restoration. This leads to weird artifacts. So we intentionally do not support View Transitions in `popstate`. If it suspends anyway for some other reason, then scroll restoration is broken anyway and then it is supported. We don't have to do anything here because this is already how things worked because the sync `popstate` special case already included the sync lane which opts it out of View Transitions. For the Navigation API, scroll restoration can be blocked. The best way to do this is to resolve the Navigation API promise after React has applied its mutation. We can detect if there's currently any pending navigation and wait to resolve the `startViewTransition` until it finishes and any scroll restoration has been applied. https://github.com/user-attachments/assets/f53b3282-6315-4513-b3d6-b8981d66964e There is a subtle thing here. If we read the viewport metrics before scroll restoration has been applied, then we might assume something is or isn't going to be within the viewport incorrectly. This is evident on the "Slide In from Left" example. When we're going forward to that page we shift the scroll position such that it's going to appear in the viewport. If we did this before applying scroll restoration, it would not animate because it wasn't in the viewport then. Therefore, we need to run the after mutation phase after scroll restoration. A consequence of this is that you have to resolve Navigation in `useInsertionEffect` as otherwise it leads to a deadlock (which eventually gets broken by `startViewTransition`'s timeout of 10 seconds). Another consequence is that now `useLayoutEffect` observes the restored state. However, I think what we'll likely do is move the layout phase to before the after mutation phase which also ensures that auto-scrolling inside `useLayoutEffect` are considered in the viewport measurements as well.
…on (#32029) This allows mutations and scrolling in the layout phase to be counted towards the mutation. This would maybe not be the case for gestures but it is useful for fire-and-forget. This also avoids the issue that if you resolve navigation in useLayoutEffect that it ends up dead locked. It also means that useLayoutEffect does not observe the scroll restoration and in fact, the scroll restoration would win over any manual scrolling in layout effects. For better or worse, this is more in line with how things worked before and how it works in popstate. So it's less of a breaking change. This does mean that we can't unify the after mutation phase with the layout phase though. To do this we need split out flushSpawnedWork from the flushLayoutEffect call. Spawned work from setState inside the layout phase is done outside and not counted towards the transition. They're sync updates and so are not eligible for their own View Transitions. It's also tricky to support this since it's unclear what things like exits in that update would mean. This work will still be able to mutate the live DOM but it's just not eligible to trigger new transitions or adjust the target of those. One difference between popstate is that this spawned work is after scroll restoration. So any scrolling spawned from a second pass would now win over scroll restoration. Another consequence of this change is that you can't safely animate pseudo elements in useLayoutEffect. We'll introduce a better event for that anyway.
Fonts flickering in while loading can be disturbing to any transition but especially View Transitions. Even if they don't cause layout thrash - the paint thrash is bad enough. We might add Suspensey fonts to all Transitions in the future but it's especially a no-brainer for View Transitions. We need to apply mutations to the DOM first to know whether that will trigger new fonts to load. For general Suspensey fonts, we'd have to revert the commit by applying mutations in reverse to return to the previous state. For View Transitions, since a snapshot is already frozen, we can freeze the screen while we're waiting for the font at no extra cost. It does mean that the page isn't responsive during this time but we should only block this for a short period anyway. The timeout needs to be short enough that it doesn't cause too much of an issue when it's a new load and slow, yet long enough that you have a chance to load it. Otherwise we wait for no reason. The assumption here is that you likely have either cached the font or preloaded it earlier - or you're on an extremely fast connection. This case is for optimizing the high end experience. Before: https://github.com/user-attachments/assets/e0acfffe-fa49-40d6-82c3-5b08760175fb After: https://github.com/user-attachments/assets/615a03d3-9d6b-4eb1-8bd5-182c4c37a628 Note that since the Navigation is blocked on the font now the browser spinner shows up while the font is loading.
## Summary Callers for this method has been removed in facebook/react-native@65bda54, so these methods no longer need to be conditionally exported and the feature flag can be removed. ## How did you test this change? Flow fabric/native
Just adding the name so it shows up. (Note that no experimental ones are added to the list of filters atm. Including SuspenseList etc.)
…tests (#32012) - Adds @compilationMode(all|infer|syntax|annotation) and @panicMode(none) directives. This is now shared with our test infra - Playground still defaults to `infer` mode while tests default to `all` mode - See added fixture tests
Feature was added in #31577, lets enable it by default. Note: for gradual rollout with React Native, we will continue to emit different event, requires some changes on React Native side to support this. I have plans to make this feature to be accessible via browser context menu, which has really limited API. In order to minimize potential divergence, lets make this the default state for the feature.
… code (#32015) In this PR: 1. Removed unused code in `Tree.js` 2. Removed logic for pre-selecting first element in the tree by default. This is a bit clowny, because it steals focus and resets scroll, when user attempts to expand / collapse some subtree. 3. Updated comments around 1c381c5. To expand on 3-rd point, for someone who might be reading this in the future: We can't guarantee focus of RDT browser extension panels, because they are hosted in an `iframe`. Attempting to fire any events won't have any result, user action with the corresponding `iframe` is required in order for this `iframe` to obtain focus. The only reason why built-in Elements panel in Chrome works correctly is because it is supported natively somewhere in Chrome / Chrome DevTools. Also, when you select an element on the application page, Chrome will make sure that Elements panel opened, which technically guarantees focus inside DevTools window and Elements panel subview. As of today, we can't navigate user to third-party extensions panels, there is no API for this, hence no ability to guarantee focused RDT panels.
Related: #31342 This fixes RDT behaviour when some DOM element was pre-selected in built-in browser's Elements panel, and then Components panel of React DevTools was opened for the first time. With this change, React DevTools will correctly display the initial state of the Components Tree with the corresponding React Element (if possible) pre-selected. Previously, we would only subscribe listener when `TreeContext` is mounted, but this only happens when user opens one of React DevTools panels for the first time. With this change, we keep state inside `Store`, which is created when Browser DevTools are opened. Later, `TreeContext` will use it for initial state value. Planned next changes: 1. Merge `inspectedElementID` and `selectedElementID`, I have no idea why we need both. 2. Fix issue with `AutoSizer` rendering a blank container.
Stacked on #31892, see commit on top. For some reason, there were 2 fields different fields for essentially same thing: `selectedElementID` and `inspectedElementID`. Basically, the change is: ``` selectedElementID -> inspectedElementID selectedElementIndex -> inspectedElementIndex ``` I have a theory that it was due to previously used async approach around element inspection, and the whole `InspectedElementView` was wrapped in `Suspense`.
…nt index is set (#31968) Stacked on #31956. See [commit on top](ecb8df4). Use `initialScrollOffset` prop for `FixedSizeList` from `react-window`. This happens when user selects an element in built-in Elements panel in DevTools, and then opens Components panel from React DevTools - elements will be synced and corresponding React Element will be pre-selected, we just have to scroll to its position now.
Stacked on #31968. See commit on top. Fixes an issue with bank tree view, when we are scrolling to an item while syncing user DOM selection. This should only have an effect on browser extension. Added events with `extension` prefix will only be emitted in browser extension implementation, for other implementations `useExtensionComponentsPanelVisibility` will return constant `true` value. Before: https://github.com/user-attachments/assets/82667c16-d495-4346-af0a-7ed22ff89cfc After: https://github.com/user-attachments/assets/a5d223fd-0328-44f0-af68-5c3863f1efee
Parity with appendChild and insertBefore. This allows reordering at the root while preserving state.
This adds refs to View Transition that can resolve to an instance of:
```js
type ViewTransitionRef = {
name: string,
group: Animatable,
imagePair: Animatable,
old: Animatable,
new: Animatable,
}
```
Animatable is a type that has `animate(keyframes, options)` and
`getAnimations()` on it. It's the interface that exists on Element that
lets you start animations on it. These ones are like that but for the
four pseudo-elements created by the view transition.
If a name changes, then a new ref is created. That way if you hold onto
a ref during an exit animation spawned by the name change, you can keep
calling functions on it. It will keep referring to the old name rather
than the new name.
This allows imperative control over the animations instead of using CSS
for this.
```js
const viewTransition = ref.current;
const groupAnimation = viewTransition.group.animate(keyframes, options);
const imagePairAnimation = viewTransition.imagePair.animate(keyframes, options);
const oldAnimation = viewTransition.old.animate(keyframes, options);
const newAnimation = viewTransition.new.animate(keyframes, options);
```
The downside of using this API is that it doesn't work with SSR so for
SSR rendered animations they'll fallback to the CSS. You could use this
for progressive enhancement though.
Note: In this PR the ref only controls one DOM node child but there can
be more than one DOM node in the ViewTransition fragment and they are
just left to their defaults. We could try something like making the
`animate()` function apply to multiple children but that could lead to
some weird consequences and the return value would be difficult to
merge. We could try to maintain an array of Animatable that updates with
how ever many things are currently animating but that makes the API more
complicated to use for the simple case. Conceptually this should be like
a fragment so we would ideally combine the multiple children into a
single isolate if we could. Maybe one day the same name could be applied
to multiple children to create a single isolate. For now I think I'll
just leave it like this and you're really expect to just use it with one
DOM node. If you have more than one they just get the default animations
from CSS.
Using this is a little tricky due timing. In this fixture I just use a
layout effect plus rAF to get into the right timing after the
startViewTransition is ready. In the future I'll add an event that fires
when View Transitions heuristics fire with the right timing.
This adds five events to `<ViewTransition>` that triggers when React
wants to animate it.
- `onEnter`: The `<ViewTransition>` or its parent Component is mounted
and there's no other `<ViewTransition>` with the same name being
deleted.
- `onExit`: The `<ViewTransition>` or its parent Component is unmounted
and there's no other `<ViewTransition>` with the same name being
deleted.
- `onLayout`: There are no updates to the content inside this
`<ViewTransition>` boundary itself but the boundary has resized or moved
due to other changes to siblings.
- `onShare`: This `<ViewTransition>` is being mounted and another
`<ViewTransition>` instance with the same name is being unmounted
elsewhere.
- `onUpdate`: The content of `<ViewTransition>` has changed either due
to DOM mutations or because an inner child `<ViewTransition>` has
resized.
Only one of these events is fired per Transition. If you want to cover
all updates you have to listen to `onLayout`, `onShare` and `onUpdate`.
We could potentially do something like fire `onUpdate` if `onLayout` or
`onShare` isn't specified but it's a little sketchy to have behavior
based on if someone is listening since it limits adding wrappers that
may or may not need it.
Each takes a `ViewTransitionInstance` as an argument so you don't need a
ref to animate it.
```js
<ViewTransition onEnter={inst => inst.new.animate(keyframes, options)}>
```
The timing of this event is after the View Transition's `ready` state
which means that's too late to do any changes to the View Transition's
snapshots but now both the new and old pseudo-elements are ready to
animate.
The order of `onExit` is parent first, where as the others are child
first. This mimics effect mount/unmount.
I implement this by adding to a queue in the commit phase and then call
it while we're finishing up the commit. This is after layout effects but
before passive effects since passive effects fire after the animation is
`finished`.
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.
See Commits and Changes for more details.
Created by
pull[bot]
Can you help keep this open source service alive? 💖 Please sponsor : )