Adds a new `earlyReturnValue` property on ReactiveScope which will be set if the
scope had one or more early returns, with information about the temporary
identifier that the early return value will be assigned to, as well as the label
to be used for breaking (to simulate the early-return). The next PR shows the
intended codegen.
The previous PR introduced `memoize` instructions whose lvalues aren't used, but
which can't be pruned by DCE due to pipeline ordering. Here we change to make
memoize an instruction intended for its side effects only, and prune during
codegen.
Adds an option to preserve existing memoization guarantees for values produced
with useMemo and useCallback. We still discard the calls to these hooks, but we
preserve the information that the value is frozen at that point in the program.
Because these values are produced solely within the useMemo/useCallback
callback, their mutation cannot have any interspersed hook calls. This means
that the values mutable range will never span a hook and end at the point of the
useMemo, ensuring that they are memoized at the same point.
The main things that can change (relative to the orignal code) are:
* Forget will infer a precise set of dependencies, ignoring the user-provided
values. In practice this should only occur if the original code had a lint
violation, which Forget would bail out on. So in practice this shouldn't happen
unless the code doesn't use the React linter.
* Forget may start the memoization block earlier than the developer did if other
values are mutated along with the value being produced. This can cause
memoization to fail, but only in situations where it would have failed
previously:
```javascript
const a = [];
useFoo();
const b = useMemo(() => {
const c = a;
c.push(1);
return c;
}, [a]);
```
In this example (sans Forget) the useMemo will invalidate on every render
because `a` will always be a new array and its listed as a dependency of the
useMemo. Forget would correctly determine that the memoization would have to
work as follows:
```javascript
let c;
if (...) {
const a = []
useFoo(); // OOPS we made a hook call conditional
const t0 = a;
t0.push(1);
c = t0;
...
} else {
c = $[...]
}
```
Because this is invalid, Forget would (later in the pipeline) strip out this
memoization block and (as with the original) leave `c` un-memoized.
In this same example, removing the hook would cause Forget to be able to memoize
a value that wasn't memoized before:
```javascript
const a = [];
const b = useMemo(() => {
const c = a;
c.push(1);
return c;
}, [a]);
```
This invalidates every render without Forget, but would memoize correctly with
Forget (it would expand the memoization block to include the declaration of
`a`).
The previous PR converts reactive scopes to normal instructions, so that Forest
mode won't have any scopes left by the time we reach codegen. This PR removes
the now-unused codegen logic for forest.
For Forest, we previously converted reactive scopes into derived signals during
Codegen. I'm moving this to a separate pass primarily to keep codegen simple
since there's enough complexity just dealing with core JS semantics. Ideally
we'd do a similar setup even for regular Forget, ie lower reactive scopes just
prior to codegen.
At the same time i also reordered the forget passes to be just before codegen,
and cleaned things up a bit. For state lowering, we now just rewrite `useState`
-> `createState`, because we actually need to keep around the setter function to
trigger scheduling updates in addition to writing the signal value.
---
I modeled guards as try-finally blocks to be extremely explicit. An alternative
implementation could flatten all nested hooks and only set / restore hook guards
when entering / exiting a React function (i.e. hook or component) -- this
alternative approach would be the easiest to represent as a separate pass
```js
// source
function Foo() {
const result = useHook(useContext(Context));
...
}
// current output
function Foo() {
try {
pushHookGuard();
const result = (() => {
try {
pushEnableHook();
return useHook((() => {
try {
pushEnableHook();
return useContext(Context);
} finally {
popEnableHook();
}
})());
} finally {
popEnableHook();
};
})();
// ...
} finally {
popHookGuard();
}
}
// alternative output
function Foo() {
try {
// check current is not lazyDispatcher;
// save originalDispatcher, set lazyDispatcher
pushHookGuard();
allowHook(); // always set originalDispatcher
const t0 = useContext(Context);
disallowHook(); // always set LazyDispatcher
allowHook(); // always set originalDispatcher
const result = useHook(t0);
disallowHook(); // always set LazyDispatcher
// ...
} finally {
popHookGuard(); // restore originalDispatcher
}
}
```
Checked that IG Web works as expected
Unless I add a sneaky useState:
<img width="705" alt="Screenshot 2023-12-05 at 6 44 59 PM"
src="https://github.com/facebook/react-forget/assets/34200447/3790bd76-7d71-44b5-a62e-f53256fb5736">
---
Prior to this PR, we were mutating functions after CodegenReactiveFunction
completes (in `Entrypoint/Program.ts`).
The reasoning for this separation was that we wanted to keep non-compiler logic
out of the core Pipeline. However, it made our code difficult to read and reason
about.
Open to other alternatives, like adding a pass after Codegen.
I did a double take when I thought we didn't handle returning the
error when reading the code and when I edited the code, typescript told
me that there's no need to return as creating the error will throw.
This PR makes it clear from the name of the function that we will throw.
Reactive scopes are currently preceded by individual variable declarations, one
for each of the scope's dependencies. Only after checking independently if each
of these dependencies has changed do we then do an "or" to check if we should
actually recompute the body of the scope.
But that's wasteful: it's more efficient to rely on `||` short-circuiting, and
recompute as soon as any input changes.
The main reason I can see not to do this is debugging. Having the change
variables makes it easy to see what changed. But at this point I think it makes
sense to optimize for code size and performance; we can always add back a
dev-only mode that uses change variables if that turns out to help debugging.
This morning @mofeiZ reminded me that our codegen doesn't really have any guards
against reordering, since temporaries are lazily emitted. We're relying on the
fact that our lowering and memoization carefully preserves order of evaluation,
such that delaying the instructions in codegen doesn't change semantics.
To help catch any mistakes with this, I had previously added code that reset the
codegen context's temporaries before/after exiting a reactive scope. That
ensured that temporaries from within the scope weren't accessible outside it.
This PR extends that approach to _all_ blocks, so that temporaries created
within a block aren't accessible outside it.
I'm also going to explore more actively resetting temporaries after they
"should" be used. There are a couple cases where temporaries are reused, though,
which we have to change first.
Updates `Environment` to store all feature flags on a single `config` object. We
now also define an object with all the default config values, and use this to
populate defaults for any missing values in the user-provided config.
Fixes an issue with incorrect spacing where spaces were getting dropped, despite
an explicit `{" "}` in the input. The issue is that we didn't maintain JSXText
all the way through compilation. BuildHIR distinguishes string literals (such as
the above, inside an expressioncontainer) from JSXText, and we propagate this
distinction all the way through to codegen.
But then codegen stores temporary values as `t.Expression` nodes, which means we
have to convert the JSXText nodes to StringLiteral and we lose the distinction.
This PR updates codegen to save temporaries as `t.Expression | t.JSXText` so
that we can preserve the difference. In most places we just coerce the value to
an expression, but the code for emitting JSX child items looks at the raw value
so it can distinguish them. JSXText is emitted as-is, while StringLiterals are
always wrapped in an expression container.
See the new test case which demonstrates the expression being preserved.
Object methods are lowered to functions and added to ObjectExpression. The
codegen is interesting because we shouldn't emit code that lowers the object
method into a separate statement and then stores it into an object expression.
An shorthand object method has different semantics than an object method using
the function syntax, so we need to preserve the shorthand object method syntax
in the generated code.
To do this, we don't immediately generate an AST node for the ObjectMethod but
instead store it in a side table during codegen. Only when emitting code for an
ObjectExpression, we lookup this side table and emit the object method inline in
the body.
This PR adds preliminary support for hoisting const variable declarations. We do
this via BuildHIR when lowering top level statements in a BlockStatement, by
first checking which bindings are in scope to be hoistable if referenced before
they are declared. The declarations are then hoisted to their earliest point
where they are referenced (ie the top level statement just before) as context
variables.
Later, prior to codegen, we restore the original source by removing the
DeclareContexts and transforming their associated StoreContexts back.
Support for hoisting other kinds of declarations will come in future PRs!
Per the title, `<fbt:param>0</fbt:param>` is invalid FBT, you must wrap the text
in an expression container. But that's not all, `fbt:param` can only have a
single child, which means we have to strip out the text elements that occur from
the whitespace in the source.
The type of ObjectProperty is specific to the key, not the ObjectProperty. In
the future, we want to add a type to represent the value of the ObjectProperty.
This PR moves out the fields related to the key of the ObjectProperty to a
separate type.
@mofeiZ noticed that some configurations of prettier don't support JSXFragment
appearing as a JSXAttribute value, in violation of the spec. Coincidentally I
noticed that our internal build system also fails on this as i was trying to
roll out on more surfaces. This PR makes sure we wrap fragments in an
expressioncontainer if they appear as jsxattribute values.
---
Add types for logged events, including a `CompileSuccess` event which can help
us record successfully compiled Forget functions and their compilation details
(e.g. # memoSlots used).
Replaces the use of `NextIterableOf` in for-in with a new `NextPropertyOf`
instruction. The key distinction is `for-of` invokes an arbitrary iterator,
which means a) each iteration may mutate the collection being iterated and b)
the returned value may be mutable. However, `for-in` invokes a language-level
mechanism to iterate: simply iterating alone _cannot_ modify the collection, and
the returned value is known to be a primitive.
First pass of ForInStatement support. This mostly just copies our handling of
ForOfStatement, but the next PR updates to use a different instruction instead
of `NextIterableOf`.
It's possible that the value thrown during a `try` block actually is a reference
to some value defined outside the scope of the try block. If the catch clause
param is also mutated, that means the mutable range of the variable would have
to include the entire try/catch.
We handle this by emitting a DeclareLocal temporary for the catch param prior to
the try/catch. If it is modified during the catch block, that will extend its
mutable range to cover the full try/catch. If any values are mutated inside the
try, their range will also (naturally) extend around the full try/catch block.
These ranges will overlap and be merged, ensuring that we capture the
possibility that the value is mutated via the catch param. See unit test.
This PR adds the other piece, a 'try' terminal which represents try/catch and
the possibility of fallthrough to the code afterwards. For now `finally` is
unsupported. We don't yet produce these terminals, see later in the stack.
Mostly reuses existing analysis of an identifier property key.
Adds a new type field to ObjectProperty to propogate the type of the key. This
is used in codegen to correctly emit a string literal or an identifier.
Instead of emitting a memo block, emit a function expression and pass it as an
argument to derived (which will then create a computed).
The naming of 'derived' needs to be tweaked.
This PR changes the way we compile ArrowFunctionExpression to allow compiling
more cases, such as within `React.forwardRef()`. We no longer convert arrow
functions to function declarations. Instead, CodeGenerator emits a generic
`CodegenFunction` type, and `Program.ts` is responsible for converting that to
the appropriate type. The rule is basically:
* Retain the original node type by default. Function declaration in, function
declaration out. Arrow function in, arrow function out.
* When gating is enabled, we emit a ConditionalExpression instead of creating a
temporary variable. If the original (and hence compiled) functions are function
declarations, we force them into FunctionExpressions only here, since we need an
expression for each branch of the conditional. Then the rules are:
* If this is a `export function Foo` ie a named export, replace it with a
variable declaration with the conditional expression as the initializer, and the
function name as the variable name.
* Else, just replace the original function node with the conditional. This works
for all other cases.
I'm open to feedback but this seems like a pretty robust approach and will allow
us to support a lot of real-world cases that we didn't yet, so i think we need
_something_ in this direction.
Sorry about the thrash in advance! This removes the top level `forget` directory
which adds unnecessary nesting to our repo
Hopefully everything still works
Joe Savona
Mofei Zhang
Sathya Gunasekaran
Lauren Tan