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: use eslint-plugin-mdx for md/mdx files #353

Draft
wants to merge 9 commits into
base: main
Choose a base branch
from

Conversation

JounQin
Copy link

@JounQin JounQin commented Dec 8, 2023

Description

Enable eslint-plugin-mdx to replace simple eslint-plugin-markdown

Linked Issues

N/A

Additional context

src/configs/markdown.ts Outdated Show resolved Hide resolved
@JounQin JounQin force-pushed the feat/eslint-plugin-mdx branch from 3b1f334 to 8a3bd14 Compare December 8, 2023 16:13
@JounQin JounQin force-pushed the feat/eslint-plugin-mdx branch from 8a3bd14 to dd8d570 Compare December 10, 2023 03:27
@JounQin JounQin force-pushed the feat/eslint-plugin-mdx branch from dd8d570 to 336e453 Compare December 10, 2023 13:25
src/configs/markdown.ts Outdated Show resolved Hide resolved
fixtures/input/mdx.mdx Outdated Show resolved Hide resolved
@@ -1,32 +1,33 @@
# @antfu/eslint-config
@antfu/eslint-config
Copy link
Author

Choose a reason for hiding this comment

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

I think they're removed unexpectedly.

Comment on lines +17 to +38
const patchedParser: Linter.ParserModule = {
...mdxParser,
parse(text, options) {
// @ts-expect-error cast
const result = mdxParser.parseForESLint!(text, options)
const body = result.ast.body

function predicate(token: { start: number, end: number }) {
for (const node of body) {
if (node.start <= token.start! && node.end >= token.end!)
return true
}
return false
}

// `eslint-mdx` produces extra tokens that are not presented in the AST, causing rules like `indent` to fail
result.ast.tokens = result.ast.tokens.filter(predicate)
result.ast.comments = result.ast.comments.filter(predicate)

return result
},
}
Copy link
Owner

@antfu antfu Dec 11, 2023

Choose a reason for hiding this comment

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

According to eslint-stylistic/eslint-stylistic#215

This patch is the closest thing I could fix, but the formatting for mdx is still not ideal.

I think I have spent too much time on working around it. I rather not have it or something in a broken state. I am giving it up at this moment, but I am open to revisiting it later if the compatibility gets better in the future.

Thanks for your effort!

Copy link
Author

Choose a reason for hiding this comment

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

I don't understand, why not just disable indent temporarily for .md/.mdx files.

Copy link
Owner

Choose a reason for hiding this comment

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

b6114b4

In this commit, even the tests are passing, you see that mdx is basically not formatted at all.

Copy link
Author

@JounQin JounQin Dec 11, 2023

Choose a reason for hiding this comment

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

Because prettier does not support mdx v2/v3 well?

prettier/prettier#12209

Copy link
Author

@JounQin JounQin Dec 11, 2023

Choose a reason for hiding this comment

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

image

I tried to run that file locally, it seems there are errors reporting but not auto fixed by eslint-plugin-prettier. I'm not sure to understand why for now. A deeper debugging is needed.

Copy link
Author

Choose a reason for hiding this comment

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

I just raised mdx-js/eslint-mdx#491 for tracking.

Copy link
Owner

Choose a reason for hiding this comment

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

Ok, looking forward to see it get fixed! 👍

@antfu
Copy link
Owner

antfu commented Dec 11, 2023

I am okay with keeping this PR open for future reference, or feel free to close it.

@antfu antfu marked this pull request as draft December 11, 2023 12:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants