Skip to content

Commit af6598a

Browse files
committed
Add local eslint rule to validate markdown codeblocks with React Compiler
In facebook/react#34462 for example, we found an issue where the compiler was incorrectly validating an example straight from the docs. In order to find more issues like this + also provide more feedback to doc authors on valid/invalid patterns, this PR adds a new local eslint rule which validates all markdown codeblocks containing components/hooks with React Compiler. An autofixer is also provided. To express that a codeblock has an expected error, we can use the following metadata: ```ts // pseudo type def type MarkdownCodeBlockMetadata = { expectedErrors?: { 'react-compiler'?: number[]; }; }; ``` and can be used like so: ```` ```js {expectedErrors: {'react-compiler': [4]}} // ❌ setState directly in render function Component({value}) { const [count, setCount] = useState(0); setCount(value); // error on L4 return <div>{count}</div>; } ``` ```` Because this is defined as a local rule, we don't have the same granular reporting that `eslint-plugin-react-hooks` yet. I can look into that later but for now this first PR just sets us up with something basic.
1 parent 5b9a2ce commit af6598a

17 files changed

+1097
-6
lines changed

.eslintrc

Lines changed: 23 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -2,18 +2,38 @@
22
"root": true,
33
"extends": "next/core-web-vitals",
44
"parser": "@typescript-eslint/parser",
5-
"plugins": ["@typescript-eslint", "eslint-plugin-react-compiler"],
5+
"plugins": ["@typescript-eslint", "eslint-plugin-react-compiler", "local-rules"],
66
"rules": {
77
"no-unused-vars": "off",
88
"@typescript-eslint/no-unused-vars": ["error", {"varsIgnorePattern": "^_"}],
99
"react-hooks/exhaustive-deps": "error",
1010
"react/no-unknown-property": ["error", {"ignore": ["meta"]}],
11-
"react-compiler/react-compiler": "error"
11+
"react-compiler/react-compiler": "error",
12+
"local-rules/lint-markdown-code-blocks": "error"
1213
},
1314
"env": {
1415
"node": true,
1516
"commonjs": true,
1617
"browser": true,
1718
"es6": true
18-
}
19+
},
20+
"overrides": [
21+
{
22+
"files": ["src/content/**/*.md"],
23+
"processor": "local-rules/markdown",
24+
"parser": "espree",
25+
"parserOptions": {
26+
"ecmaVersion": "latest",
27+
"sourceType": "script"
28+
},
29+
"rules": {
30+
"no-unused-vars": "off",
31+
"@typescript-eslint/no-unused-vars": "off",
32+
"react-hooks/exhaustive-deps": "off",
33+
"react/no-unknown-property": "off",
34+
"react-compiler/react-compiler": "off",
35+
"local-rules/lint-markdown-code-blocks": "error"
36+
}
37+
}
38+
]
1939
}
Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,8 @@
1+
```jsx
2+
import {useState} from 'react';
3+
function Counter() {
4+
const [count, setCount] = useState(0);
5+
setCount(count + 1);
6+
return <div>{count}</div>;
7+
}
8+
```
Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,8 @@
1+
```jsx title="Counter" {expectedErrors: {'react-compiler': [99]}} {expectedErrors: {'react-compiler': [2]}}
2+
import {useState} from 'react';
3+
function Counter() {
4+
const [count, setCount] = useState(0);
5+
setCount(count + 1);
6+
return <div>{count}</div>;
7+
}
8+
```
Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,8 @@
1+
```jsx {expectedErrors: {'react-compiler': 'invalid'}}
2+
import {useState} from 'react';
3+
function Counter() {
4+
const [count, setCount] = useState(0);
5+
setCount(count + 1);
6+
return <div>{count}</div>;
7+
}
8+
```
Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,7 @@
1+
```bash
2+
setCount()
3+
```
4+
5+
```txt
6+
import {useState} from 'react';
7+
```
Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
```jsx {expectedErrors: {'react-compiler': [3]}}
2+
function Hello() {
3+
return <h1>Hello</h1>;
4+
}
5+
```
Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,8 @@
1+
```jsx {expectedErrors: {'react-compiler': [4]}}
2+
import {useState} from 'react';
3+
function Counter() {
4+
const [count, setCount] = useState(0);
5+
setCount(count + 1);
6+
return <div>{count}</div>;
7+
}
8+
```
Lines changed: 124 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,124 @@
1+
const assert = require('assert');
2+
const fs = require('fs');
3+
const path = require('path');
4+
const {ESLint} = require('eslint');
5+
const plugin = require('..');
6+
7+
const FIXTURES_DIR = path.join(
8+
__dirname,
9+
'fixtures',
10+
'src',
11+
'content'
12+
);
13+
14+
function createESLint({fix = false} = {}) {
15+
return new ESLint({
16+
useEslintrc: false,
17+
fix,
18+
plugins: {
19+
'local-rules': plugin,
20+
},
21+
overrideConfig: {
22+
processor: 'local-rules/markdown',
23+
plugins: ['local-rules'],
24+
rules: {
25+
'local-rules/lint-markdown-code-blocks': 'error',
26+
},
27+
parserOptions: {
28+
ecmaVersion: 2020,
29+
sourceType: 'module',
30+
},
31+
},
32+
});
33+
}
34+
35+
function readFixture(name) {
36+
return fs.readFileSync(path.join(FIXTURES_DIR, name), 'utf8');
37+
}
38+
39+
async function lintFixture(name, {fix = false} = {}) {
40+
const eslint = createESLint({fix});
41+
const filePath = path.join(FIXTURES_DIR, name);
42+
const markdown = readFixture(name);
43+
const [result] = await eslint.lintText(markdown, {filePath});
44+
return result;
45+
}
46+
47+
async function run() {
48+
const basicResult = await lintFixture('basic-error.md');
49+
assert.strictEqual(
50+
basicResult.messages.length,
51+
1,
52+
'expected one diagnostic'
53+
);
54+
assert(
55+
basicResult.messages[0].message.includes('Calling setState during render'),
56+
'expected message to mention setState during render'
57+
);
58+
59+
const suppressedResult = await lintFixture('suppressed-error.md');
60+
assert.strictEqual(
61+
suppressedResult.messages.length,
62+
0,
63+
'expected suppression metadata to silence diagnostic'
64+
);
65+
66+
const staleResult = await lintFixture('stale-expected-error.md');
67+
assert.strictEqual(
68+
staleResult.messages.length,
69+
1,
70+
'expected stale metadata error'
71+
);
72+
assert.strictEqual(
73+
staleResult.messages[0].message,
74+
'React Compiler expected error on line 3 was not triggered'
75+
);
76+
77+
const duplicateResult = await lintFixture('duplicate-metadata.md');
78+
assert.strictEqual(
79+
duplicateResult.messages.length,
80+
2,
81+
'expected duplicate metadata to surface compiler diagnostic and stale metadata notice'
82+
);
83+
const duplicateFixed = await lintFixture('duplicate-metadata.md', {
84+
fix: true,
85+
});
86+
assert(
87+
duplicateFixed.output.includes(
88+
"{expectedErrors: {'react-compiler': [4]}}"
89+
),
90+
'expected duplicates to be rewritten to a single canonical block'
91+
);
92+
assert(
93+
!duplicateFixed.output.includes('[99]'),
94+
'expected stale line numbers to be removed from metadata'
95+
);
96+
97+
const mixedLanguageResult = await lintFixture('mixed-language.md');
98+
assert.strictEqual(
99+
mixedLanguageResult.messages.length,
100+
0,
101+
'expected non-js code fences to be ignored'
102+
);
103+
104+
const malformedResult = await lintFixture('malformed-metadata.md');
105+
assert.strictEqual(
106+
malformedResult.messages.length,
107+
1,
108+
'expected malformed metadata to fall back to compiler diagnostics'
109+
);
110+
const malformedFixed = await lintFixture('malformed-metadata.md', {
111+
fix: true,
112+
});
113+
assert(
114+
malformedFixed.output.includes(
115+
"{expectedErrors: {'react-compiler': [4]}}"
116+
),
117+
'expected malformed metadata to be replaced with canonical form'
118+
);
119+
}
120+
121+
run().catch(error => {
122+
console.error(error);
123+
process.exitCode = 1;
124+
});

