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.
---
Changed `panicOnBailout: boolean` to `panicThreshold`, which has the following
options. Note that `ALL_ERRORS` corresponds to `panicOnBailout = true` and
`CRITICAL_ERRORS` corresponds to `panicOnBailout = false`. `NONE` is a new
option.
```js
export type PanicThresholdOptions =
// Bail out of compilation on all errors by throwing an exception.
| "ALL_ERRORS"
// Bail out of compilation only on critical or unrecognized errors.
// Instead, silently skip the erroring function.
| "CRITICAL_ERRORS"
// Never bail out of compilation. Instead, silently skip the erroring
// function or file.
| "NONE";
```
Jest seems to run babel through a different pipeline than Metro and -
(perhaps through its complex `require` interjection logic). When running jest
tests, exceptions thrown by babel transforms will bubble up to the nearest
exception boundary which is often the jest test itself. This may not be a useful
signal to anyone running a jest test with Forget enabled, as the erroring code
may be within Forget itself or a transitively required module.
Another reason to immediately bailing out on critical errors is that we may want
to record errors found in the rest of the file.
---
I'm not convinced that this change makes sense. A counterargument is that any
CriticalErrors *should* be reported as parse errors, regardless of the runtime
mode. Anyhow, this would be useful long term for our static analysis scripts
(e.g. collecting info on bailouts and compilation info e.g. # slots used for
JSX) as we want to compile-as-much-as-possible in those.
---
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).
We currently have multiple flags for targeting which functions to compile, but
they are actually mutually exclusive. This PR consolidates to a single
`compilationMode: 'annotation' | 'infer' | 'all'` flag:
* Annotation compiles only functions that explicitly opt-in with "use forget"
* Infer compiles explicitly opted-in functions (via "use forget") as well as any
known/inferred components or hooks:
* Component declarations
* Component or hook-like functions (same rules as the ESLint plugin but with an
extra check for whether it uses JSX or calls a hook)
* All compiles all top-level functions. We should get rid of this in a follow-up
and make tests use infer mode by default, and add explicit opt-ins where
necessary.
In all modes, "use no forget" always takes precedence and can be used to
opt-out. The default mode is now "infer".
Minimal repro extracted from our internal codebase. Our inference mode sees that
this arrow function is component-like and attempts to compile it, which then
fails because the function accesses `this` which we bailout on.
I ran into the same issue that @poteto and @gsathya (and probably @mofeiZ) have
run into: "Duplicate declaration of '$'" caused by Babel visiting a function
twice despite our calling `skip()`. This PR keeps a set of nodes that we have
already visited to avoid visiting them again, as a workaround for skip not
working.
# Test Plan
Synced to www and confirmed that the previous bug no longer reproduces, and the
compiled output looks sane.
Completes a todo (ie fixes a silly mistake) from a PR earlier in the stack, so
we now correctly recognize and compile arguments to `React.forwardRef()` and
`React.memo()`.
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.
We currently have multiple flags for targeting which functions to compile, but
they are actually mutually exclusive. This PR consolidates to a single
`compilationMode: 'annotation' | 'infer' | 'all'` flag:
* Annotation compiles only functions that explicitly opt-in with "use forget"
* Infer compiles explicitly opted-in functions (via "use forget") as well as any
known/inferred components or hooks:
* Component declarations
* Component or hook-like functions (same rules as the ESLint plugin but with an
extra check for whether it uses JSX or calls a hook)
* All compiles all top-level functions. We should get rid of this in a follow-up
and make tests use infer mode by default, and add explicit opt-ins where
necessary.
In all modes, "use no forget" always takes precedence and can be used to
opt-out. The default mode is now "infer".
Adds a new option to infer which functions to compile, based on React's ESLint
rule. The main difference is that in addition to checking the function name we
also check that it creates JSX or calls a hook. This should cover a significant
majority of components and reduce the chance of accidentally targeting
non-components, but it will leave some false negatives.
Note that some cases that the ESLint plugin infers as React functions don't work
yet: we don't compile FunctionExpressions, only ArrowFunctionExpressions, and
the way we handle ArrowFunctionExpression doesn't work with things like
forwardRef or variable declarations. We'll need more updates to fully handle all
these cases, which I'll do later in the stack.
Sorry about the thrash in advance! This removes the top level `forget` directory
which adds unnecessary nesting to our repo
Hopefully everything still works
Sathya Gunasekaran
Mofei Zhang
Lauren Tan
Joe Savona