Previously useMemo inlining created a new StoreLocal assignment (not
reassignment!) instruction for every return value. This breaks when the return
is inside a block (like an if-block) as the scope is tied to the block.
For example: ``` let x = useMemo(() => { if (...) { return { ... }; } })
``` would become: ``` if (...) { const temp = { ... }; } const x = temp; ```
This PR instead changes the inlining to declare a temporary in the function
prologue and then reassign values to it when replacing return statements.
``` let x = useMemo(() => { if (...) { return { ... }; } }) ```
becomes
``` let temp; if (...) { temp = { ... }; } const x = temp; ```
This is a simplified version of #1454. The goal of this PR is to inline the
contents of `useMemo()` callbacks, rather than just immediately invoke the
lambda. Turning useMemo() into an IIFE works, but it means that we can't
optimize within the lambda block. Our investigations showed that there's a lot
of room to optimize at a finer granularity than manually written useMemo calls.
For example, one product instance had a useMemo that created a list of child JSX
elements. Most of those elements only relied on a single variable (`a`), but a
few relied on a second variable (`b). Thus _all_ elements were invalidated
whenever `b` changed. If Forget retains the original lambda, we have no choice
but to keep that (coarse) granularity for memoization. When we inline, we can
optimize to make e.g. individual JSX elements depend on their precise
dependencies.
The rough idea is:
* Keep track of all function expressions
* When we find a useMemo, lookup its function expression, and add its CFG to the
main function (the previous PR ensures that BlockIds won't collide)
* Replace any return statements with a StoreLocal to save the result and a Goto
to the code following the useMemo call.
* Then we run the usual set of passes to patch the HIR back up again.
Example:
```javascript
// Before
function Component(props) {
const x = useMemo(() => {
if (props.cond) {
return null;
}
return foo(props.x);
}, [props.x]);
return x + props.y;
}
// Intended - **before** memoization
function Component(props) {
let x;
if (props.cond) {
x = null;
} else {
x = foo(props.x);
}
return x + props.y;
}
```
This PR ensures (via a static assertion function) that all terminal variants
have a SourceLocation, and adds locations to the variants which didn't have it
before. This also adds a static assertion that terminals have an InstructionId,
though we already relied on that so it was checked via usage.
This is a prerequisite to inlining `useMemo()` lambdas so that we can better
optimize them. Nested functions are evaluated with a fresh HIRBuilder, which
means that they currently have their own `bindings` object for mapping
identifier instances to IdentifierIds. This means that identifier ids in a
closure are _always_ different that those outside the closure, even when they
refer to the same identifier:
```
function Component(props) {
props; // becomes e.g. props$1
const onClick = () => {
props // becomes e.g. props$2
};
}
```
For useMemo inlining this is problematic because we've lost the association that
these identifiers actually refer to the same thing. This PR changes that,
sharing the name resolution data structure between the top-level function and
any nested function expressions.
To unblock internal experimentation, for now let's just skip over compiling any
file that contains one or more disables of React's eslint rules, and log that.
This is a little coarse in the sense that we could skip over just functions that
contain the comments, but Babel doesn't provide an easy way to traverse comments
afaict so this is the simplest solution. I did check our internal repo and noted
that there was only one disable of exhaustive-hooks in that entire directory in
one file, so this should be fine.
Notably we are not throwing any errors if we detect these violations as we don't
want to fail the build, we just want to skip them for now.
While running the latest Forget build on www I noticed that a lot of the
bailouts were special-cases where we used interpolation in the error `reason`
string to provide more context for debugging. This is a pretty cool result,
because it means that we actually support nearly all the common syntax (at least
based on a sample of the codebase). But it makes our tools for aggregating
errors break down a bit.
This PR adds a new, nullable `description` property to CompilerErrorDetail, and
manually updates to ensure that we always pass a static `reason` and only use
interpolation in the `description`. This will allow our aggregation tools to
group by the reason.
NOTE: See background in #1476.
Updates BuildHIR to use the new LabelTerminal for LabeledStatements, and adds
support for HIR->ReactiveFunction transformation and codegen. Note that we
sometimes produce an extraneous block wrapper if it turns out the label wasn't
necessary, that seems...fine?
Adds a new `LabelTerminal` which will be used to represent LabeledStatements
that contain a statement other than a loop. What we do for these cases is
basically break the containing block in two, with a goto after the inner
statement to the fallthrough. This allows us to model the label, and any `break`
to it, in the HIR. However this fails in codegen because we can't find the
fallthrough branch — we need a high level terminal that knows about this
structure.
Hence LabelTerminal. Now, instead of just a continuation block and a goto, we
have a structured terminal. The LabelTerminal expresses the block for the
labeled statement and the continuation, and we can use this to put it back
together when constructing a ReactiveFunction. Note that this PR is just the
scaffolding for LabelTerminal, the next PR is the interesting bits.
Adds a script to automate adding/updating the copyright header to all
appropriate files. For now i've excluded fixture inputs, just because it would
impact fixture outputs too, but we can add them in a later PR if we want.
This PR ensures that we use a single id space for the `BlockId`s in both
top-level functions as well as any nested FunctionExpressions (note, we already
do this for `IdentifierId`). This will make it easier for follow-ups to merge
the CFG of nested functions (ie useMemo bodies) with the parent without block id
collisions.
Fixes `<fbt>`. This required a bunk of yak shaving to work through several
issues:
* First, there was a bug in codegen for JsxNamedspacedName. I added handling for
it for identifiers, but JsxNamespacedName gets converted to a Primitive. The
output looked correct because Babel happily creates invalid Jsx identifiers!
* Next, I needed to add locations to JSX nodes. It took me a while to pinpoint
which specific node needed the location, so I ended up just adding locations to
all the parts of a Jsx element.
* That uncovered the fact that FBT was expecting the `<fbt:param>`'s `name`
attribute value to be a StringLiteral, not a StringLiteral wrapped in a
JsxExpressionContainer. So now we special-case JsxAttribute and emit raw
StringLiteral (either is allowed per the spec)
And with that, voila, `<fbt>` works.
Adds the two FBT (https://facebook.github.io/fbt/) plugins to our test setup so
that we can verify Forget plays well with FBT. Unfortunately FBT's plugins are a
bit finicky, and things that are technically allowed per the JSX spec (such as
wrapping string attribute values in a JsxExpressionContainer) aren't supported
by FBT's plugin. This PR is just to add the fbt plugins and highlight some cases
that fail; these are fixed in later PRs in the stack.
For example, the `fbt-params.js` fixture fails on this PR:
Input
```error.fbt-params.js
import fbt from "fbt";
function Component(props) {
return (
<fbt desc={"Dialog to show to user"}>
Hello <fbt:param name="user name">{props.name}</fbt:param>
</fbt>
);
}
```
Output
```
React Forget › __tests__/fixtures/compiler › fixtures › fbt-params
Expected fixture 'fbt-params' to succeed but it failed with error:
/Users/joesavona/github/react-forget/forget/fbt-params: fbt: unsupported babel
node: MemberExpression
---
props.name
---
```
See fixes later in the stack.
Adds support for `for` statements with an empty or unreachable update
expression. In both cases, reversePostorderBlocks() will remove the
empty/unreachable update block, leaving the ForTerminal.update pointing to a
non-existent block. We explicitly rewrite this (much like we null out
unreachable fallthroughs after shrink). When transforming to ReactiveFunction,
we emit the update block as null if it was the same as the test block.
Fix for the previous issue, suggested by @gsathya: when we run
InferReferenceEffects on the outer function we check each closure to see if it
actually captured any mutable values. If it didn't, we can mark the closure as
readonly and memoize it independently.
Repro of a closure that we currently treat as readonly because it captures a
possibly-mutable value, but which we later realize is not mutable. Specifically,
when we check `exit()` we think `dispatch()` is mutable and therefore consider
it captured, which means we can't independently memoize `exit`.
Updates `PruneNonEscapingScopes` to consider hook arguments as potentially
escaping. This is because hook inputs are "owned" by React — for example,
closures passed to `useEffect`, or a value that is passed to a custom hook and
which then becomes a memoized input.
Similar to what we did for `<fbt>` jsx elements, this PR ensures that `fbt()`
calls have their operands memoized in the same scope to honor the limited
contract for what's allowed as an argument of an fbt() call expression.
There are some internal restrictions in Metro that only allow us to specify one
gating module as an injected dependency. To allow multiple projects, this PR
updates the Babel plugin to take a gating options config specifiying a project
name. The project name is used as a suffix for the generated import; for
example:
```js
const options = {
// ...
gating: {
module: "ReactForgetFeatureFlag",
importSpecifierName: "isForgetEnabled_Secret",
};
// generates
import {isForgetEnabled_Secret} from "ReactForgetFeatureFlag"; // a module that
exports multiple flags
// ...
```
This is a Meta-ism, but adding it for now to unblock. We special-case the
`<fbt>` element for translation purposes, and have a transform that requires the
children of this element to be a limited subset of nodes. Notably, any dynamic
translation values must appear as `<fbt:param>` children — we disallow
identifiers as children of `<fbt>` nodes.
This PR adds a new pass which finds `<fbt>` nodes and ensures their immediate
operands are not independently memoized. Note that this still allows the values
of `<fbt:param>` to be independently memoized, as demonstrated in the unit test.
```js
// here, `a?.b.c` is a single optional chain
// (evaluates to undefined if a is nullish)
a?.b.c;
// here, 'a?.b` is an optional chain, and `.c` is an unconditional load
// (nullthrows if a is nullish)
(a?.b).c;
```
---
Next PR in stack will add a bailout for `(a?.b).c`.
(If we want to properly handle `(a?.b).c`, we might want to model optional
chains explicitly in the HIR. We currently assume that any `PropertyLoad` whose
lhs is an optional property load is read conditionally.)
Sathya Gunasekaran
Joe Savona
Lauren Tan
mofeiZ
Mofei Zhang