forked from facebook/react
-
Notifications
You must be signed in to change notification settings - Fork 0
Mirror of upstream PR #34715 #481
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Open
kushxg
wants to merge
643
commits into
main
Choose a base branch
from
upstream-pr-34715
base: main
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
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
…ook#34228) While we still use this package internally, we now ask users to install eslint-plugin-react-hooks instead, so this package can now be deprecated on npm.
…t paint (facebook#34230) Before the first rAF, we don't know if there has been other paints before this and if so when. (We could get from performance observer.) We can assume that it's not earlier than 0 so we used delay up until the throttle time starting from zero but if the first paint is about to happen that can be very soon after. Instead, this reveals it during the next paint which should let us be able to get into the first paint. If we can trust `rel="expect"` to have done its thing we should schedule our raf before first paint but ofc browsers can cheat and paint earlier if they want to. If we're wrong, this is at least more batched than doing it synchronously. However it will mean that things might get more flashy than it should be if it would've been throttled. An alternative would be to always throttle first reveal.
…ited in anonymous functions (facebook#34234)
…#34236) When a debug channel is used between the Flight server and a browser Flight client, we want to allow the same RSC stream to be used for server-side rendering. To support this, the Edge and Node Flight clients also need to accept a `debugChannel` option. Without it, debug information would be missing (e.g. for SSR error stacks), and in some cases this could result in `Connection closed` errors. This PR adds support for the `debugChannel` option in the Edge and Node clients for ESM, Parcel, Turbopack, and Webpack. Unlike the browser clients, these clients only support a one-way channel, since the Flight server’s return protocol is not designed for multiple clients. The implementation follows the approach used in the browser clients, but excludes the writable parts.
) 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.
Co-authored-by: Jack Pope <[email protected]> Co-authored-by: Rick Hanlon <[email protected]>
Fixes a bug where insertion effects were not cleaned up if a hidden Activity is unmounted.
The canaries have been published depending on 0.27-canary. Bumping scheduler just in case to be sure.
…ebook#34672) Follow up to facebook#34649. This adds the compiler rules back so they can be opted-in 6.1.0, but aren't included in the presets as that would be a breaking change.
This change allows it so that tabs that were open before a compiler error are automatically opened again when the error is resolved. Quality of life change for those especially working with the advanced view of the playground. https://github.com/user-attachments/assets/cd2dc117-e6fc-4f57-a08f-259757c4f5e8
The View Transition docs were unclear about this but apparently the `finished` promise never settles if the animation never started. So if there's an error that rejects the `ready` promise, we'll never run the clean up which can cause it to stall. Fixes facebook#34662. However, ultimately that is caused by Chrome stalling our default `onDefaultTransitionIndicator` but it should be unblocked after 10 seconds, not a minute.
…ine profiler is not supported (facebook#34684)
…ebook#34503) The `@enablePreserveExistingMemoizationGuarantees` mode can still fail to preserve manual memoization due to mismtached dependencies. Specifically, where the user's dependencies are more precise than the compiler infers bc the compiler is being conservative about what might be nullable. In this mode though we're intentionally using information from the manual memoization and can also rely on the deps as a signal for what's non-nullable. The idea of the PR is that we treat manual memo deps just like other inferred-as-non-nullable objects during PropagateScopeDeps. We're careful to not treat the full path as non-nullable, only up to the last property index. So `x.y.z` as a manual dep treats `x` and `x.y` as non-nullable, allowing us to preserve a conditional dependency on `x.y.z`. Optionals within manual dependencies are a bit trickier and aren't handled yet, but hopefully that's less common and something we can improve in a follow-up. Not handling them just means that developers may hit false positives on validating existing memoization if they use optional chains in manual dependencies. --- [//]: # (BEGIN SAPLING FOOTER) Stack created with [Sapling](https://sapling-scm.com). Best reviewed with [ReviewStack](https://reviewstack.dev/facebook/react/pull/34503). * facebook#34689 * __->__ facebook#34503
…lt (facebook#34689) This enables `@enablePreserveExistingMemoizationGuarantees` by default. As of the previous PR (facebook#34503), this mode now enables the following behaviors: - Treating variables referenced within a `useMemo()` or `useCallback()` as "frozen" (immutable) as of the start of the call. Ie, the compiler will assume that the values you reference are not mutated by the body of the useMemo, not are they mutated later. Directly modifying them (eg `var.property = true`) will be an error. - Similarly, the results of the useMemo/useCallback are treated as frozen (immutable) after the call. These two rules match the behavior for other hooks: this means that developers will see similar behavior to swapping out `useMemo()` for a custom `useMyMemo()` wrapper/alias. Additionally, as of facebook#34503 the compiler uses information from the manual dependencies to know which variables are non-nullable. Even if a useMemo block conditionally accesses a nested property — `if (cond) { log(x.y.z) }` — where the compiler would not usually know that `x` is non-nullable, if the user specifies `x.y.z` as a manual dependency then the compiler knows that `x` and `x.y` are non-nullable and can infer a more precise dependency. Finally, this mode also ensures that we always memoize function calls that return primitives. See facebook#34343 for more details. For now, I've explicitly opted out of this feature in all test fixtures where the behavior changed.
…efault (facebook#34654) Rebased on facebook#34454. Always include the root in the timeline even if it has no unique suspenders, since even if it won't suspend, we have to be able to see that and step to one step before the next boundary to see the first boundary that does suspend in its fallback state. Also, if there's no current selection on initial mount, select the last entry in the timeline. We usually do this with `selectedSuspenseID` but that doesn't happen on initial load. So this does it on initial load if nothing else is selected by then. That way when you reload you get the initial root selected. There's a problem here because we should really use one source of truth and `selectedSuspenseID` doesn't really do anything now. Either it should be its separate source of truth and you can't show components in the side-panel or it should be derived from the other state. If it's derived, once there's a selection, e.g. in the root, then even if new timelines load it will never change but that's probably a good thing.
Stacked on facebook#34654. The root is special since it represents "Initial Paint" (or a "Transition" when an Activity is selected). This gives it a different color in the timeline as well as gives it an outline that's clickable. Hovering the timeline now shows "Initial Paint" or "Suspense". Also made the cursor a pointer to invite you to try to click things and some rounded corners. <img width="1219" height="420" alt="Screenshot 2025-10-02 at 1 26 38 PM" src="https://github.com/user-attachments/assets/12451f93-8917-4f3b-8f01-930129e5fc13" /> <img width="1217" height="419" alt="Screenshot 2025-10-02 at 1 26 54 PM" src="https://github.com/user-attachments/assets/02b5e94c-3fbe-488d-b0f2-225b73578608" /> <img width="1215" height="419" alt="Screenshot 2025-10-02 at 1 27 24 PM" src="https://github.com/user-attachments/assets/c24e8861-e74a-4ccc-8643-ee9d04bef43c" /> <img width="1216" height="419" alt="Screenshot 2025-10-02 at 1 27 10 PM" src="https://github.com/user-attachments/assets/d5cc2b62-fa64-41bf-b485-116b1cd67467" />
…#34630) We're showing too much noise in the side-panel when selecting a Suspense boundary. The interesting thing to see directly is the "Suspended by". The "props" are mostly useless because the `"name"` prop is already in the tree. I'm now also showing it in the title bar of the selected element panel. The "children" and "fallback" props are just the thing that you can see in the tree view anyway. The "state" is this weird section with just one field in it, which we already have duplicated in the top toolbar as well. We can just delete this. I make sure to show the icon and a "suspended..." section while the boundary is still loading but now yet resuspended by force suspending. While still loading: <img width="600" height="193" alt="Screenshot 2025-09-27 at 11 54 37 PM" src="https://github.com/user-attachments/assets/1c3f3a96-46e0-4b11-806f-032569c7d5b5" /> After loading: <img width="602" height="266" alt="Screenshot 2025-09-27 at 11 54 53 PM" src="https://github.com/user-attachments/assets/c43cc4cb-036f-4ced-9b0d-226c6320cd76" /> Resuspended after loading: <img width="602" height="300" alt="Screenshot 2025-09-27 at 11 55 07 PM" src="https://github.com/user-attachments/assets/0be01735-48a7-47dc-b5cf-e72ec71e0148" />
…acebook#34695) Somehow my last commit didn't make it in facebook#34630.
…k#34694) When we flush a Suspense boundary we might not flush the fallback segment, it might only flush a placeholder instead. In this case the segment can flush again but we do not want to flush the boundary itself a second time. We now detach the boundary after flushing it. better solution to: facebook#34668
…e on (facebook#34698) This auto updates to select the last entry in the timeline until we make the first selection. That way when new content loads in, we show the last timeline of what is visible.
The previous DiffEditor view of the playground looked broken and not cohesive. There would be parts of the scrollbar appearing on the left side for some reason, along with two scrollbars on the right side. This PR makes the DiffEditor look more cohesive. Previous: https://github.com/user-attachments/assets/1aa1c775-5940-43b2-a75a-9b46452fb78b After: https://github.com/user-attachments/assets/b5c04998-6a6c-4b52-b3c5-b2fef21729e0
Adds back the compiler rules to the recommended preset, intended for the next release. --- [//]: # (BEGIN SAPLING FOOTER) Stack created with [Sapling](https://sapling-scm.com). Best reviewed with [ReviewStack](https://reviewstack.dev/facebook/react/pull/34675). * facebook#34703 * facebook#34700 * facebook#34699 * __->__ facebook#34675
Updates the eslint fixture lockfiles. --- [//]: # (BEGIN SAPLING FOOTER) Stack created with [Sapling](https://sapling-scm.com). Best reviewed with [ReviewStack](https://reviewstack.dev/facebook/react/pull/34699). * facebook#34703 * facebook#34700 * __->__ facebook#34699 * facebook#34675
…ok#34700) Previously, the `recommended` config used the legacy ESLint format (plugins as an array of strings). This causes errors when used with ESLint v9's `defineConfig()` helper. This was following [eslint's own docs](https://eslint.org/docs/latest/extend/plugins#backwards-compatibility-for-legacy-configs): > With this approach, both configuration systems recognize "recommended". The old config system uses the recommended key while the current config system uses the flat/recommended key. The defineConfig() helper first looks at the recommended key, and if that is not in the correct format, it looks for the flat/recommended key. This allows you an upgrade path if you’d later like to rename flat/recommended to recommended when you no longer need to support the old config system. However, [`isLegacyConfig()`](https://github.com/eslint/rewrite/blob/main/packages/config-helpers/src/define-config.js#L73-L81) (also see [`eslintrcKeys`](https://github.com/eslint/rewrite/blob/main/packages/config-helpers/src/define-config.js#L24-L35)) function doesn't check for the `plugins` key, so our config was incorrectly treated as flat config despite being in legacy format. This PR fixes the issue, along with a few other fixes combined: 1. Convert `recommended` to flat config format 2. Separate basic rules (exhaustive-deps, rules-of-hooks) from compiler rules 3. Add `recommended-latest-legacy` config for non-flat config users who want all recommended rules (including compiler rules) 4. Adding more types for the exported config Our shipped presets in 6.x.x will essentially be: - `recommended-legacy`: legacy (non-flat), with basic rules only - `recommended-latest-legacy`: legacy (non-flat), all rules (basic + compiler) - `flat/recommended`: flat, basic rules only (now the same as recommended, but to avoid making a breaking change we'll just keep it around in 6.x.x) - `recommended-latest`: flat, all rules (basic + compiler) - `recommended`: flat, basic rules only In the next breaking release 7.x.x, we will collapse down the presets into three: - `recommended-legacy`: all recommended rules - `recommended`: all recommended rules - `recommended-experimental`: all recommended rules + new bleeding edge experimental rules Closes facebook#34679 --- [//]: # (BEGIN SAPLING FOOTER) Stack created with [Sapling](https://sapling-scm.com). Best reviewed with [ReviewStack](https://reviewstack.dev/facebook/react/pull/34700). * facebook#34703 * __->__ facebook#34700
This rule was a leftover from a while ago and doesn't actually lint anything useful. Specifically, you get a lint error if you try to opt out a component that isn't already bailing out. If there's a bailout the compiler already safely skips over it, so adding `'use no memo'` there is unnecessary. Fixes facebook#31407 --- [//]: # (BEGIN SAPLING FOOTER) Stack created with [Sapling](https://sapling-scm.com). Best reviewed with [ReviewStack](https://reviewstack.dev/facebook/react/pull/34703). * __->__ facebook#34703 * facebook#34700
Stacked on facebook#34533 for root fragment handling This is the same approach as DOM, where we call getRootNode on the parent. Tests are in react-native using Fantom.
Stacked on facebook#34544 We only have getBoundingClientRect available from RN currently. This should work as a substitute for this case because the equivalent of multi-rect elements in RN is a nested Text component. We only include the rects of top-level host components here so we can assume that calling getBoundingClientRect on each child is the same result. Tested in react-native with Fantom.
Fixed two small issues with the config panel in the compiler playground: 1. Object descriptions were being confined in the config box and most of it would not be visible upon hover 2. Changed it so that "Applied Configs" would only display a valid set of configs, rather than switching between "Invalid Configs" and the set of options. This would be less visually jarring for users as the Output panel already displays errors. Additionally, if users want to see the list of config options but have a currently broken config, they would previously not know how to fix it. Object hover before: <img width="702" height="481" alt="Screenshot 2025-09-26 at 10 41 03 AM" src="https://github.com/user-attachments/assets/b2ddec2f-16ba-41a1-be1f-96211f46764c" /> Hover after: <img width="702" height="481" alt="Screenshot 2025-09-26 at 10 40 37 AM" src="https://github.com/user-attachments/assets/dc713a22-4710-46a8-a5d7-485060cc9074" /> Applied Configs always displays the last valid set of configs: https://github.com/user-attachments/assets/2fb9232f-7388-4488-9b7a-bb48bf09e4ca
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#34715