mirror of
https://github.com/facebook/react.git
synced 2025-11-01 09:12:30 +00:00
[compiler][rfc] Switch eslint-plugin-react-compiler to single rule for Meta compat
Now that the official way to use React Compiler's linting is via `eslint-plugin-react-hooks` v7, eslint-plugin-react-compiler isn't strictly necessary anymore. However, at Meta our linting setup makes it a bit tedious to use the current eprh setup with lots of rules. Here I'm experimenting with making eslint-plugin-react-compiler just report all issues under one rule, to make it a bit easier to sync internally. Unclear if we even need to land this or just use it to help figure out the migration.
This commit is contained in:
+15
-23
@@ -5,22 +5,15 @@
|
||||
* LICENSE file in the root directory of this source tree.
|
||||
*/
|
||||
|
||||
import {
|
||||
ErrorCategory,
|
||||
getRuleForCategory,
|
||||
} from 'babel-plugin-react-compiler/src/CompilerError';
|
||||
import {normalizeIndent, testRule, makeTestCaseError} from './shared-utils';
|
||||
import {allRules} from '../src/rules/ReactCompilerRule';
|
||||
import ReactCompilerRule from '../src/rules/ReactCompilerRule';
|
||||
|
||||
testRule(
|
||||
'no impure function calls rule',
|
||||
allRules[getRuleForCategory(ErrorCategory.Purity).name].rule,
|
||||
{
|
||||
valid: [],
|
||||
invalid: [
|
||||
{
|
||||
name: 'Known impure function calls are caught',
|
||||
code: normalizeIndent`
|
||||
testRule('no impure function calls rule', ReactCompilerRule, {
|
||||
valid: [],
|
||||
invalid: [
|
||||
{
|
||||
name: 'Known impure function calls are caught',
|
||||
code: normalizeIndent`
|
||||
function Component() {
|
||||
const date = Date.now();
|
||||
const now = performance.now();
|
||||
@@ -28,12 +21,11 @@ testRule(
|
||||
return <Foo date={date} now={now} rand={rand} />;
|
||||
}
|
||||
`,
|
||||
errors: [
|
||||
makeTestCaseError('Cannot call impure function during render'),
|
||||
makeTestCaseError('Cannot call impure function during render'),
|
||||
makeTestCaseError('Cannot call impure function during render'),
|
||||
],
|
||||
},
|
||||
],
|
||||
},
|
||||
);
|
||||
errors: [
|
||||
makeTestCaseError('Cannot call impure function during render'),
|
||||
makeTestCaseError('Cannot call impure function during render'),
|
||||
makeTestCaseError('Cannot call impure function during render'),
|
||||
],
|
||||
},
|
||||
],
|
||||
});
|
||||
|
||||
@@ -5,30 +5,23 @@
|
||||
* LICENSE file in the root directory of this source tree.
|
||||
*/
|
||||
|
||||
import {
|
||||
ErrorCategory,
|
||||
getRuleForCategory,
|
||||
} from 'babel-plugin-react-compiler/src/CompilerError';
|
||||
import {normalizeIndent, makeTestCaseError, testRule} from './shared-utils';
|
||||
import {allRules} from '../src/rules/ReactCompilerRule';
|
||||
import {AllRules} from '../src/rules/ReactCompilerRule';
|
||||
|
||||
testRule(
|
||||
'rules-of-hooks',
|
||||
allRules[getRuleForCategory(ErrorCategory.Hooks).name].rule,
|
||||
{
|
||||
valid: [
|
||||
{
|
||||
name: 'Basic example',
|
||||
code: normalizeIndent`
|
||||
testRule('rules-of-hooks', AllRules, {
|
||||
valid: [
|
||||
{
|
||||
name: 'Basic example',
|
||||
code: normalizeIndent`
|
||||
function Component() {
|
||||
useHook();
|
||||
return <div>Hello world</div>;
|
||||
}
|
||||
`,
|
||||
},
|
||||
{
|
||||
name: 'Violation with Flow suppression',
|
||||
code: `
|
||||
},
|
||||
{
|
||||
name: 'Violation with Flow suppression',
|
||||
code: `
|
||||
// Valid since error already suppressed with flow.
|
||||
function useHook() {
|
||||
if (cond) {
|
||||
@@ -37,11 +30,11 @@ testRule(
|
||||
}
|
||||
}
|
||||
`,
|
||||
},
|
||||
{
|
||||
// OK because invariants are only meant for the compiler team's consumption
|
||||
name: '[Invariant] Defined after use',
|
||||
code: normalizeIndent`
|
||||
},
|
||||
{
|
||||
// OK because invariants are only meant for the compiler team's consumption
|
||||
name: '[Invariant] Defined after use',
|
||||
code: normalizeIndent`
|
||||
function Component(props) {
|
||||
let y = function () {
|
||||
m(x);
|
||||
@@ -52,49 +45,42 @@ testRule(
|
||||
return y;
|
||||
}
|
||||
`,
|
||||
},
|
||||
{
|
||||
name: "Classes don't throw",
|
||||
code: normalizeIndent`
|
||||
},
|
||||
{
|
||||
name: "Classes don't throw",
|
||||
code: normalizeIndent`
|
||||
class Foo {
|
||||
#bar() {}
|
||||
}
|
||||
`,
|
||||
},
|
||||
],
|
||||
invalid: [
|
||||
{
|
||||
name: 'Simple violation',
|
||||
code: normalizeIndent`
|
||||
},
|
||||
],
|
||||
invalid: [
|
||||
{
|
||||
name: 'Simple violation',
|
||||
code: normalizeIndent`
|
||||
function useConditional() {
|
||||
if (cond) {
|
||||
useConditionalHook();
|
||||
}
|
||||
}
|
||||
`,
|
||||
errors: [
|
||||
makeTestCaseError(
|
||||
'Hooks must always be called in a consistent order',
|
||||
),
|
||||
],
|
||||
},
|
||||
{
|
||||
name: 'Multiple diagnostics within the same function are surfaced',
|
||||
code: normalizeIndent`
|
||||
errors: [
|
||||
makeTestCaseError('Hooks must always be called in a consistent order'),
|
||||
],
|
||||
},
|
||||
{
|
||||
name: 'Multiple diagnostics within the same function are surfaced',
|
||||
code: normalizeIndent`
|
||||
function useConditional() {
|
||||
cond ?? useConditionalHook();
|
||||
props.cond && useConditionalHook();
|
||||
return <div>Hello world</div>;
|
||||
}`,
|
||||
errors: [
|
||||
makeTestCaseError(
|
||||
'Hooks must always be called in a consistent order',
|
||||
),
|
||||
makeTestCaseError(
|
||||
'Hooks must always be called in a consistent order',
|
||||
),
|
||||
],
|
||||
},
|
||||
],
|
||||
},
|
||||
);
|
||||
errors: [
|
||||
makeTestCaseError('Hooks must always be called in a consistent order'),
|
||||
makeTestCaseError('Hooks must always be called in a consistent order'),
|
||||
],
|
||||
},
|
||||
],
|
||||
});
|
||||
|
||||
+11
-19
@@ -5,22 +5,15 @@
|
||||
* LICENSE file in the root directory of this source tree.
|
||||
*/
|
||||
|
||||
import {
|
||||
ErrorCategory,
|
||||
getRuleForCategory,
|
||||
} from 'babel-plugin-react-compiler/src/CompilerError';
|
||||
import {normalizeIndent, testRule, makeTestCaseError} from './shared-utils';
|
||||
import {allRules} from '../src/rules/ReactCompilerRule';
|
||||
import ReactCompilerRule from '../src/rules/ReactCompilerRule';
|
||||
|
||||
testRule(
|
||||
'no ambiguous JSX rule',
|
||||
allRules[getRuleForCategory(ErrorCategory.ErrorBoundaries).name].rule,
|
||||
{
|
||||
valid: [],
|
||||
invalid: [
|
||||
{
|
||||
name: 'JSX in try blocks are warned against',
|
||||
code: normalizeIndent`
|
||||
testRule('no ambiguous JSX rule', ReactCompilerRule, {
|
||||
valid: [],
|
||||
invalid: [
|
||||
{
|
||||
name: 'JSX in try blocks are warned against',
|
||||
code: normalizeIndent`
|
||||
function Component(props) {
|
||||
let el;
|
||||
try {
|
||||
@@ -31,8 +24,7 @@ testRule(
|
||||
return el;
|
||||
}
|
||||
`,
|
||||
errors: [makeTestCaseError('Avoid constructing JSX within try/catch')],
|
||||
},
|
||||
],
|
||||
},
|
||||
);
|
||||
errors: [makeTestCaseError('Avoid constructing JSX within try/catch')],
|
||||
},
|
||||
],
|
||||
});
|
||||
|
||||
+27
-41
@@ -4,22 +4,15 @@
|
||||
* This source code is licensed under the MIT license found in the
|
||||
* LICENSE file in the root directory of this source tree.
|
||||
*/
|
||||
import {
|
||||
ErrorCategory,
|
||||
getRuleForCategory,
|
||||
} from 'babel-plugin-react-compiler/src/CompilerError';
|
||||
import {normalizeIndent, makeTestCaseError, testRule} from './shared-utils';
|
||||
import {allRules} from '../src/rules/ReactCompilerRule';
|
||||
import ReactCompilerRule from '../src/rules/ReactCompilerRule';
|
||||
|
||||
testRule(
|
||||
'no-capitalized-calls',
|
||||
allRules[getRuleForCategory(ErrorCategory.CapitalizedCalls).name].rule,
|
||||
{
|
||||
valid: [],
|
||||
invalid: [
|
||||
{
|
||||
name: 'Simple violation',
|
||||
code: normalizeIndent`
|
||||
testRule('no-capitalized-calls', ReactCompilerRule, {
|
||||
valid: [],
|
||||
invalid: [
|
||||
{
|
||||
name: 'Simple violation',
|
||||
code: normalizeIndent`
|
||||
import Child from './Child';
|
||||
function Component() {
|
||||
return <>
|
||||
@@ -27,15 +20,13 @@ testRule(
|
||||
</>;
|
||||
}
|
||||
`,
|
||||
errors: [
|
||||
makeTestCaseError(
|
||||
'Capitalized functions are reserved for components',
|
||||
),
|
||||
],
|
||||
},
|
||||
{
|
||||
name: 'Method call violation',
|
||||
code: normalizeIndent`
|
||||
errors: [
|
||||
makeTestCaseError('Capitalized functions are reserved for components'),
|
||||
],
|
||||
},
|
||||
{
|
||||
name: 'Method call violation',
|
||||
code: normalizeIndent`
|
||||
import myModule from './MyModule';
|
||||
function Component() {
|
||||
return <>
|
||||
@@ -43,15 +34,13 @@ testRule(
|
||||
</>;
|
||||
}
|
||||
`,
|
||||
errors: [
|
||||
makeTestCaseError(
|
||||
'Capitalized functions are reserved for components',
|
||||
),
|
||||
],
|
||||
},
|
||||
{
|
||||
name: 'Multiple diagnostics within the same function are surfaced',
|
||||
code: normalizeIndent`
|
||||
errors: [
|
||||
makeTestCaseError('Capitalized functions are reserved for components'),
|
||||
],
|
||||
},
|
||||
{
|
||||
name: 'Multiple diagnostics within the same function are surfaced',
|
||||
code: normalizeIndent`
|
||||
import Child1 from './Child1';
|
||||
import MyModule from './MyModule';
|
||||
function Component() {
|
||||
@@ -60,12 +49,9 @@ testRule(
|
||||
{MyModule.Child2()}
|
||||
</>;
|
||||
}`,
|
||||
errors: [
|
||||
makeTestCaseError(
|
||||
'Capitalized functions are reserved for components',
|
||||
),
|
||||
],
|
||||
},
|
||||
],
|
||||
},
|
||||
);
|
||||
errors: [
|
||||
makeTestCaseError('Capitalized functions are reserved for components'),
|
||||
],
|
||||
},
|
||||
],
|
||||
});
|
||||
|
||||
+11
-19
@@ -5,30 +5,22 @@
|
||||
* LICENSE file in the root directory of this source tree.
|
||||
*/
|
||||
|
||||
import {
|
||||
ErrorCategory,
|
||||
getRuleForCategory,
|
||||
} from 'babel-plugin-react-compiler/src/CompilerError';
|
||||
import {normalizeIndent, testRule, makeTestCaseError} from './shared-utils';
|
||||
import {allRules} from '../src/rules/ReactCompilerRule';
|
||||
import ReactCompilerRule from '../src/rules/ReactCompilerRule';
|
||||
|
||||
testRule(
|
||||
'no ref access in render rule',
|
||||
allRules[getRuleForCategory(ErrorCategory.Refs).name].rule,
|
||||
{
|
||||
valid: [],
|
||||
invalid: [
|
||||
{
|
||||
name: 'validate against simple ref access in render',
|
||||
code: normalizeIndent`
|
||||
testRule('no ref access in render rule', ReactCompilerRule, {
|
||||
valid: [],
|
||||
invalid: [
|
||||
{
|
||||
name: 'validate against simple ref access in render',
|
||||
code: normalizeIndent`
|
||||
function Component(props) {
|
||||
const ref = useRef(null);
|
||||
const value = ref.current;
|
||||
return value;
|
||||
}
|
||||
`,
|
||||
errors: [makeTestCaseError('Cannot access refs during render')],
|
||||
},
|
||||
],
|
||||
},
|
||||
);
|
||||
errors: [makeTestCaseError('Cannot access refs during render')],
|
||||
},
|
||||
],
|
||||
});
|
||||
|
||||
@@ -5,19 +5,10 @@
|
||||
* LICENSE file in the root directory of this source tree.
|
||||
*/
|
||||
|
||||
import {
|
||||
ErrorCategory,
|
||||
getRuleForCategory,
|
||||
} from 'babel-plugin-react-compiler/src/CompilerError';
|
||||
import {
|
||||
normalizeIndent,
|
||||
testRule,
|
||||
makeTestCaseError,
|
||||
TestRecommendedRules,
|
||||
} from './shared-utils';
|
||||
import {allRules} from '../src/rules/ReactCompilerRule';
|
||||
import {normalizeIndent, testRule, makeTestCaseError} from './shared-utils';
|
||||
import {AllRules} from '../src/rules/ReactCompilerRule';
|
||||
|
||||
testRule('plugin-recommended', TestRecommendedRules, {
|
||||
testRule('plugin-recommended', AllRules, {
|
||||
valid: [
|
||||
{
|
||||
name: 'Basic example with component syntax',
|
||||
|
||||
+3
-6
@@ -6,11 +6,8 @@
|
||||
*/
|
||||
|
||||
import {RuleTester} from 'eslint';
|
||||
import {
|
||||
CompilerTestCases,
|
||||
normalizeIndent,
|
||||
TestRecommendedRules,
|
||||
} from './shared-utils';
|
||||
import {CompilerTestCases, normalizeIndent} from './shared-utils';
|
||||
import ReactCompilerRule from '../src/rules/ReactCompilerRule';
|
||||
|
||||
const tests: CompilerTestCases = {
|
||||
valid: [
|
||||
@@ -62,4 +59,4 @@ const eslintTester = new RuleTester({
|
||||
// @ts-ignore[2353] - outdated types
|
||||
parser: require.resolve('@typescript-eslint/parser'),
|
||||
});
|
||||
eslintTester.run('react-compiler', TestRecommendedRules, tests);
|
||||
eslintTester.run('react-compiler', ReactCompilerRule, tests);
|
||||
|
||||
@@ -1,8 +1,5 @@
|
||||
import {RuleTester as ESLintTester, Rule} from 'eslint';
|
||||
import {type ErrorCategory} from 'babel-plugin-react-compiler/src/CompilerError';
|
||||
import escape from 'regexp.escape';
|
||||
import {configs} from '../src/index';
|
||||
import {allRules} from '../src/rules/ReactCompilerRule';
|
||||
|
||||
/**
|
||||
* A string template tag that removes padding from the left side of multi-line strings
|
||||
@@ -46,31 +43,4 @@ export function testRule(
|
||||
eslintTester.run(name, rule, tests);
|
||||
}
|
||||
|
||||
/**
|
||||
* Aggregates all recommended rules from the plugin.
|
||||
*/
|
||||
export const TestRecommendedRules: Rule.RuleModule = {
|
||||
meta: {
|
||||
type: 'problem',
|
||||
docs: {
|
||||
description: 'Disallow capitalized function calls',
|
||||
category: 'Possible Errors',
|
||||
recommended: true,
|
||||
},
|
||||
// validation is done at runtime with zod
|
||||
schema: [{type: 'object', additionalProperties: true}],
|
||||
},
|
||||
create(context) {
|
||||
for (const ruleConfig of Object.values(
|
||||
configs.recommended.plugins['react-compiler'].rules,
|
||||
)) {
|
||||
const listener = ruleConfig.rule.create(context);
|
||||
if (Object.entries(listener).length !== 0) {
|
||||
throw new Error('TODO: handle rules that return listeners to eslint');
|
||||
}
|
||||
}
|
||||
return {};
|
||||
},
|
||||
};
|
||||
|
||||
test('no test', () => {});
|
||||
|
||||
@@ -5,37 +5,24 @@
|
||||
* LICENSE file in the root directory of this source tree.
|
||||
*/
|
||||
|
||||
import {type Linter} from 'eslint';
|
||||
import {
|
||||
allRules,
|
||||
mapErrorSeverityToESlint,
|
||||
recommendedRules,
|
||||
} from './rules/ReactCompilerRule';
|
||||
import ReactCompilerRule from './rules/ReactCompilerRule';
|
||||
|
||||
const meta = {
|
||||
name: 'eslint-plugin-react-compiler',
|
||||
};
|
||||
|
||||
const configs = {
|
||||
recommended: {
|
||||
plugins: {
|
||||
'react-compiler': {
|
||||
rules: allRules,
|
||||
module.exports = {
|
||||
rules: {
|
||||
'react-compiler': ReactCompilerRule,
|
||||
},
|
||||
configs: {
|
||||
recommended: {
|
||||
plugins: {
|
||||
'react-compiler': {
|
||||
rules: {
|
||||
'react-compiler': ReactCompilerRule,
|
||||
},
|
||||
},
|
||||
},
|
||||
rules: {
|
||||
'react-compiler/react-compiler': 'error',
|
||||
},
|
||||
},
|
||||
rules: Object.fromEntries(
|
||||
Object.entries(recommendedRules).map(([name, ruleConfig]) => {
|
||||
return [
|
||||
'react-compiler/' + name,
|
||||
mapErrorSeverityToESlint(ruleConfig.severity),
|
||||
];
|
||||
}),
|
||||
) as Record<string, Linter.StringSeverity>,
|
||||
},
|
||||
};
|
||||
|
||||
const rules = Object.fromEntries(
|
||||
Object.entries(allRules).map(([name, {rule}]) => [name, rule]),
|
||||
);
|
||||
|
||||
export {configs, rules, meta};
|
||||
|
||||
@@ -14,7 +14,7 @@ import {
|
||||
import type {Linter, Rule} from 'eslint';
|
||||
import runReactCompiler, {RunCacheEntry} from '../shared/RunReactCompiler';
|
||||
import {
|
||||
ErrorSeverity,
|
||||
ErrorCategory,
|
||||
LintRulePreset,
|
||||
LintRules,
|
||||
type LintRule,
|
||||
@@ -108,14 +108,15 @@ function hasFlowSuppression(
|
||||
return false;
|
||||
}
|
||||
|
||||
function makeRule(rule: LintRule): Rule.RuleModule {
|
||||
function makeRule(rules: Array<LintRule>): Rule.RuleModule {
|
||||
const categories = new Set(rules.map(rule => rule.category));
|
||||
const create = (context: Rule.RuleContext): Rule.RuleListener => {
|
||||
const result = getReactCompilerResult(context);
|
||||
|
||||
for (const event of result.events) {
|
||||
if (event.kind === 'CompileError') {
|
||||
const detail = event.detail;
|
||||
if (detail.category === rule.category) {
|
||||
if (categories.has(detail.category)) {
|
||||
const loc = detail.primaryLocation();
|
||||
if (loc == null || typeof loc === 'symbol') {
|
||||
continue;
|
||||
@@ -150,8 +151,8 @@ function makeRule(rule: LintRule): Rule.RuleModule {
|
||||
meta: {
|
||||
type: 'problem',
|
||||
docs: {
|
||||
description: rule.description,
|
||||
recommended: rule.preset === LintRulePreset.Recommended,
|
||||
description: 'React Compiler diagnostics',
|
||||
recommended: true,
|
||||
},
|
||||
fixable: 'code',
|
||||
hasSuggestions: true,
|
||||
@@ -162,47 +163,13 @@ function makeRule(rule: LintRule): Rule.RuleModule {
|
||||
};
|
||||
}
|
||||
|
||||
type RulesConfig = {
|
||||
[name: string]: {rule: Rule.RuleModule; severity: ErrorSeverity};
|
||||
};
|
||||
export default makeRule(
|
||||
LintRules.filter(
|
||||
rule =>
|
||||
rule.preset === LintRulePreset.Recommended ||
|
||||
rule.preset === LintRulePreset.RecommendedLatest ||
|
||||
rule.category === ErrorCategory.CapitalizedCalls,
|
||||
),
|
||||
);
|
||||
|
||||
export const allRules: RulesConfig = LintRules.reduce((acc, rule) => {
|
||||
acc[rule.name] = {rule: makeRule(rule), severity: rule.severity};
|
||||
return acc;
|
||||
}, {} as RulesConfig);
|
||||
|
||||
export const recommendedRules: RulesConfig = LintRules.filter(
|
||||
rule => rule.preset === LintRulePreset.Recommended,
|
||||
).reduce((acc, rule) => {
|
||||
acc[rule.name] = {rule: makeRule(rule), severity: rule.severity};
|
||||
return acc;
|
||||
}, {} as RulesConfig);
|
||||
|
||||
export const recommendedLatestRules: RulesConfig = LintRules.filter(
|
||||
rule =>
|
||||
rule.preset === LintRulePreset.Recommended ||
|
||||
rule.preset === LintRulePreset.RecommendedLatest,
|
||||
).reduce((acc, rule) => {
|
||||
acc[rule.name] = {rule: makeRule(rule), severity: rule.severity};
|
||||
return acc;
|
||||
}, {} as RulesConfig);
|
||||
|
||||
export function mapErrorSeverityToESlint(
|
||||
severity: ErrorSeverity,
|
||||
): Linter.StringSeverity {
|
||||
switch (severity) {
|
||||
case ErrorSeverity.Error: {
|
||||
return 'error';
|
||||
}
|
||||
case ErrorSeverity.Warning: {
|
||||
return 'warn';
|
||||
}
|
||||
case ErrorSeverity.Hint:
|
||||
case ErrorSeverity.Off: {
|
||||
return 'off';
|
||||
}
|
||||
default: {
|
||||
assertExhaustive(severity, `Unhandled severity: ${severity}`);
|
||||
}
|
||||
}
|
||||
}
|
||||
export const AllRules = makeRule(LintRules);
|
||||
|
||||
Reference in New Issue
Block a user