-
-
Notifications
You must be signed in to change notification settings - Fork 658
Add lint rule to validate JSDoc codeblocks using TS compiler #1265
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Add lint rule to validate JSDoc codeblocks using TS compiler #1265
Conversation
This comment was marked as outdated.
This comment was marked as outdated.
c641bf8 to
89b14c8
Compare
This comment was marked as outdated.
This comment was marked as outdated.
|
@sindresorhus This PR is still a WIP, because there are still a lot of code blocks that have errors in them, but would appreciate if you could give an early feedback on this. Couple of questions:
|
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
Should be fairly fast now, the env creation happens only once now! Now, the only task left is to fix the remaining 214 errors 😪 |
186f9e2 to
d974efa
Compare
No, it can't run just once.
The env creation now happens per file instead of the initial per file per code block setup. So, it now takes around a min. Refer #d974efa. |
|
This is a very useful rule 🙌 |
I think we should just ignore the internal types. Many will eventually be exposed and then they can be fixed then.
👍
Doesn't ESLint cache things? Maybe something is not working with the cache. Alternatively, we could only run this rule in CI. |
7a0bde1 to
d000858
Compare
som-sm
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I've added comments wherever I felt it needed some explanation. Hope it makes the review process a tad bit easier.
| // TypeScript now knows that `someArg` is `SomeType` automatically. | ||
| // It also knows that this function must return `Promise<Foo>`. | ||
| // If you have `@typescript-eslint/promise-function-async` linter rule enabled, it will even report that "Functions that return promises must be async.". |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Changed this example because these three points have nothing to do with the type.
| IsNever<Exclude<A, B>> extends true ? true : false, | ||
| IsNever<Exclude<B, A>> extends true ? true : false |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Removed this example because the primary focus here wasn't on IsNever. Added a better example (see below).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Replaced this example because it had multiple issues, but the updated example conveys a similar idea as the original example.
| type Union = typeof union; | ||
| //=> {a1(): void; b1(): void} | {a2(argA: string): void; b2(argB: string): void} | ||
| type Intersection = UnionToIntersection<Union>; | ||
| //=> {a1(): void; b1(): void; a2(argA: string): void; b2(argB: string): void} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This example is almost similar to the previous one for this type, so simply removed it instead of replacing it with an alternative.
| onlyBar('bar'); | ||
| //=> 2 | ||
| type C = ValueOf<{id: number; name: string; active: boolean}, 'id' | 'name'>; | ||
| //=> number | string |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The existing example had issues, and a simpler example seemed better in this case.
| // type A = string; | ||
| // ``` | ||
| // @category Test | ||
| // */ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There's a zero-width space character b/w * and / to prevent */ from being interpreted as the end of the JSDoc comment. Because of which there's a /* eslint-disable no-irregular-whitespace */ comment at the start of this file.
Umm...not sure if there's a better way of achieving this, I tried a couple different solutions but nothing seemed to work.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Because of which there's a
/* eslint-disable no-irregular-whitespace */comment at the start of this file.
Now, the rule is disabled only for comments.
Lines 88 to 98 in 657dd4d
| { | |
| files: 'lint-rules/test-utils.js', | |
| rules: { | |
| 'no-irregular-whitespace': [ | |
| 'error', | |
| { | |
| 'skipComments': true, | |
| }, | |
| ], | |
| }, | |
| }, |
| exportTypeAndOption(''), | ||
| exportType('type Some = number;'), | ||
| exportTypeAndOption('// Not block comment'), | ||
| exportTypeAndOption('/* Block comment, but not JSDoc */'), | ||
|
|
||
| // No codeblock in JSDoc | ||
| exportType(jsdoc('No codeblock here')), | ||
|
|
||
| // With text before and after | ||
| exportTypeAndOption(jsdoc('Some description.', fence(code1), '@category Test')), | ||
|
|
||
| // With line breaks before and after | ||
| exportTypeAndOption( | ||
| jsdoc('Some description.\n', 'Note: Some note.\n', fence(code1, 'ts'), '\n@category Test'), | ||
| ), | ||
|
|
||
| // With `@example` tag | ||
| exportTypeAndOption(jsdoc('@example', fence(code1))), | ||
|
|
||
| // With language specifiers | ||
| exportTypeAndOption(jsdoc(fence(code1, 'ts'))), | ||
| exportTypeAndOption(jsdoc(fence(code1, 'typescript'))), | ||
|
|
||
| // Multiple code blocks | ||
| exportTypeAndOption( | ||
| jsdoc('@example', fence(code1, 'ts'), '\nSome text in between.\n', '@example', fence(code2)), | ||
| ), | ||
|
|
||
| // Multiple exports and multiple properties | ||
| exportTypeAndOption(jsdoc(fence(code1)), jsdoc(fence(code2))), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
These utils help make the test cases terse, otherwise this already lengthy file would have been even lengthier.
For example, the following
exportTypeAndOption(jsdoc(fence(code1)), jsdoc(fence(code2)))evaluates to:
/**
```
type A = string;
```
*/
export type T0 = string;
/**
```
type B = number;
```
*/
export type T1 = string;
export type TOptions = {
/**
```
type A = string;
```
*/
p0: string;
/**
```
type B = number;
```
*/
p1: string;
};| */ | ||
| p0: string; | ||
| }; | ||
| `, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
For invalid tests, the code is not generated via some utility, as it is for valid cases, because here we want to validate the exact line and column numbers where the errors appear, and if the code were obfuscated behind some utility, then those numbers would feel magical/arbitrary, reducing the overall readability and maintainability of the test cases.
| errorAt({line: 4, textBeforeStart: 'type A = ', target: 'Subtract'}), | ||
| errorAt({line: 14, textBeforeStart: '\ttype A = ', target: 'Subtract'}), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
For errors, we use a utility because manually typing the column and endColumn values is error-prone and also less readable.
For example, in the following test
{
code: dedenter`
/**
@example
\`\`\`ts
type NoImport = Add<1, 2>;
\`\`\`
*/
export type Add = number;
`,
errors: [
{messageId: 'invalidCodeblock', line: 4, column: 17, endLine: 4, endColumn: 20},
],
},the numbers 17 and 20 feel magical/arbitrary and it's very easy to mess up those numbers and end up with false-positive tests.
It's much more readable if the column and endColumn values are generated like this:
errors: [
{
messageId: 'invalidCodeblock',
line: 4,
column: 'type NoImport = '.length + 1,
endLine: 4,
endColumn: 'type NoImport = '.length + 1 + 'Add'.length,
},
],But, this is still repetitive and has scope for simplification.
And, that's what the errorAt utility simplifies:
errors: [
errorAt({
line: 4,
textBeforeStart: 'type NoImport = ',
target: 'Add',
}),
],Note: The line and endLine values are manually entered because those are pretty straightforward. And errorAt uses the line value if an endLine is not explicitly passed, which also reduces the repetition a bit.
| ``` | ||
| import type {IsUnknown} from 'type-fest'; | ||
| // https://github.com/pajecawav/tiny-global-store/blob/master/src/index.ts |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The existing example had several issues, and it wasn’t really instantiating IsUnknown with unknown. Replaced it with a simpler example.
|
@sindresorhus This PR is finally ready for review. It took some effort to get it to the finish line, but I’m happy with how it turned out. I’ve also updated the PR description to include a detailed explanation of how this lint rule works. The only changes within the
|
NOTE: Using the
|
657dd4d to
b3b5b9c
Compare
|
This is super nice! 🎉 And seeing all the changes to code examples, it was really needed. It's hard to manually ensure code examples are correct. |



