From bc145f6f1fb469a7e09608c1714d074734620871 Mon Sep 17 00:00:00 2001 From: Joe Savona Date: Mon, 29 Jan 2024 16:58:49 -0500 Subject: [PATCH] Support customizable eslint suppressions 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. --- .../src/Entrypoint/EslintSuppression.ts | 27 +++++++++---------- .../src/Entrypoint/Options.ts | 12 +++++++++ .../src/Entrypoint/Program.ts | 10 ++++++- ...ut-on-suppression-of-custom-rule.expect.md | 27 +++++++++++++++++++ ...r.bailout-on-suppression-of-custom-rule.js | 10 +++++++ .../fixture-test-utils/src/compiler-utils.ts | 7 +++++ 6 files changed, 78 insertions(+), 15 deletions(-) create mode 100644 compiler/packages/babel-plugin-react-forget/src/__tests__/fixtures/compiler/error.bailout-on-suppression-of-custom-rule.expect.md create mode 100644 compiler/packages/babel-plugin-react-forget/src/__tests__/fixtures/compiler/error.bailout-on-suppression-of-custom-rule.js diff --git a/compiler/packages/babel-plugin-react-forget/src/Entrypoint/EslintSuppression.ts b/compiler/packages/babel-plugin-react-forget/src/Entrypoint/EslintSuppression.ts index f62447088e..eec9fd0237 100644 --- a/compiler/packages/babel-plugin-react-forget/src/Entrypoint/EslintSuppression.ts +++ b/compiler/packages/babel-plugin-react-forget/src/Entrypoint/EslintSuppression.ts @@ -71,11 +71,20 @@ export function filterEslintSuppressionsThatAffectFunction( } export function findProgramEslintSuppressions( - programComments: Array + programComments: Array, + ruleNames: Array ): Array { const suppressionRanges: Array = []; let disableComment: t.Comment | null = null; let enableComment: t.Comment | null = null; + + const rulePattern = `(${ruleNames.join("|")})`; + const disableNextLinePattern = new RegExp( + `eslint-disable-next-line ${rulePattern}` + ); + const disablePattern = new RegExp(`eslint-disable ${rulePattern}`); + const enablePattern = new RegExp(`eslint-enable ${rulePattern}`); + for (const comment of programComments) { if (comment.start == null || comment.end == null) { continue; @@ -87,27 +96,17 @@ export function findProgramEslintSuppressions( * CommentLine within the block. */ disableComment == null && - /eslint-disable-next-line react-hooks\/(exhaustive-deps|rules-of-hooks)/.test( - comment.value - ) + disableNextLinePattern.test(comment.value) ) { disableComment = comment; enableComment = comment; } - if ( - /eslint-disable react-hooks\/(exhaustive-deps|rules-of-hooks)/.test( - comment.value - ) - ) { + if (disablePattern.test(comment.value)) { disableComment = comment; } - if ( - /eslint-enable react-hooks\/(exhaustive-deps|rules-of-hooks)/.test( - comment.value - ) - ) { + if (enablePattern.test(comment.value)) { enableComment = comment; } diff --git a/compiler/packages/babel-plugin-react-forget/src/Entrypoint/Options.ts b/compiler/packages/babel-plugin-react-forget/src/Entrypoint/Options.ts index bc4dbe5e22..90a8bb4fd2 100644 --- a/compiler/packages/babel-plugin-react-forget/src/Entrypoint/Options.ts +++ b/compiler/packages/babel-plugin-react-forget/src/Entrypoint/Options.ts @@ -91,6 +91,17 @@ export type PluginOptions = { * ``` */ enableUseMemoCachePolyfill: boolean; + + /** + * By default React Compiler will skip compilation of code that suppresses the default + * React ESLint rules, since this is a strong indication that the code may be breaking React rules + * in some way. + * + * Use eslintSuppressionRules to pass a custom set of rule names: any code which suppresses the + * provided rules will skip compilation. To disable this feature (never bailout of compilation + * even if the default ESLint is suppressed), pass an empty array. + */ + eslintSuppressionRules?: Array | null | undefined; }; const CompilationModeSchema = z.enum([ @@ -157,6 +168,7 @@ export const defaultOptions: PluginOptions = { gating: null, noEmit: false, enableUseMemoCachePolyfill: false, + eslintSuppressionRules: null, } as const; export function parsePluginOptions(obj: unknown): PluginOptions { diff --git a/compiler/packages/babel-plugin-react-forget/src/Entrypoint/Program.ts b/compiler/packages/babel-plugin-react-forget/src/Entrypoint/Program.ts index a918c24521..ea394c9ea6 100644 --- a/compiler/packages/babel-plugin-react-forget/src/Entrypoint/Program.ts +++ b/compiler/packages/babel-plugin-react-forget/src/Entrypoint/Program.ts @@ -177,6 +177,11 @@ export function createNewFunctionNode( */ const ALREADY_COMPILED: WeakSet | Set = new (WeakSet ?? Set)(); +const DEFAULT_ESLINT_SUPPRESSIONS = [ + "react-hooks/exhaustive-deps", + "react-hooks/rules-of-hooks", +]; + export function compileProgram( program: NodePath, pass: CompilerPass @@ -189,7 +194,10 @@ export function compileProgram( * we may still need to run Forget's analysis on every function (even if we * have already encountered errors) for reporting. */ - const eslintSuppressions = findProgramEslintSuppressions(pass.comments); + const eslintSuppressions = findProgramEslintSuppressions( + pass.comments, + options.eslintSuppressionRules ?? DEFAULT_ESLINT_SUPPRESSIONS + ); const lintError = suppressionsToCompilerError(eslintSuppressions); let hasCriticalError = lintError != null; const compiledFns: CompileResult[] = []; diff --git a/compiler/packages/babel-plugin-react-forget/src/__tests__/fixtures/compiler/error.bailout-on-suppression-of-custom-rule.expect.md b/compiler/packages/babel-plugin-react-forget/src/__tests__/fixtures/compiler/error.bailout-on-suppression-of-custom-rule.expect.md new file mode 100644 index 0000000000..703957512b --- /dev/null +++ b/compiler/packages/babel-plugin-react-forget/src/__tests__/fixtures/compiler/error.bailout-on-suppression-of-custom-rule.expect.md @@ -0,0 +1,27 @@ + +## Input + +```javascript +// @eslintSuppressionRules(my-app/react-rule) + +/* eslint-disable my-app/react-rule */ +function lowercasecomponent() { + "use forget"; + const x = []; + // eslint-disable-next-line my-app/react-rule + return
{x}
; +} +/* eslint-enable my-app/react-rule */ + +``` + + +## Error + +``` +[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 my-app/react-rule (3:3) + +[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 my-app/react-rule (7:7) +``` + + \ No newline at end of file diff --git a/compiler/packages/babel-plugin-react-forget/src/__tests__/fixtures/compiler/error.bailout-on-suppression-of-custom-rule.js b/compiler/packages/babel-plugin-react-forget/src/__tests__/fixtures/compiler/error.bailout-on-suppression-of-custom-rule.js new file mode 100644 index 0000000000..5935d1f0f2 --- /dev/null +++ b/compiler/packages/babel-plugin-react-forget/src/__tests__/fixtures/compiler/error.bailout-on-suppression-of-custom-rule.js @@ -0,0 +1,10 @@ +// @eslintSuppressionRules(my-app/react-rule) + +/* eslint-disable my-app/react-rule */ +function lowercasecomponent() { + "use forget"; + const x = []; + // eslint-disable-next-line my-app/react-rule + return
{x}
; +} +/* eslint-enable my-app/react-rule */ diff --git a/compiler/packages/fixture-test-utils/src/compiler-utils.ts b/compiler/packages/fixture-test-utils/src/compiler-utils.ts index 0b144de860..599dd07cb8 100644 --- a/compiler/packages/fixture-test-utils/src/compiler-utils.ts +++ b/compiler/packages/fixture-test-utils/src/compiler-utils.ts @@ -84,6 +84,12 @@ export function transformFixtureInput( panicThreshold = "NONE"; } + let eslintSuppressionRules: Array | null = null; + const match = /@eslintSuppressionRules\(([^)]+)\)/.exec(firstLine); + if (match != null) { + eslintSuppressionRules = match[1].split("|"); + } + const config = parseConfigPragmaFn(firstLine); const result = pluginFn( input, @@ -132,6 +138,7 @@ export function transformFixtureInput( panicThreshold, noEmit: false, enableUseMemoCachePolyfill, + eslintSuppressionRules, }, includeAst );