Skip to content
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

feat(mdx-loader): Remark plugin to report unused MDX / Markdown directives #9394

Merged
merged 27 commits into from
Oct 24, 2023
Merged
Show file tree
Hide file tree
Changes from 5 commits
Commits
Show all changes
27 commits
Select commit Hold shift + click to select a range
1c2b05b
wip: unused directives plugin
OzakIOne Oct 11, 2023
2f6cddb
wip: parse tree from root
OzakIOne Oct 12, 2023
0478dd8
wip: unit test mock
OzakIOne Oct 12, 2023
f934f84
wip: better error log
OzakIOne Oct 12, 2023
ea124d3
Merge branch 'main' into ozaki/mdx-unused-directives
slorber Oct 12, 2023
cefc3e4
prevent duplicate logging of unused directives in prod build
slorber Oct 12, 2023
f87a0b4
fix TS vfile datamap issue
slorber Oct 12, 2023
3ea1c74
wip: add text & leaf tests
OzakIOne Oct 12, 2023
6a0337a
fix: include lines in path
OzakIOne Oct 13, 2023
e4e7255
remark unused directive tests should handle compilerName option
slorber Oct 13, 2023
1b59e60
wip: improve log messages
OzakIOne Oct 13, 2023
b21537e
Merge branch 'main' into ozaki/mdx-unused-directives
slorber Oct 23, 2023
59cb00f
update unused directives snapshots
slorber Oct 23, 2023
61b41ae
refactor unused directives
slorber Oct 23, 2023
cb54192
refactor unused directives
slorber Oct 23, 2023
10d4057
refactor unused directives
slorber Oct 23, 2023
b777c29
remove annoying eslint rule
slorber Oct 23, 2023
a142e60
change format of warning
slorber Oct 23, 2023
23f6cb9
refactor
slorber Oct 23, 2023
55a0abe
Implement unused simple text directive re-serialization
slorber Oct 24, 2023
5fb34ae
use transformNode everywhere
slorber Oct 24, 2023
764e9ab
eslint
slorber Oct 24, 2023
bfa8a90
eslint
slorber Oct 24, 2023
99f83b2
fix bug + add client/server output mismatch tests
slorber Oct 24, 2023
a85f049
increase playwright timeout
slorber Oct 24, 2023
d7cf972
Merge branch 'main' into ozaki/mdx-unused-directives
slorber Oct 24, 2023
65406ac
ignore playwright path
slorber Oct 24, 2023
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
2 changes: 2 additions & 0 deletions packages/docusaurus-mdx-loader/src/processor.ts
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ import details from './remark/details';
import head from './remark/head';
import mermaid from './remark/mermaid';
import transformAdmonitions from './remark/admonitions';
import unusedDirectivesWarning from './remark/unusedDirectives';
import codeCompatPlugin from './remark/mdx1Compat/codeCompatPlugin';
import {getFormat} from './format';
import type {MDXFrontMatter} from './frontMatter';
Expand Down Expand Up @@ -123,6 +124,7 @@ async function createProcessorFactory() {
gfm,
options.markdownConfig.mdx1Compat.comments ? comment : null,
...(options.remarkPlugins ?? []),
unusedDirectivesWarning,
].filter((plugin): plugin is MDXPlugin => Boolean(plugin));

// codeCompatPlugin needs to be applied last after user-provided plugins
Expand Down

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
// Jest Snapshot v1, https://goo.gl/fbAQLP

exports[`directives remark plugin default behavior for custom keyword 1`] = `
"<admonition type="danger"><p>Take care of snowstorms...</p></admonition>
<div><p>unused directive content</p></div>
<p>:::NotAContainerDirective with a phrase after</p>
<p>:::</p>
<p>Phrase before :::NotAContainerDirective</p>
<p>:::</p>"
`;

exports[`directives remark plugin default behavior for custom keyword 2`] = `
[
[
"[WARNING] We found a potential error in your documentation unusedDirective "packages/docusaurus-mdx-loader/src/remark/unusedDirectives/__tests__/__fixtures__/containerDirectives.md":7:1",
],
]
`;
Original file line number Diff line number Diff line change
@@ -0,0 +1,45 @@
/**
* Copyright (c) Facebook, Inc. and its affiliates.
*
* This source code is licensed under the MIT license found in the
* LICENSE file in the root directory of this source tree.
*/