This PR introduces a lint rule that extracts example code blocks from JSDoc comments and validates them using
@typescript/vfs. It then reports any errors at their corresponding locations.The errors and their locations are identical to normal TS errors, so the experience feels just like editing a regular TS file.
Screen.Recording.2025-10-10.at.6.10.33.PM.mov
Working
Linting a single file involves the following steps:
Setting up the environment
A virtual TypeScript environment with a single virtual file is created on top of the existing file system. And the virtual environment is created such that it has access to the files on disk.
Location of virtual files
While setting up the virtual environment, we specify the root directory. The virtual environment then creates a virtual directory called
vfswithin the root directory, and all virtual files are created relative to thisvfsdirectory.For example, the following snippet:
creates the following directory structure:
So, if we want to
importtheOrtype from ourexample-codeblock.tsvirtual file, we'd have to use'../source/or.d.ts'as the import path, like:The location of the virtual file might seem problematic, but in practice it’s not, because our JSDoc example codeblocks never use relative imports. Instead, they always import from
'type-fest':Note: Importing directly from
'type-fest'works fine because Node.js allows self referencing a package using its name.Running the lint rule on each exported type node
The lint rule surfaced around ~250 errors (on publicly accessible docs), so this PR also fixes/improves multiple JSDoc code examples. Some common errors that it catches:
Missing

importstatementsFloating examples

References to hypothetical functions/variables

Duplicate identifiers

Syntax errors

Typos

