Revamp compilation modes
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".
Mofei Zhang committed
Sep 14, 2023 at 19:40 UTC
869466bb0b46d8ede59bbc61b91de294f783f576
7 files changed
+109
-147
compiler/packages/babel-plugin-react-forget/src/Entrypoint/Program.ts
+56
-75
@@ -33,13 +33,9 @@ export type CompilerPass = {
33
comments: (t.CommentBlock | t.CommentLine)[];
34
};
35
36
-function hasUseForgetDirective(directive: t.Directive): boolean {
37
- return directive.value.value === "use forget";
38
-}
39
-
36
function hasAnyUseForgetDirectives(directives: t.Directive[]): boolean {
37
for (const directive of directives) {
42
- if (hasUseForgetDirective(directive)) {
38
+ if (directive.value.value === "use forget") {
39
return true;
40
}
41
}
@@ -54,6 +50,26 @@ function hasAnyUseNoForgetDirectives(directives: t.Directive[]): boolean {
50
}
51
return false;
52
}
53
+function handleError(pass: CompilerPass, err: unknown): void {
54
+ if (pass.opts.logger && err) {
55
+ pass.opts.logger.logEvent("err", err);
56
+ }
57
+ /** Always throw if the flag is enabled, otherwise we only throw if the error is critical
58
+ * (eg an invariant is broken, meaning the compiler may be buggy). See
59
+ * {@link CompilerError.isCritical} for mappings.
60
+ * */
61
+ if (
62
+ pass.opts.panicOnBailout ||
63
+ !(err instanceof CompilerError) ||
64
+ (err instanceof CompilerError && err.isCritical())
65
+ ) {
66
+ throw err;
67
+ } else {
68
+ if (pass.opts.isDev) {
69
+ log(err, pass.filename ?? null);
70
+ }
71
+ }
72
+}
73
74
/**
75
* Runs the Compiler pipeline and mutates the source AST to include the newly compiled function.
@@ -73,24 +89,7 @@ function compileAndInsertNewFunctionDeclaration(
89
try {
90
compiledFn = compileFn(fnPath, pass.opts.environment);
91
} catch (err) {
76
- if (pass.opts.logger && err) {
77
- pass.opts.logger.logEvent("err", err);
78
- }
79
- /** Always throw if the flag is enabled, otherwise we only throw if the error is critical
80
- * (eg an invariant is broken, meaning the compiler may be buggy). See
81
- * {@link CompilerError.isCritical} for mappings.
82
- * */
83
- if (
84
- pass.opts.panicOnBailout ||
85
- !(err instanceof CompilerError) ||
86
- (err instanceof CompilerError && err.isCritical())
87
- ) {
88
- throw err;
89
- } else {
90
- if (pass.opts.isDev) {
91
- log(err, pass.filename ?? null);
92
- }
93
- }
92
+ handleError(pass, err);
93
return false;
94
}
95
@@ -186,20 +185,10 @@ function insertNewFunctionDeclaration(
185
}
186
}
187
189
-// This is a hack to work around what seems to be a Babel bug. Babel doesn't
190
-// consistently respect the `skip()` function to avoid revisiting a node within
191
-// a pass, so we use this set to track nodes that we have compiled.
192
-const ALREADY_COMPILED: WeakSet<object> | Set<object> = new (WeakSet ?? Set)();
193
-
194
-export function compileProgram(
195
- program: NodePath<t.Program>,
196
- pass: CompilerPass
197
-): void {
198
- const options = parsePluginOptions(pass.opts);
188
+function findEslintSuppressions(
189
+ fileComments: Array<t.CommentBlock | t.CommentLine>
190
+): CompilerError | null {
191
const violations = [];
200
- const fileComments = pass.comments;
201
- let hasForgetMutatedOriginalSource: boolean = false;
202
- let fileHasUseForgetDirective = false;
192
193
if (Array.isArray(fileComments)) {
194
for (const comment of fileComments) {
@@ -214,26 +203,10 @@ export function compileProgram(
203
}
204
205
if (violations.length > 0) {
217
- program.traverse({
218
- Directive(directive) {
219
- if (hasUseForgetDirective(directive.node)) {
220
- fileHasUseForgetDirective = true;
221
- }
222
- },
223
- });
224
-
206
const reason =
207
"React Forget has bailed out of optimizing this component as one or more React eslint rules were disabled. React Forget only works when your components follow all the rules of React, disabling them may result in undefined behavior";
208
const error = new CompilerError();
209
for (const violation of violations) {
229
- if (options.logger != null) {
230
- options.logger.logEvent("err", {
231
- reason,
232
- filename: pass.filename,
233
- violation,
234
- });
235
- }
236
-
210
error.pushErrorDetail(
211
new CompilerErrorDetail({
212
reason,
@@ -250,19 +223,24 @@ export function compileProgram(
223
})
224
);
225
}
226
+ return error;
227
+ } else {
228
+ return null;
229
+ }
230
+}
231
254
- if (fileHasUseForgetDirective) {
255
- if (options.panicOnBailout || error.isCritical()) {
256
- throw error;
257
- } else {
258
- if (options.isDev) {
259
- log(error, pass.filename ?? null);
260
- }
261
- }
262
- }
232
+// This is a hack to work around what seems to be a Babel bug. Babel doesn't
233
+// consistently respect the `skip()` function to avoid revisiting a node within
234
+// a pass, so we use this set to track nodes that we have compiled.
235
+const ALREADY_COMPILED: WeakSet<object> | Set<object> = new (WeakSet ?? Set)();
236
264
- return;
265
- }
237
+export function compileProgram(
238
+ program: NodePath<t.Program>,
239
+ pass: CompilerPass
240
+): void {
241
+ const options = parsePluginOptions(pass.opts);
242
+ const lintError = findEslintSuppressions(pass.comments);
243
+ let hasForgetMutatedOriginalSource: boolean = false;
244
245
// Main traversal to compile with Forget
246
program.traverse(
@@ -287,10 +265,11 @@ export function compileProgram(
265
): void {
266
if (!shouldVisitNode(fn, pass)) {
267
return;
290
- }
291
-
292
- if (compileAndInsertNewFunctionDeclaration(fn, pass) === true) {
293
- hasForgetMutatedOriginalSource = true;
268
+ } else if (lintError != null) {
269
+ handleError(pass, lintError);
270
+ } else {
271
+ const hasMutated = compileAndInsertNewFunctionDeclaration(fn, pass);
272
+ hasForgetMutatedOriginalSource ||= hasMutated;
273
}
274
},
275
@@ -300,10 +279,11 @@ export function compileProgram(
279
): void {
280
if (!shouldVisitNode(fn, pass)) {
281
return;
303
- }
304
-
305
- if (compileAndInsertNewFunctionDeclaration(fn, pass) === true) {
306
- hasForgetMutatedOriginalSource = true;
282
+ } else if (lintError != null) {
283
+ handleError(pass, lintError);
284
+ } else {
285
+ const hasMutated = compileAndInsertNewFunctionDeclaration(fn, pass);
286
+ hasForgetMutatedOriginalSource ||= hasMutated;
287
}
288
},
289
@@ -313,10 +293,11 @@ export function compileProgram(
293
): void {
294
if (!shouldVisitNode(fn, pass)) {
295
return;
316
- }
317
-
318
- if (compileAndInsertNewFunctionDeclaration(fn, pass) === true) {
319
- hasForgetMutatedOriginalSource = true;
296
+ } else if (lintError != null) {
297
+ handleError(pass, lintError);
298
+ } else {
299
+ const hasMutated = compileAndInsertNewFunctionDeclaration(fn, pass);
300
+ hasForgetMutatedOriginalSource ||= hasMutated;
301
}
302
},
303
},
compiler/packages/babel-plugin-react-forget/src/__tests__/fixtures/compiler/error.sketchy-code-exhaustive-deps.expect.md
new
+26
@@ -0,0 +1,26 @@
1
+
2
+## Input
3
+
4
+```javascript
5
+function Component() {
6
+ const item = [];
7
+ const foo = useCallback(
8
+ () => {
9
+ item.push(1);
10
+ }, // eslint-disable-next-line react-hooks/exhaustive-deps
11
+ []
12
+ );
13
+
14
+ return <Button foo={foo} />;
15
+}
16
+
17
+```
18
+
19
+
20
+## Error
21
+
22
+```
23
+[ReactForget] InvalidReact: React Forget has bailed out of optimizing this component as one or more React eslint rules were disabled. React Forget only works when your components follow all the rules of React, disabling them may result in undefined behavior. eslint-disable-next-line react-hooks/exhaustive-deps (6:6)
24
+```
25
+
26
+
\ No newline at end of file
compiler/packages/babel-plugin-react-forget/src/__tests__/fixtures/compiler/error.sketchy-code-exhaustive-deps.js
renamed
compiler/packages/babel-plugin-react-forget/src/__tests__/fixtures/compiler/error.sketchy-code-rules-of-hooks.expect.md
new
+27
@@ -0,0 +1,27 @@
1
+
2
+## Input
3
+
4
+```javascript
5
+/* eslint-disable react-hooks/rules-of-hooks */
6
+function lowercasecomponent() {
7
+ const x = [];
8
+ return <div>{x}</div>;
9
+}
10
+/* eslint-enable react-hooks/rules-of-hooks */
11
+
12
+export const FIXTURE_ENTRYPOINT = {
13
+ fn: lowercasecomponent,
14
+ params: [],
15
+ isComponent: false,
16
+};
17
+
18
+```
19
+
20
+
21
+## Error
22
+
23
+```
24
+[ReactForget] InvalidReact: React Forget has bailed out of optimizing this component as one or more React eslint rules were disabled. React Forget only works when your components follow all the rules of React, disabling them may result in undefined behavior. eslint-disable react-hooks/rules-of-hooks (1:1)
25
+```
26
+
27
+
\ No newline at end of file
compiler/packages/babel-plugin-react-forget/src/__tests__/fixtures/compiler/error.sketchy-code-rules-of-hooks.js
renamed
compiler/packages/babel-plugin-react-forget/src/__tests__/fixtures/compiler/sketchy-code-exhaustive-deps.expect.md
deleted
-35
@@ -1,35 +0,0 @@
1
-
2
-## Input
3
-
4
-```javascript
5
-function Component() {
6
- const item = [];
7
- const foo = useCallback(
8
- () => {
9
- item.push(1);
10
- }, // eslint-disable-next-line react-hooks/exhaustive-deps
11
- []
12
- );
13
-
14
- return <Button foo={foo} />;
15
-}
16
-
17
-```
18
-
19
-## Code
20
-
21
-```javascript
22
-function Component() {
23
- const item = [];
24
- const foo = useCallback(
25
- () => {
26
- item.push(1);
27
- }, // eslint-disable-next-line react-hooks/exhaustive-deps
28
- []
29
- );
30
-
31
- return <Button foo={foo} />;
32
-}
33
-
34
-```
35
-
\ No newline at end of file
compiler/packages/babel-plugin-react-forget/src/__tests__/fixtures/compiler/sketchy-code-rules-of-hooks.expect.md
deleted
-37
@@ -1,37 +0,0 @@
1
-
2
-## Input
3
-
4
-```javascript
5
-/* eslint-disable react-hooks/rules-of-hooks */
6
-function lowercasecomponent() {
7
- const x = [];
8
- return <div>{x}</div>;
9
-}
10
-/* eslint-enable react-hooks/rules-of-hooks */
11
-
12
-export const FIXTURE_ENTRYPOINT = {
13
- fn: lowercasecomponent,
14
- params: [],
15
- isComponent: false,
16
-};
17
-
18
-```
19
-
20
-## Code
21
-
22
-```javascript
23
-/* eslint-disable react-hooks/rules-of-hooks */
24
-function lowercasecomponent() {
25
- const x = [];
26
- return <div>{x}</div>;
27
-}
28
-/* eslint-enable react-hooks/rules-of-hooks */
29
-
30
-export const FIXTURE_ENTRYPOINT = {
31
- fn: lowercasecomponent,
32
- params: [],
33
- isComponent: false,
34
-};
35
-
36
-```
37
-
\ No newline at end of file