import path from 'path';
import remark2rehype from 'remark-rehype';
import stringify from 'rehype-stringify';
import vfile from 'to-vfile';
import plugin from '../index';
import admonition from '../../admonitions';

const processFixture = async (name: string) => {
const {remark} = await import('remark');
const {default: directives} = await import('remark-directive');

const filePath = path.join(__dirname, '__fixtures__', `${name}.md`);
const file = await vfile.read(filePath);

const result = await remark()
.use(directives)
.use(admonition)
.use(plugin)
.use(remark2rehype)
.use(stringify)
.process(file);

return result.value;
};

describe('directives remark plugin', () => {
it('default behavior for custom keyword', async () => {
const consoleMock = jest
.spyOn(console, 'warn')
.mockImplementation(() => {});

const result = await processFixture('containerDirectives');

expect(result).toMatchSnapshot();

expect(consoleMock.mock.calls).toMatchSnapshot();
});
});
Original file line number Diff line number Diff line change
@@ -0,0 +1,72 @@
/**
* Copyright (c) Facebook, Inc. and its affiliates.
*
* This source code is licensed under the MIT license found in the
* LICENSE file in the root directory of this source tree.
*/
import path from 'path';
import process from 'process';
import visit from 'unist-util-visit';
import logger from '@docusaurus/logger';
import {posixPath} from '@docusaurus/utils';
// @ts-expect-error: TODO see https://github.com/microsoft/TypeScript/issues/49721
import type {Transformer, Processor, Parent} from 'unified';
import type {
Directive,
// @ts-expect-error: TODO see https://github.com/microsoft/TypeScript/issues/49721
} from 'mdast-util-directive';

// TODO as of April 2023, no way to import/re-export this ESM type easily :/
// This might change soon, likely after TS 5.2
// See https://github.com/microsoft/TypeScript/issues/49721#issuecomment-1517839391
// import type {Plugin} from 'unified';
type Plugin = any; // TODO fix this asap

const directiveTypes = ['containerDirective', 'leafDirective', 'textDirective'];

const plugin: Plugin = function plugin(this: Processor): Transformer {
return (tree, file) => {
const unusedDirectives: Array<{
name: string;
type: string;
position:
| {
line: number;
column: number;
}
| undefined;
}> = [];

visit<Parent>(tree, directiveTypes, (node: Directive) => {
if (!node.data) {
unusedDirectives.push({
name: node.name,
type: node.type,
position: node.position
? {
line: node.position.start.line,
column: node.position.start.column,
}
: undefined,
});
}
});

if (unusedDirectives.length > 0) {
const warningMessage = unusedDirectives
.map((unusedDirective) => {
const positionMessage = unusedDirective.position
? logger.interpolate`number=${unusedDirective.position.line}:number=${unusedDirective.position.column}`
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Note, I'm not sure if IDEs can provide navigation if the line numbers are surrounded by ANSI sequences.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For now we agreed navigation was not a goal because we need to use relative paths anyway otherwise CI tests would fail due to different absolute paths in snapshots.

And navigation apparently doesn't work in VSCode with relative paths.

Copy link
Collaborator Author

@OzakIOne OzakIOne Oct 12, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've tested on VSCode and in fact there is no problem opening the file from terminal even if the path is relative or surrounded by ANSI sequences (colors in the terminal am I right?)

VSCode seems to support multiple path syntax, full path, relative with or without ./ (e.g.: ./website/README.md or website/README.md)

I've tested on WebStorm but it doesn't seem to have this feature (ctrl+click) to open file in editor (not a WebStorm user so I didn't dig too much)

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, VS Code supports opening files with paths relative to the current project. I think it even supports arbitrary paths, as long as the name is unambiguous (otherwise it shows a dropdown chooser). For example foo.tsx can open to src/components/foo.tsx.

: '';

const customPath = posixPath(path.relative(process.cwd(), file.path));

return logger.interpolate`We found a potential error in your documentation name=${unusedDirective.name} path=${customPath}:${positionMessage}`;
})
.join('\n');
logger.warn(warningMessage);
}
};
};

export default plugin;