forked from facebook/react
-
Notifications
You must be signed in to change notification settings - Fork 0
Mirror of upstream PR #34855 #540
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
716
commits into
main
Choose a base branch
from
upstream-pr-34855
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
<!-- 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 Part 3 of adding a "Config Override" panel to the React compiler playground. Added a button to apply config changes to the Input panel, as well as making the tab collapsible. Added validation for the the PluginOptions type (although comes with a bit more boilerplate) to make it very obvious what the possible config errors could be. Added some toasts for trying to apply broken configs. <!-- Explain the **motivation** for making this change. What existing problem does the pull request solve? --> ## How did you test this change? https://github.com/user-attachments/assets/63ab8636-396f-45ba-aaa5-4136e62ccccc <!-- 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. -->
…facebook#34407) ### Problem - Users encounter “Failed to construct 'URL': Invalid base URL” when clicking the “View source” action in DevTools if the underlying base URL is invalid. - This exception originates from `new URL(relative, base)` and bubbles up, interrupting the DevTools UI. - Fixes GitHub issue [facebook#34317](facebook#34317) ### Solution - Wrap URL construction to: - First try `new URL(sourceMapAt, sourceURL)`. - If that fails, try `new URL(sourceMapAt)` as an absolute URL. - If both fail, return `null` (no symbolication) rather than throwing. - This preserves normal behavior for valid bases and absolute URLs, while avoiding crashes for invalid bases. ### Implementation details - Updated `symbolicateSource` in `packages/react-devtools-shared/src/symbolicateSource.js` to handle invalid base URL scenarios without throwing. - Added/verified tests in `packages/react-devtools-shared/src/__tests__/utils-test.js`: - “should not throw for invalid base URL with relative source map” → resolves to `null`. - “should resolve absolute source map even if base URL is invalid” → still resolves correctly. ### Test plan - Lint/format: - `yarn prettier-check` - `yarn linc` - Type checking: - `yarn flow dom-node` - Unit tests: - `yarn test --watchAll=false utils-test` - Optionally: `yarn test --watchAll=false utils-test inspectedElement` - All of the above pass locally for experimental channel. ### Risks and rollout - Risk: Low. Only affects cases where the base URL is invalid. - Normal cases (valid base or absolute `sourceMappingURL`) are unchanged. - No user-facing API changes; DevTools UX becomes more resilient. ### Affected packages - `react-devtools-shared` ### Related - Fixes GitHub issue [facebook#34317](facebook#34317) ### Checklist - [x] Ran `yarn prettier-check` - [x] Ran `yarn linc` - [x] Ran `yarn flow dom-node` - [x] Relevant unit tests passing - [x] Linked issue and added a concise summary <!-- Thanks for submitting a pull request! We appreciate you spending the time to work on these changes. Please provide enough information so that others can review your pull request. The three fields below are mandatory. Before submitting a pull request, please make sure the following is done: 1. Fork [the repository](https://github.com/facebook/react) and create your branch from `main`. 2. Run `yarn` in the repository root. 3. If you've fixed a bug or added code that should be tested, add tests! 4. Ensure the test suite passes (`yarn test`). Tip: `yarn test --watch TestName` is helpful in development. 5. Run `yarn test --prod` to test in the production environment. It supports the same options as `yarn test`. 6. If you need a debugger, run `yarn test --debug --watch TestName`, open `chrome://inspect`, and press "Inspect". 7. Format your code with [prettier](https://github.com/prettier/prettier) (`yarn prettier`). 8. Make sure your code lints (`yarn lint`). Tip: `yarn linc` to only check changed files. 9. Run the [Flow](https://flowtype.org/) type checks (`yarn flow`). 10. If you haven't already, complete the CLA. Learn more about contributing: https://reactjs.org/docs/how-to-contribute.html --> ## Summary <!-- Explain the **motivation** for making this change. What existing problem does the pull request solve? --> ## 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. -->
With facebook#34176 we now have granular lint rules created for each compiler ErrorCategory. However, we had remnants of our old error severities still in use which makes reporting errors quite clunky. Previously you would need to specify both a category and severity which often ended up being the same. This PR moves severity definition into our rules which are generated from our categories. For now I decided to defer "upgrading" categories from a simple string to a sum type since we are only using severities to map errors to eslint severity. --- [//]: # (BEGIN SAPLING FOOTER) Stack created with [Sapling](https://sapling-scm.com). Best reviewed with [ReviewStack](https://reviewstack.dev/facebook/react/pull/34401). * facebook#34409 * facebook#34404 * facebook#34403 * facebook#34402 * __->__ facebook#34401
Now that we have a new CompilerDiagnostic type (which the CompilerError aggregate can hold), the old CompilerErrorDetail type can be marked as deprecated. Eventually we should migrate everything to the new CompilerDiagnostic type. --- [//]: # (BEGIN SAPLING FOOTER) Stack created with [Sapling](https://sapling-scm.com). Best reviewed with [ReviewStack](https://reviewstack.dev/facebook/react/pull/34402). * facebook#34409 * facebook#34404 * facebook#34403 * __->__ facebook#34402 * facebook#34401
…infra (facebook#34403) Mechanical PR to migrate existing invariants to use the new CompilerDiagnostic infra @josephsavona added. Will tackle the others at a later time. --- [//]: # (BEGIN SAPLING FOOTER) Stack created with [Sapling](https://sapling-scm.com). Best reviewed with [ReviewStack](https://reviewstack.dev/facebook/react/pull/34403). * facebook#34409 * facebook#34404 * __->__ facebook#34403
Small fix to make all descriptions consistently printed with a single period at the end. Ran `grep -rn "description:" packages/babel-plugin-react-compiler/src --include="*.ts" --exclude-dir="__tests__" | grep '\.\s*["\`]'` to find all descriptions ending in a period and manually fixed them. --- [//]: # (BEGIN SAPLING FOOTER) Stack created with [Sapling](https://sapling-scm.com). Best reviewed with [ReviewStack](https://reviewstack.dev/facebook/react/pull/34404). * facebook#34409 * __->__ facebook#34404
…4409) This PR stops error details of severity `ErrorSeverity.Off` from being reported. --- [//]: # (BEGIN SAPLING FOOTER) Stack created with [Sapling](https://sapling-scm.com). Best reviewed with [ReviewStack](https://reviewstack.dev/facebook/react/pull/34409). * __->__ facebook#34409 * facebook#34404
…acebook#34406) Adds missing locations to all the statement kinds that we produce in codegenInstruction(), and adds generic handling of source locations for the nodes produced by codegenInstructionValue(). There are definitely some places where we are still missing a location, but this should address some of the known issues we've seen such as missing location on `throw`. --- [//]: # (BEGIN SAPLING FOOTER) Stack created with [Sapling](https://sapling-scm.com). Best reviewed with [ReviewStack](https://reviewstack.dev/facebook/react/pull/34406). * facebook#34394 * __->__ facebook#34406 * facebook#34346
…k#34419) I'm about to add info for pretty much all of these anyway since they all depend on the data stream itself.
) --- [//]: # (BEGIN SAPLING FOOTER) Stack created with [Sapling](https://sapling-scm.com). Best reviewed with [ReviewStack](https://reviewstack.dev/facebook/react/pull/34323). * facebook#34276 * __->__ facebook#34323
<!-- 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 Added a "Show Internals" toggle switch to either show only the Config, Input, Output, and Source Map tabs, or these tabs + all the additional compiler options. The open/close state of these tabs will be preserved (unless on page refresh, which is the same as the currently functionality). <!-- Explain the **motivation** for making this change. What existing problem does the pull request solve? --> ## 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. --> https://github.com/user-attachments/assets/8eb0f69e-360c-4e9b-9155-7aa185a0c018
The compiler playground was crashing at any small syntax errors in the `Input` panel due to updating the `CompilerErrorDetailOptions` type in facebook#34401. Updated the option to take in a `ErrorCategory` instead. --------- Co-authored-by: lauren <[email protected]>
…ptions for Fusebox (facebook#34429) Each integrator: browser extension, Chrome DevTools Frontend fork, Electron shell must define and provide `fetchFileWithCaching` in order for DevTools to be able to fetch application resources, such as scripts or source maps. More specifically, if this is available, React DevTools will be able to symbolicate source locations for component frames, owner stacks, "suspended by" Promises call frames. This will be available with the next release of React DevTools.
) Adds a `@enableNameAnonymousFunctions` feature to infer helpful names for anonymous functions within components and hooks. The logic is inspired by a custom Next.js transform, flagged to us by @eps1lon, that does something similar. Implementing this transform within React Compiler means that all React (Compiler) users can benefit from more helpful names when debugging. The idea builds on the fact that JS engines try to infer helpful names for anonymous functions (in stack traces) when those functions are accessed through an object property lookup: ```js ({'a[xyz]': () => { throw new Error('hello!') } }['a[xyz]'])() // Stack trace: Uncaught Error: hello! at a[xyz] (<anonymous>:1:26) // <-- note the name here at <anonymous>:1:60 ``` The new NameAnonymousFunctions transform is gated by the above flag, which is off by default. It attemps to infer names for functions as follows: First, determine a "local" name: * Assigning a function to a named variable uses the variable name. `const f = () => {}` gets the name "f". * Passing the function as an argument to a function gets the name of the function, ie `foo(() => ...)` get the name "foo()", `foo.bar(() => ...)` gets the name "foo.bar()". Note the parenthesis to help understand that it was part of a call. * Passing the function to a known hook uses the name of the hook, `useEffect(() => ...)` uses "useEffect()". * Passing the function as a JSX prop uses the element and attr name, eg `<div onClick={() => ...}` uses "<div>.onClick". Second, the local name is combined with the name of the outer component/hook, so the final names will be strings like `Component[f]` or `useMyHook[useEffect()]`. --- [//]: # (BEGIN SAPLING FOOTER) Stack created with [Sapling](https://sapling-scm.com). Best reviewed with [ReviewStack](https://reviewstack.dev/facebook/react/pull/34410). * facebook#34434 * __->__ facebook#34410
…ebook#34421) Eslint is expecting a map of [string] => RuleModule. Before we were passing {rule: RuleModule, severity: ErrorSeverity} which was breaking legacy Eslint configurations
…#34319) Alternative to facebook#34276 --- (Summary taken from @josephsavona 's facebook#34276) Partial fix for facebook#34262. Consider this example: ```js function useInputValue(input) { const object = React.useMemo(() => { const {value} = transform(input); return {value}; }, [input]); return object; } ``` React Compiler breaks this code into two reactive scopes: * One for `transform(input)` * One for `{value}` When we run ValidatePreserveExistingMemo, we see that the scope for `{value}` has the dependency `value`, whereas the original memoization had the dependency `input`, and throw an error that the dependencies didn't match. In other words, we're flagging the fact that memoized _better than the user_ as a problem. The more complete solution would be to validate that there is a subgraph of reactive scopes with a single input and output node, where the input node has the same dependencies as the original useMemo, and the output has the same outputs. That is true in this case, with the subgraph being the two consecutive scopes mentioned above. But that's complicated. As a shortcut, this PR checks for any dependencies that are defined after the start of the original useMemo. If we find one, we know that it's a case where we were able to memoize more precisely than the original, and we don't report an error on the dependency. We still check that the original _output_ value is able to be memoized, though. So if the scope of `object` were extended, eg with a call to `mutate(object)`, then we'd still correctly report an error that we couldn't preserve memoization. Co-authored-by: Joe Savona <[email protected]>
The previous PR added name hints for anonymous functions, but didn't handle the case of outlined functions. Here we do some cleanup around function `id` and name hints: * Make `HIRFunction.id` a ValidatedIdentifierName, which involved some cleanup of the validation helpers * Add `HIRFunction.nameHint: string` as a place to store the generated name hints which are not valid identifiers * Update Codegen to always use the `id` as the actual function name, and only use nameHint as part of generating the object+property wrapper for debug purposes. This ensures we don't conflate synthesized hints with real function names. Then, we also update OutlineFunctions to use the function name _or_ the nameHint as the input to generating a unique identifier. This isn't quite as nice as the object form since we lose our formatting, but it's a simple step that gives more context to the developer than `_temp` does. Switching to output the object+property lookup form for outlined functions is a bit more involved, let's do that in a follow-up.
One thing that can suspend is the downloading of the RSC stream itself. This tracks an I/O entry for each Promise (`SomeChunk<T>`) that represents the request to the RSC stream. As the value we use the `Response` for `createFromFetch` (or the `ReadableStream` for `createFromReadableStream`). The start time is when you called those. Since we're not awaiting the whole stream, each I/O entry represents the part of the stream up until it got unblocked. However, in a production environment with TLS packets and buffering in practice the chunks received by the client isn't exactly at the boundary of each row. It's a bit longer into larger chunks. From testing, it seems like multiples of 16kb or 64kb uncompressed are common. To simulate a production environment we group into roughly 64kb chunks if they happen in rapid sequence. Note that this might be too small to give a good idea because of the throttle many boundaries might be skipped anyway so this might show too many. The React DevTools will see each I/O entry as separate but dedupe if an outer boundary already depends on the same chunk. This deduping makes it so that small boundaries that are blocked on the same chunk, don't get treated as having unique suspenders. If you have a boundary with large content, then that content will likely be in a separate chunk which is not in the parent and then it gets marked as. This is all just an approximation. The goal of this is just to highlight that very large boundaries will very likely suspend even if they don't suspend on any I/O on the server. In practice, these boundaries can float around a lot and it's really any Suspense boundary that might suspend but some are more likely than others which this is meant to highlight. It also just lets you inspect how many bytes needs to be transferred before you can show a particular part of the content, to give you an idea that it's not just I/O on the server that might suspend. If you don't use the debug channel it can be misleading since the data in development mode stream will have a lot more data in it which leads to more chunking. Similarly to "client references" these I/O infos don't have an "env" since it's the client that has the I/O and so those are excluded from flushing in the Server performance tracks. Note that currently the same Response can appear many times in the same Instance of SuspenseNode in DevTools when there are multiple chunks. In a follow up I'll show only the last one per Response at any given level. Note that when a separate debugChannel is used it has its own I/O entry that's on the `_debugInfo` for the debug chunks in that channel. However, if everything works correctly these should never leak into the DevTools UI since they should never be propagated from a debug chunk to the values waited by the runtime. This is easy to break though.
) This was fun. We previously added the MaybeAlias effect in facebook#33984 in order to describe the semantic that an unknown function call _may_ alias its return value in its result, but that we don't know this for sure. We record mutations through MaybeAlias edges when walking backward in the data flow graph, but downgrade them to conditional mutations. See the original PR for full context. That change was sufficient for the original case like ```js const frozen = useContext(); useEffect(() => { frozen.method().property = true; }, [...]); ``` But it wasn't sufficient for cases where the aliasing occured between operands: ```js const dispatch = useDispatch(); <div onClick={(e) => { dispatch(...e.target.value) e.target.value = ...; }} /> ``` Here we would record a `Capture dispatch <- e.target` effect. Then during processing of the `event.target.value = ...` assignment we'd eventually _forward_ from `event` to `dispatch` (along a MaybeAlias edge). But in facebook#33984 I missed that this forward walk also has to downgrade to conditional. In addition to that change, we also have to be a bit more precise about which set of effects we create for alias/capture/maybe-alias. The new logic is a bit clearer, I think: * If the value is frozen, it's an ImmutableCapture edge * If the values are mutable, it's a Capture * If it's a context->context, context->mutable, or mutable->context, count it as MaybeAlias.
This is exported in the prod version of ReactServer experimental but not the development version so we can't use it in fixtures from Server Components.
Fixes facebook#34098. There's an issue in Chrome where the `InvalidStateError` always has the same error message. The spec doesn't specify the error message to use but it's more useful to have a specific one for each case like Safari does. One reason it's better to have a specific error message is because the browser console is not the main surface that people look for errors. Chrome relies on a separate log also in the console. Frameworks has built-in error dialogs that pop up first and that's where you see the error and that dialog can't show something specific. Additionally, these errors can't log something specific to servers in production logging. So this is a bad strategy. It's not good to have those error dialogs pop up for non-actionable errors like when it doesn't start because the document was hidden. Since we don't have more specific information we have no choice but to hide all of them. This includes actionable things like duplicate names (although we also have a React specific warning for that in the common case).
…ebook#34435) Stacked on facebook#34425. RSC stream info is split into one I/O entry per chunk. This means that when a single instance or boundary depends on multiple chunks, it'll show the same stream multiple times. This makes it so just the last one is shown. This is a special case for the name "RSC stream" but ideally we'd more explicitly model the concept of awaiting only part of a stream. <img width="667" height="427" alt="Screenshot 2025-09-09 at 2 09 43 PM" src="https://github.com/user-attachments/assets/890f6f61-4657-4ca9-82fd-df55a696bacc" /> Another remaining issue is that it's possible for an intermediate chunk to be depended on by just a child boundary. In that case that can be considered a "unique suspender" even though the parent depends on a later one. Ideally it would dedupe on everything below. Could also model it as every Promise depends on its chunk and every previous chunk.
…oundary (facebook#34438) Stacked on facebook#34435. This adds a method to get all suspended by filtered by a specific Instance. The purpose of this is to power the feature when you filter by Activity. This would show you the "root" within that Activity boundary. This works by selecting the nearest Suspense boundary parent and then filtering its data based on if all the instances for a given I/O info is within the Activity instance. If something suspended within the Suspense boundary but outside the Activity it's not included even if it's also suspending inside the Activity since we assume it would've already been loaded then. Right now I wire this up to be a special case when you select an Activity boundary same as when you select a Suspense boundary in the Components tab but we could also only use this when you select the root in the Suspense tab for example.
When the search query changes, we kick off a transition that updates the search query in a reducer for TreeContext. The search input is also using this value for an `input` HTML element. For a larger applications, sometimes there is a noticeable delay in displaying the updated search query. This changes the approach to also keep a local synchronous state that is being updated on a change callback.
…et value is derived from a ref (facebook#34462) @stipsan found this issue where the compiler would bailout on the `useLayoutEffect` examples in the React docs. While setState in an effect is typically an anti-pattern due to the fact that it hurts performance through cascading renders, the one scenario where it _is_ allowed is if the value being set flows from a ref.
) Two small QoL improvements inspired by feedback: * `if (ref.current === undefined) { ref.current = ... }` is now allowed. * `if (!ref.current) { ref.current = ... }` is still disallowed, but we emit an extra hint suggesting the `if (!ref.current == null)` pattern. I was on the fence about the latter. We got feedback asking to allow `if (!ref.current)` but if your ref stores a boolean value then this would allow reading the ref in render. The unary form is also less precise in general due to sketchy truthiness conversions. I figured a hint is a good compromise. --- [//]: # (BEGIN SAPLING FOOTER) Stack created with [Sapling](https://sapling-scm.com). Best reviewed with [ReviewStack](https://reviewstack.dev/facebook/react/pull/34449). * __->__ facebook#34449 * facebook#34424
Treat fake eval anonymous stacks as built-in. Hide built-in stack frames unless they're used to call into a non-ignored stack frame. The two main things to fix here is that 1) we're showing a linkified stack for fake anonymous and 2) we're showing only built-ins when the stack is completely internal. Meaning framework code is all noise.
There was a bug where the other output passes (aside from the "Output" tab) were unable to open on compiler error. This PR still allows for the "Output" tab to automatically open on error, but also allows other tabs to be opened. https://github.com/user-attachments/assets/157bf5d6-c289-46fd-bafb-073c2e0ff52b
We already do this in the update pass. That's what `shouldMeasureSuspenseNode` does. We also don't update measurements when we're inside an offscreen tree. However, we didn't check if the boundary itself was in a suspended state when in the `measureUnchangedSuspenseNodesRecursively` path. This caused boundaries to disappear when their fallback didn't have a rect (including their timeline entries).
We can't measure Text nodes directly but we can measure a Range around them. This is useful since it's common, at least in examples, to use text nodes as children of a Suspense boundary. Especially fallbacks.
I get the wish to click the shadow but not all child boundaries are within the bounds of the outer Suspense boundary's node. Sometimes they overflow naturally and if we make it overflow hidden we hide the boundaries. Maybe it would be ok if they're actually clipped by the real DOM but right now it covers up boundaries that should be there. Additionally, there's also a common case where the parent boundary shrinks when suspending the children. That then causes the suspended child boundaries to be clipped so that you can't restore them. Maybe the virtual boundary shouldn't shrink in this case.
…ransparent (facebook#34853) This makes the rects that are currently in a suspended state appear ghostly so that you can see where along the timeline you are in the rects screen. <img width="451" height="407" alt="Screenshot 2025-10-14 at 11 43 20 PM" src="https://github.com/user-attachments/assets/f89e362b-a0d5-46e3-8171-564909715cd1" />
Nobody knows what this terminology means. Also, this tooltip component sucks: <img width="634" height="137" alt="Screenshot 2025-10-15 at 12 04 49 AM" src="https://github.com/user-attachments/assets/a1c33650-7c7d-441f-8f8b-0ea7ebea9351" />
…tion (facebook#34859) This revealed that a lot of the event types were defined on the wrong end of the bridge. It was also a problem that events with the same name couldn't have different arguments.
…acebook#34847) In InferTypes when we infer types for properties during destructuring, we were breaking out of the loop when we encounter a hole in the array. Instead we should just skip that element and continue inferring later properties. Closes facebook#34748 --- [//]: # (BEGIN SAPLING FOOTER) Stack created with [Sapling](https://sapling-scm.com). Best reviewed with [ReviewStack](https://reviewstack.dev/facebook/react/pull/34847). * facebook#34855 * __->__ facebook#34847
I find it very frustrating that the highlight covers up the content that I'm trying to review when stepping through the timeline. It also triggered on keyboard navigation due to the focus which was annoying. We could highlight something in the rects instead potentially.
We should only persist a selection once you click. Currently, we persist the selection if you just hover which means you lose your selection immediately when just starting to inspect. That's not what Chrome Elements tab does - it selects on click.
We now do a single pass over the HIR, building up two data structures:
* One tracks values that are known macro tags or macro calls.
* One tracks operands of macro-related instructions so that we can later
group them.
After building up these data structures, we do a pass over the latter
structure. For each macro call instruction, we recursively traverse its
operands to ensure they're in the same scope. Thus, something like
`fbt('hello' + fbt.param(foo(), "..."))` will correctly merge the fbt
call, the `+` binary expression, the `fbt.param()` call, and `foo()`
into a single scope.
---
[//]: # (BEGIN SAPLING FOOTER)
Stack created with [Sapling](https://sapling-scm.com). Best reviewed
with [ReviewStack](https://reviewstack.dev/facebook/react/pull/34865).
* facebook#34855
* __->__ facebook#34865
Add inspection button to Suspense tab which lets you select only among Suspense nodes. It highlights all the DOM nodes in the root of the Suspense node instead of just the DOM element you hover. The name is inferred. <img width="1172" height="841" alt="Screenshot 2025-10-15 at 8 03 34 PM" src="https://github.com/user-attachments/assets/f04d965b-ef6e-4196-9ba0-51626148fa1a" />
This triggers unnecessary fetches.
In a demo today, `cookies()` showed up as `cookieses`. While adorable, is wrong.
Currently the sub-pixel precision is lost which can lead to things not lining up properly and being slightly off or overlapping. We need some sub-pixel precision. Ideally we'd just keep the floating point as is. I'm not sure why the operations is limited to integers. We don't send it as a typed array anyway it seems which would ideally be more optimal. Even if we did, we haven't defined a precision for the protocol. Is it 32bit integer? 64bit? If it's 64bit we can fit a float anyway. Ideally it would be more variable precision like just pushing into a typed array directly with the option to write whatever precision we want.
The hover now has a reach tooltip for the "environment" instead.
This ensures that the outline of a previous rectangle lines up on the same pixel as the next rectangle so that they appear consecutive. <img width="244" height="51" alt="Screenshot 2025-10-16 at 11 35 32 AM" src="https://github.com/user-attachments/assets/75ffde6f-8cc6-49c1-8855-3953569546b4" /> I don't love this implementation. There's probably a smarter way. Was trying to avoid adding another element.
## Summary When upgrading to `[email protected]` in a project that uses `zod@3` we are running into TypeScript errors like: ``` node_modules/babel-plugin-react-compiler/dist/index.d.ts:435:10 - error TS2694: Namespace '"/REDACTED/node_modules/zod/v3/external"' has no exported member 'core'. 435 }, z.core.$strip>>>; ~~~~ ``` This problem seems to be related to d6eb735, which introduced zod v3/v4 compatibility. Since `zod` is bundled into the compiler source this does not cause runtime issues and only manifests as TypeScript errors. My proposed solution is this PR is to use zod's [subpath versioning strategy](https://zod.dev/v4/versioning?id=versioning-in-zod-4) which allows you to support v3 and v4 APIs on both major versions. Changes in this PR include: - Updated `zod` import paths to `zod/v4` - Bumped min `zod` version to `^3.25.0` for zod which guarantees the `zod/v4` subpath is available. - Updated `zod-validation-error` import paths to `zod-validation-error/v4` - Bumped min `zod-validation-error ` version to `^3.5.0` - Updated `externals` tsup configuration where appropriate. Once the compiler drops zod v3 support we could optionally remove the `/v4` subpath from the imports. ## How did you test this change? Not totally sure the best way to test. I ran `NODE_ENV=production yarn workspace babel-plugin-react-compiler run build --dts` and diffed the `dist/` folder between my change and `v1.0.0` and it looks correct. We have a `patch-package` patch to workaround this for now and it works as expected. ```diff diff --git a/node_modules/babel-plugin-react-compiler/dist/index.d.ts b/node_modules/babel-plugin-react-compiler/dist/index.d.ts index 81c3f3d..daafc2c 100644 --- a/node_modules/babel-plugin-react-compiler/dist/index.d.ts +++ b/node_modules/babel-plugin-react-compiler/dist/index.d.ts @@ -1,7 +1,7 @@ import * as BabelCore from '@babel/core'; import { NodePath as NodePath$1 } from '@babel/core'; import * as t from '@babel/types'; -import { z } from 'zod'; +import { z } from 'zod/v4'; import { NodePath, Scope } from '@babel/traverse'; interface Result<T, E> { ``` Co-authored-by: Henry Q. Dineen <[email protected]>
…acebook#34820) ## Summary Fixes facebook#34793. We are allowing passing down effect events when they are inlined as a prop. ``` <Child onClick={useEffectEvent(...)} /> ``` This seems like a case that someone not familiar with `useEffectEvent`'s purpose could fall for so this PR introduces logic to disallow its usage. An alternative implementation would be to modify the name and function of `recordAllUseEffectEventFunctions` to record all `useEffectEvent` instances either assigned to a variable or not, but this seems clearer. Or we could also specifically disallow its usage inside JSX. Feel free to suggest any improvements. ## How did you test this change? - Added a new test in `packages/eslint-plugin-react-hooks/__tests__/ESLintRulesOfHooks-test.js`. All tests pass.
Added the standard Meta Platforms, Inc. MIT license notice to the top of the feature flag comparison script to ensure compliance with repository licensing requirements and for code consistency. **No functional or logic changes were made to the code.**
Two additional validations for useMemo: * Disallow reassigning to values declared outside the useMemo callback (always on) * Disallow unused useMemo calls (part of the validateNoVoidUseMemo feature flag, which in turn is off by default) We should probably enable this flag though! --- [//]: # (BEGIN SAPLING FOOTER) Stack created with [Sapling](https://sapling-scm.com). Best reviewed with [ReviewStack](https://reviewstack.dev/facebook/react/pull/34868). * facebook#34855 * facebook#34882 * __->__ facebook#34868
This is a great validation, so let's enable by default. Changes: * Move the validation logic into ValidateUseMemo alongside the new check that the useMemo result is used * Update the lint description * Make the void memo errors lint-only, they don't require us to skip compilation (as evidenced by the fact that we've had this validation off)
…urn in useMemo Potential fix for facebook#34750. With a useMemo with multiple returns we can sometimes end up thinking the FinishMemoize value wasn't memoized, even though it definitely is. But the same case works if you rewrite to have a let binding, assign to the let instead of returning, and then have a single return with the let — which is the exact shape we convert to when we inline the useMemo! The only difference is that when we inline, we end up with an extra LoadLocal. Adding an extra LoadLocal "fixes" the issue but this is definitely a hack. However, along the way this helped me find that we also have a bug in PruneAlwaysInvalidatingScopes, where we were forgetting to set FinishMemoize instructions to pruned if their declaration always invalidates. Putting up for feedback, not necessarily expecting to land as-is.
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#34855