Joseph Savona

[compiler] Aggregate error reporting, separate eslint rules (#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 <feifei0@meta.com>

[compiler] Add snap subcommand to minimize a test input (#35663) Snap now supports subcommands 'test' (default) and 'minimize`. The minimize subcommand attempts to minimize a single failing input fixture by incrementally simplifying the ast so long as the same error occurs. I spot-checked it and it seemed to work pretty well. This is intended for use in a new subagent designed for investigating bugs — fixture simplification is an important part of the process and we can automate this rather than light tokens on fire. Example Input: ```js function Component(props) { const x = []; let result; for (let i = 0; i < 10; i++) { if (cond) { try { result = {key: bar([props.cond && props.foo])}; } catch (e) { console.log(e); } } } x.push(result); return <Stringify x={x} />; } ``` Command output: ``` $ yarn snap minimize --path .../input.js Minimizing: .../input.js Minimizing................ --- Minimized Code --- function Component(props) { try { props && props; } catch (e) {} } Reduced from 16 lines to 5 lines ``` This demonstrates things like: * Removing one statement at at time * Replacing if/else with the test, consequent, or alternate. Similar for other control-flow statements including try/catch * Removing individual array/object expression properties * Replacing single-value array/object with the value * Replacing control-flow expression (logical, consequent) w the test or left/right values * Removing call arguments * Replacing calls with a single argument with the argument * Replacing calls with multiple arguments with an array of the arguments * Replacing optional member/call with non-optional versions * Replacing member expression with the object. If computed, also try replacing w the key * And a bunch more strategies, see the code

[compiler] Improve IIFE inlining (#33726) We currently inline IIFEs by creating a temporary and a labeled block w the original code. The original return statements turn into an assignment to the temporary and break out of the label. However, many cases of IIFEs are due to inlining of manual `useMemo()`, and these cases often have only a single return statement. Here, the output is cleaner if we avoid the temporary and label - so that's what we do in this PR. Note that the most complex part of the change is actually around ValidatePreserveExistingMemo - we have some logic to track the IIFE temporary reassignmetns which needs to be updated to handle the simpler version of inlining. --- [//]: # (BEGIN SAPLING FOOTER) Stack created with [Sapling](https://sapling-scm.com). Best reviewed with [ReviewStack](https://reviewstack.dev/facebook/react/pull/33726). * __->__ #33726 * #33725

[compiler] Upstream experimental flow integration (#34121) all credit on the Flood/ code goes to @mvitousek and @jbrown215, i'm just the one upstreaming it

[compiler] Collapse CompilerError.{invariant,simpleInvariant} (#35614) `invariant()` was a pain to use - we always record a single location, but the API required passing a compiler detail. This PR replaces `invariant()` (keeping the name) with `simpleInvariant()`s signature, and updates call sites accordingly. I've noticed that agents consistently get invariant() wrong, which aligns with it being tedious to call when you're writing code by hand. The simplified API should help a bit.

[compiler] Improve merging of scopes that invalidate together (#34049) We try to merge consecutive reactive scopes that will always invalidate together, but there's one common case that isn't handled. ```js const y = [[x]]; ``` Here we'll create two consecutive scopes for the inner and outer array expressions. Because the input to the second scope is a temporary, they'll merge into one scope. But if we name the inner array, the merging stops: ```js const array = [x]; const y = [array]; ``` This is because the merging logic checks if all the dependencies of the second scope are outputs of the first scope, but doesn't account for renaming due to LoadLocal/StoreLocal. The fix is to track these temporaries. --- [//]: # (BEGIN SAPLING FOOTER) Stack created with [Sapling](https://sapling-scm.com). Best reviewed with [ReviewStack](https://reviewstack.dev/facebook/react/pull/34049). * __->__ #34049 * #34047 * #34044

[compiler] Phase 2+7: Wrap pipeline passes in tryRecord for fault tolerance (#35874) - Change runWithEnvironment/run/compileFn to return Result<CodegenFunction, CompilerError> - Wrap all pipeline passes in env.tryRecord() to catch and record CompilerErrors - Record inference pass errors via env.recordErrors() instead of throwing - Handle codegen Result explicitly, returning Err on failure - Add final error check: return Err(env.aggregateErrors()) if any errors accumulated - Update tryCompileFunction and retryCompileFunction in Program.ts to handle Result - Keep lint-only passes using env.logErrors() (non-blocking) - Update 52 test fixture expectations that now report additional errors This is the core integration that enables fault tolerance: errors are caught, recorded, and the pipeline continues to discover more errors. --- [//]: # (BEGIN SAPLING FOOTER) Stack created with [Sapling](https://sapling-scm.com). Best reviewed with [ReviewStack](https://reviewstack.dev/facebook/react/pull/35874). * #35888 * #35884 * #35883 * #35882 * #35881 * #35880 * #35879 * #35878 * #35877 * #35876 * #35875 * __->__ #35874

[compiler] Fix `for` loops in try/catch (#35686) This is a combination of a) a subagent for investigating compiler errors and b) testing that agent by fixing bugs with for loops within try/catch. My recent diffs to support maybe-throw within value blocks was incomplete and handled many cases, like optionals/logicals/etc within try/catch. However, the handling for for loops was making more assumptions and needed additional fixes. Key changes: * `maybe-throw` terminal `handler` is now nullable. PruneMaybeThrows nulls the handler for blocks that cannot throw, rather than changing to a `goto`. This preserves more information, and makes it easier for BuildReactiveFunction's visitValueBlock() to reconstruct the value blocks * Updates BuildReactiveFunction's handling of `for` init/test/update (and similar for `for..of` and `for..in`) to correctly extract value blocks. The previous logic made assumptions about the shape of the SequenceExpression which were incorrect in some cases within try/catch. The new helper extracts a flattened SequenceExpression. Supporting changes: * The agent itself (tested via this diff) * Updated the script for invoking snap to keep `compiler/` as the working directory, allowing relative paths to work more easily * Add an `--update` (`-u`) flag to `yarn snap minimize`, which updates the fixture in place w the minimized version

[compiler] Enable validateExhaustiveMemoizationDependencies by default (#35201) Enables `@validateExhaustiveMemoizationDependencies` feature flag by default, and disables it in select tests that failed due to the change. Some of our tests intentionally use incorrect memo dependencies in order to test edge cases. --- [//]: # (BEGIN SAPLING FOOTER) Stack created with [Sapling](https://sapling-scm.com). Best reviewed with [ReviewStack](https://reviewstack.dev/facebook/react/pull/35201). * #35213 * __->__ #35201

[compiler] Improve ref validation error message (#34003) Improves the error message for ValidateNoRefAccessInRender, using the new diagnostic type as well as providing a longer but succinct summary of what refs are for and why they're unsafe to access in render. --- [//]: # (BEGIN SAPLING FOOTER) Stack created with [Sapling](https://sapling-scm.com). Best reviewed with [ReviewStack](https://reviewstack.dev/facebook/react/pull/34003). * #34027 * #34026 * #34025 * #34024 * #34005 * #34006 * #34004 * __->__ #34003