Infer if a function is a component or hook when we're deciding to compile a
function and store that in the environment.
This is used in passes like InferReferenceEffects rather than having to re-parse
the name in each pass.
filepath
Internal rollout currently has a good number of test failures.
`enableEmitInstrumentForget` can help developers understand which functions /
files they should look at:
```
// input
function Foo() {
userCode();
// ...
}
// output
function Foo() {
if (__DEV__ && inE2eTestMode) {
logRender("Foo", "/path/to/filename.js");
}
const $ = useMemoCache(...);
userCode();
}
```
Updates the compiler to understand Flow hook syntax. Like component syntax, in
infer mode hooks are compiled by default unless opted out.
Looking ahead, i can imagine splitting up our compilation modes as follows:
* Annotations: opt-in explicitly
* Declarations: annotations + component/hook declarations
* Infer: annotations, component/hook declarations, + component/hook-like
functions
This also suggest an alternative annotation strategy: "use react" (or "use
component" / "use hook") as a general way to tell the compiler that a function
is intended for React. Then opting out of memoization could do "use
react(nomemo)".
We want to start moving away from "Forget", so this PR adds support "use memo"
and "use no memo"
I've left "use forget" and "use no forget" directives unchanged for now, as we
need to migrate existing users first and then come back and delete support for
these directives.
Currently we only allow adding the directive to function bodies, but there may
be cases where we want to always opt out an entire module from being compiled by
Forget
---
This change simply logs on every function we encounter with a `use no forget`
directive. A few nuances -- `compilationMode: "infer"` only compiles functions
we infer to be 'react functions'.
```js
// `add` would not be compiled, as it has no jsx, no hook calls,
// and is not named as a component or hook
function add(a, b) {
return a + b;
}
```
With this PR, we would report todos for functions that Forget wouldn't
ordinarily try to compile.
```js
// Todo: Skipped due to "use no forget" directive.
function add(a, b) {
"use no forget";
return a + b;
}
```
This seems fine to me as (1) it's a bit nonsensical to have a `use no forget`
direction on a non-react function, and (2) we're goalling on getting `use no
forget`s down to 0.
We're doing some internal benchmarking using a lightweight bundler that @pieterv
wrote for experimentation purposes. It's designed to fully preserve Flow type
annotations so we can experiment with type-driven compilation and test out what
benefits we might get from "cross-module" compilation more easily (ie by just
bundling together a few modules so we can see them all as one).
However, the bundler renames local variables and imports, so that a reference to
`useMemo()` might end up as `React$useMemo()` or similar. This PR adds a flag to
tell the compiler that builtin hooks might be prefixed and resolve them
appropriately.
The compiler bails out of compiling code that contains suppressions of the
official React ESLint rules. However, some apps may use additional rules that
they want to trigger bailouts for, or use the official rules under a different
name (we do this at Meta). This PR adds a compiler flag to specify a custom set
of line rule names, suppression of which should trigger a bailout.
Interpolating values into the `reason` field of an error breaks our error
aggregation. This PR moves the offending function name into the `description`
field which isn't used for aggregation.
It's starting to get complex just with a couple of extra
passes — we either need to substantially extend the HIR or (as i've done so far)
pass information from early passes to later ones. This PR changes things so that
very early in the babel plugin we fork into a separate mode. Forest has
its own `compileProgram()` equivalent, its own pipeline, its own codegen, etc.
---
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.
Instead of using the source location to check for hoisting, just stop checking
for a given component after we reach it's declaration.
By definition all references to it before are (potential) hoisting errors.
Note that there could be false positives but that's ok.
When you have your panic threshold set to "NONE" as we recommend, it's easy to
miss that your config is wrong (which makes everything not compile) because
those errors were being silenced. This made debugging FluentUI and the
forget-feedback testapp pretty difficult to figure out at first, and defeats the
purpose of having config validation in the first place.
This pr makes it so InvalidConfig errors always throw, regardless of the panic
threshold set. In general our plugin should never throw at build time due to
component bailouts, but because an InvalidConfig would bailout everything from
being compiled at all, it seems reasonable to throw here
Babel doesn't attach Comment nodes to anything, so they dangle off of
the Program node while only specifying a range. This meant that
previously we first had to traverse all of the Program's comments to
find an eslint suppression of the rules of React, then during traversal
of the individual functions, we would check if there were any *global*
eslint suppressions, then bailout all components.
This PR updates our logic to determine if individual functions are
affected by an eslint suppression range:
- If an eslint suppression range falls within its body; or
- If an eslint suppression wraps the function
This reverts commit 10d129a8406e9d226abdb6943bf8512e34ce91db
---
Reverts #2311 due to undocumented assumptions being broken. I also added some
comments to `LoggerEvents` to explain each event type.
In `Program.ts`, we have something like the following code. `compile` could
produce any number of errors (not just expected errors / instances of
`CompilerError`). As an example, we sometimes error in `Codegen` due to babel
version incompatibilities (`Error: ObjectMethod: Too many arguments passed.
Received 7 but can receive no more than 5`).
```js
try {
// any error could be thrown here
compile(input);
} catch (e) {
// unknown type for e
handleError(e, ...);
}
```
We should only add imports if we actually compiled anything, this is what caused
the internal issue despite the file in question not having any functions
opted-in to compilation.
Use zod to do runtime validation and throw if incorrect.
This PR only adds validation for ExternalFunction, will validate other options
in follow on PRs.
Not every consumer of Forget will be able to run an experimental version of
React. In the meantime before useMemoCache is stable, provide a way for OSS to
pass in a userspace impl.
Skips compilation of code that has a reference to `useMemoCache()`, as a
last-resort to avoid double-compilation of code. This is meant as a quick way to
unblock since we're still seeing some double compilation issues when syncing
internally.
Joe Savona
Sathya Gunasekaran
Mofei Zhang
Jordan Brown
Lauren Tan
Mike Vitousek