Per comments on the new validation pass, this disallows creating JSX (expression/fragment) within a try statement. Developers sometimes use this pattern thinking that they can catch errors during the rendering of the element, without realizing that rendering is lazy. The validation allows us to teach developers about the error boundary pattern.
ghstack-source-id: 0bc722aeae
Pull Request resolved: https://github.com/facebook/react/pull/30725
Addresses a todo from a while back. We now validate environment options when parsing the plugin options, which means we can stop re-parsing/validating in later phases.
ghstack-source-id: b19806e843
Pull Request resolved: https://github.com/facebook/react/pull/30726
This PR updates the eslint plugin to report unused opt out directives.
One of the downsides of the opt out directive is that it opts the
component/hook out of compilation forever, even if the underlying issue
was fixed in product code or fixed in the compiler.
ghstack-source-id: 81deb5c11b
Pull Request resolved: https://github.com/facebook/react/pull/30721
This PR updates the babel plugin to continue the compilation pipeline as
normal on components/hooks that have been opted out using a directive.
Instead, we no longer emit the compiled function when the directive is
present.
Previously, we would skip over the entire pipeline. By continuing to
enter the pipeline, we'll be able to detect if there are unused
directives.
The end result is:
- (no change) 'use forget' will always opt into compilation
- (new) 'use no forget' will opt out of compilation but continue to log
errors without throwing them. This means that a Program containing
multiple functions (some of which are opted out) will continue to
compile correctly
ghstack-source-id: 5bd85df2f8
Pull Request resolved: https://github.com/facebook/react/pull/30720
Summary:
The change earlier in this stack makes it less safe to have ref enforcement disabled. This diff enables it by default.
ghstack-source-id: d3ab5f1b28
Pull Request resolved: https://github.com/facebook/react/pull/30716
Summary:
We previously were excessively strict about preventing functions that access refs from being returned--doing so is potentially valid for hooks, because the return value may only be used in an event or effect.
ghstack-source-id: cfa8bb1b54
Pull Request resolved: https://github.com/facebook/react/pull/30724
Summary:
Since we want to make ref-in-render errors enabled by default, we should position those errors at the location of the read. Not only will this be a better experience, but it also aligns the behavior of Forget and Flow.
This PR also cleans up the resulting error messages to not emit implementation details about place values.
ghstack-source-id: 1d11317068
Pull Request resolved: https://github.com/facebook/react/pull/30723
Test Plan:
Documents that useCallback calls interfere with it being ok for refs to escape as part of functions into jsx
ghstack-source-id: a5df427981
Pull Request resolved: https://github.com/facebook/react/pull/30714
Summary:
Refs, as stable values that the rules of react around mutability do not apply to, currently are treated as having mutable ranges, and through aliasing, this can extend the mutable range for other values and disrupt good memoization for those values. This PR excludes refs and their .current values from having mutable ranges.
Note that this is unsafe if ref access is allowed in render: if a mutable value is assigned to ref.current and then ref.current is mutated later, we won't realize that the original mutable value's range extends.
ghstack-source-id: e8f36ac25e
Pull Request resolved: https://github.com/facebook/react/pull/30713
Per discussion today, adds validation against calling setState "during" passive effects. Basically, it's fine to _schedule_ setState to be called (via a timeout, listener, etc) but generally not recommended to call setState during the effect since that will trigger a cascading render.
This validation is off by default, i'm putting this up for discussion and to experiment with it internally.
ghstack-source-id: 5f385ddab5
Pull Request resolved: https://github.com/facebook/react/pull/30685
Summary:
UseTransition is a builtin hook that returns a stable value, like useState. This PR represents that in Forget, and marks the startTransition function as stable.
ghstack-source-id: 0e76a64f2d
Pull Request resolved: https://github.com/facebook/react/pull/30681
Summary:
In theory, as I understand it, the result of a useRef will never change between renders, because we'll always provide the same ref value consistently. That means that memoization that depends on a ref value will never re-compute, so I think we could not infer it as a dependency in Forget. This diff, however, doesn't do that: it instead allows the validatePreserveExistingMemoizationGuarantees analysis to admit mismatches between explicit dependencies and implicit ones when the implicit dependency is a ref that doesn't exist in source.
ghstack-source-id: 685d859d1e
Pull Request resolved: https://github.com/facebook/react/pull/30679
Summary:
Builds support for macros that are invoked as methods rather than just function calls or jsx.
We now record macros as a schema that represents arbitrary member expressions including wildcards (so we can support, e.g., myMacro.*.foo.bar). When examining PropertyLoads in the macro memoization stage, we build up a map of partially-satisfied macro patterns until we determine that the pattern has been fully satisfied, at which point we treat the result of the PropertyLoad as a macro value.
ghstack-source-id: d78d9ba704
Pull Request resolved: https://github.com/facebook/react/pull/30589
Previously the compiler would add an import for the specified context
callee even if the context access was not lowered, leading to unused
imports.
This PR tracks if lowering has happened and adds the import only when
necessary.
ghstack-source-id: 6ad794da41
Pull Request resolved: https://github.com/facebook/react/pull/30628
If a value is specified for the LowerContextAccess environment config,
we rewrite the callee from 'useContext' to the specificed value.
This will allow us run an experiment internally.
ghstack-source-id: 00e161b988
Pull Request resolved: https://github.com/facebook/react/pull/30612
*This is only for internal profiling, not intended to ship.*
This pass is intended to be used with https://github.com/facebook/react/pull/30407.
This pass synthesizes selector functions by collecting immediately
destructured context acesses. We bailout for other types of context
access.
This pass lowers context access to use a selector function by passing
the synthesized selector function as the second argument.
ghstack-source-id: 92d0f6ff2f
Pull Request resolved: https://github.com/facebook/react/pull/30548
This PR updates to use SSA form through the entire compilation pipeline. This means that in both HIR form and ReactiveFunction form, `Identifier` instances map 1:1 to `IdentifierId` values. If two identifiers have the same IdentifierId, they are the same instance. What this means is that all our passes can use this more precise information to determine if two particular identifiers are not just the same variable, but the same SSA "version" of that variable.
However, some parts of our analysis really care about program variables as opposed to SSA versions, and were relying on LeaveSSA to reset identifiers such that all Identifier instances for a particular program variable would have the same IdentifierId (though not necessarily the same Identifier instance). With LeaveSSA removed, those analysis passes can now use DeclarationId instead to uniquely identify a program variable.
Note that this PR surfaces some opportunties to improve edge-cases around reassigned values being declared/reassigned/depended-upon across multiple scopes. Several passes could/should use IdentifierId to more precisely identify exactly which values are accessed - for example, a scope that reassigns `x` but doesn't use `x` prior to reassignment doesn't have to take a dependency on `x`. But today we take a dependnecy.
My approach for these cases was to add a "TODO LeaveSSA" comment with notes and the name of the fixture demonstrating the difference, but to intentionally preserve the existing behavior (generally, switching to use DeclarationId when IdentifierId would have been more precise).
Beyond updating passes to use DeclarationId instead of Identifier/IdentifierId, the other change here is to extract out the remaining necessary bits of LeaveSSA into a new pass that rewrites InstructionKind (const/let/reassign/etc) based on whether a value is actually const or has reassignments and should be let.
ghstack-source-id: 69afdaee5f
Pull Request resolved: https://github.com/facebook/react/pull/30573
Adds `Identifier.declarationId` and the new `DeclarationId` (simulated) opaque type. DeclarationId allows uniquely identifying a variable in the original source, ie regardless of reassignments. This allows us to stay in SSA form throughout compilation (see next diff) while still being able to distinguish SSA versions (via IdentifierId) and non-SSA versions (DeclarationId).
ghstack-source-id: f2547a58aa
Pull Request resolved: https://github.com/facebook/react/pull/30569
Update createTemporaryPlace to use makeTemporary and also rename
makeTemporary to makeTemporaryIdentifier to make it less ambiguous.
ghstack-source-id: b5955d3d66
Pull Request resolved: https://github.com/facebook/react/pull/30585
Summary:
Fixes issue documented by #30435. We change the pipeline order so that outlining comes after tracking macro operands, and any function that is referenced in a macro will now not be outlined.
ghstack-source-id: f731ad65c8
Pull Request resolved: https://github.com/facebook/react/pull/30587
This is a useful utility function similar to the existing
`makeInstructionId` and `makeIdentifierId` functions.
This PR moves it outside the HIRBuilder so we can use this in passes
that don't have access to the builder instance.
ghstack-source-id: 1ac0839e6c
Pull Request resolved: https://github.com/facebook/react/pull/30545
Rather than storing the entire babel node, store only the required
information which is the node type.
This will be useful for when we synthesize new functions that don't have
a corresponding babel node.
ghstack-source-id: 9098cbdbc4
Pull Request resolved: https://github.com/facebook/react/pull/30544
ghstack failed to land #30552 properly, resubmitting
Developers sometimes use `useMemo()` as a way to conditionally execute code, including conditionally calling setState. However, the compiler may remove existing useMemo calls if they are not necessary, which _should_ always be a safe optimization. If the useMemo has side effects (eg sets state), then this isn't safe.
This PR improves ValidateNoSetStateInRender to disallow any setState in useMemo (even if it's conditional), expanding on the previous check for unconditional setState in render. Note that the approach uses the StartMemoize/FinishMemoize instructions added in DropManualMemo to know whether a particular setState call is within a useMemo or not. This means enabling the validation in DropManualMemo when the setState validation is enabled, but that's fine since that validation is on everywhere by default (_except_ for in fixtures, which we have a todo for)
ghstack-source-id: 65bb3289c3
Pull Request resolved: https://github.com/facebook/react/pull/30583
Summary:
This diff extends the existing work on validating against locals being reassigned after render, by propagating the reassignment "effect" into the lvalues of instructions when the rvalue operands include values known to cause reassignments. In particular, this "closes the loop" for function definitions and function calls: a function that returns a function that reassigns will be considered to also perform reassignments, but previous to this we didn't consider the result of a `Call` of a function that reassigns to itself be a value that reassigns.
This causes a number of new bailouts in test cases, all of which appear to me to be legit.
ghstack-source-id: 770bf02d07
Pull Request resolved: https://github.com/facebook/react/pull/30540
While debugging #30536 I happened to notice that the bug only reproduced
when there was interleaving scopes, and observed that an unpruned scope
nested inside of a pruned one was not being visited by
CollectPromotableTemporaries, which keeps track of which identifiers
should be promoted later. Therefore when actually promoting temporaries
we were skipping over the identifiers in children of pruned scopes
ghstack-source-id: d805f62f22
Pull Request resolved: https://github.com/facebook/react/pull/30537