eslint-local-rules/index.js

Lines changed: 47 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,47 @@
1+
const lintMarkdownCodeBlocks = require('./rules/lint-markdown-code-blocks');
2+
3+
module.exports = {
4+
rules: {
5+
'lint-markdown-code-blocks': lintMarkdownCodeBlocks,
6+
},
7+
processors: {
8+
markdown: {
9+
preprocess(text) {
10+
// Hack: wrap the markdown file in a dummy JS comment so ESLint feeds it
11+
// through our custom rule without needing a real markdown parser here.
12+
return [
13+
`// Markdown content for processing\n/* ${text.replace(
14+
/\*\//g,
15+
'*\\/'
16+
)} */`,
17+
];
18+
},
19+
postprocess(messages) {
20+
const processedMessages = messages.flat().map((message) => {
21+
if (message.suggestions) {
22+
const processedSuggestions = message.suggestions.map(
23+
(suggestion) => {
24+
if (suggestion.fix) {
25+
// Unescape */ in fix text
26+
const fixedText = suggestion.fix.text.replace(
27+
/\*\\\//g,
28+
'*/'
29+
);
30+
return {
31+
...suggestion,
32+
fix: {...suggestion.fix, text: fixedText},
33+
};
34+
}
35+
return suggestion;
36+
}
37+
);
38+
return {...message, suggestions: processedSuggestions};
39+
}
40+
return message;
41+
});
42+
return processedMessages;
43+
},
44+
supportsAutofix: true,
45+
},
46+
},
47+
};

eslint-local-rules/package.json

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,9 @@
1+
{
2+
"name": "eslint-plugin-local-rules",
3+
"version": "0.0.0",
4+
"main": "index.js",
5+
"private": "true",
6+
"scripts": {
7+
"test": "node __tests__/lint-markdown-code-blocks.test.js"
8+
}
9+
}

0 commit comments

Comments
 (0)