[compiler][eslint] Use logger callback instead of exceptions to report eslint diagnostics
--- * panicThreshold: `all_errors` -> `none` * inject an error logger through compiler config (instead of using exceptions) We currently report at most one lint warning per file, this lets us exhaustively report all available ones (see new test fixture for example) ghstack-source-id: 5299315574d11929efc39ee8f6033e3035d1e378 Pull Request resolved: https://github.com/facebook/react/pull/30336
Mofei Zhang committed
Jul 15, 2024 at 17:44 UTC
e307a4954446d1aeda9cbe23fda914289406c5f7
2 files changed
+60
-24
compiler/packages/eslint-plugin-react-compiler/__tests__/ReactCompilerRule-test.ts
+30
@@ -149,6 +149,36 @@ const tests: CompilerTestCases = {
149
},
150
],
151
},
152
+ {
153
+ name: "Multiple diagnostics are surfaced",
154
+ options: [
155
+ {
156
+ reportableLevels: new Set([
157
+ ErrorSeverity.Todo,
158
+ ErrorSeverity.InvalidReact,
159
+ ]),
160
+ },
161
+ ],
162
+ code: normalizeIndent`
163
+ function Foo(x) {
164
+ var y = 1;
165
+ return <div>{y * x}</div>;
166
+ }
167
+ function Bar(props) {
168
+ props.a.b = 2;
169
+ return <div>{props.c}</div>
170
+ }`,
171
+ errors: [
172
+ {
173
+ message:
174
+ "(BuildHIR::lowerStatement) Handle var kinds in VariableDeclaration",
175
+ },
176
+ {
177
+ message:
178
+ "Mutating component props or hook arguments is not allowed. Consider using a local variable instead",
179
+ },
180
+ ],
181
+ },
182
],
183
};
184
compiler/packages/eslint-plugin-react-compiler/src/rules/ReactCompilerRule.ts
+30
-24
@@ -15,9 +15,9 @@ import BabelPluginReactCompiler, {
15
ErrorSeverity,
16
parsePluginOptions,
17
validateEnvironmentConfig,
18
- type CompilerError,
18
type PluginOptions,
19
} from "babel-plugin-react-compiler/src";
20
+import { Logger } from "babel-plugin-react-compiler/src/Entrypoint";
21
import type { Rule } from "eslint";
22
import * as HermesParser from "hermes-parser";
23
@@ -29,10 +29,6 @@ function assertExhaustive(_: never, errorMsg: string): never {
29
throw new Error(errorMsg);
30
}
31
32
-function isReactCompilerError(err: Error): err is CompilerError {
33
- return err.name === "ReactCompilerError";
34
-}
35
-
32
const DEFAULT_REPORTABLE_LEVELS = new Set([
33
ErrorSeverity.InvalidReact,
34
ErrorSeverity.InvalidJS,
@@ -105,7 +101,7 @@ function makeSuggestions(
101
const COMPILER_OPTIONS: Partial<PluginOptions> = {
102
noEmit: true,
103
compilationMode: "infer",
108
- panicThreshold: "all_errors",
104
+ panicThreshold: "none",
105
};
106
107
const rule: Rule.RuleModule = {
@@ -137,6 +133,33 @@ const rule: Rule.RuleModule = {
133
...parsePluginOptions(userOpts),
134
...COMPILER_OPTIONS,
135
};
136
+ const userLogger: Logger | null = options.logger;
137
+ options.logger = {
138
+ logEvent: (filename, event): void => {
139
+ userLogger?.logEvent(filename, event);
140
+ if (event.kind === "CompileError") {
141
+ const detail = event.detail;
142
+ if (!isReportableDiagnostic(detail)) {
143
+ return;
144
+ }
145
+ if (hasFlowSuppression(detail.loc, "react-rule-hook")) {
146
+ // If Flow already caught this error, we don't need to report it again.
147
+ return;
148
+ }
149
+ const loc =
150
+ detail.loc == null || typeof detail.loc == "symbol"
151
+ ? event.fnLoc
152
+ : detail.loc;
153
+ if (loc != null) {
154
+ context.report({
155
+ message: detail.reason,
156
+ loc,
157
+ suggest: makeSuggestions(detail),
158
+ });
159
+ }
160
+ }
161
+ },
162
+ };
163
164
try {
165
options.environment = validateEnvironmentConfig(
@@ -206,24 +229,7 @@ const rule: Rule.RuleModule = {
229
babelrc: false,
230
});
231
} catch (err) {
209
- if (isReactCompilerError(err) && Array.isArray(err.details)) {
210
- for (const detail of err.details) {
211
- if (!isReportableDiagnostic(detail)) {
212
- continue;
213
- }
214
- if (hasFlowSuppression(detail.loc, "react-rule-hook")) {
215
- // If Flow already caught this error, we don't need to report it again.
216
- continue;
217
- }
218
- context.report({
219
- message: detail.reason,
220
- loc: detail.loc,
221
- suggest: makeSuggestions(detail),
222
- });
223
- }
224
- } else {
225
- options.logger?.logEvent("", err);
226
- }
232
+ /* errors handled by injected logger */
233
}
234
}
235
return {};