-
Notifications
You must be signed in to change notification settings - Fork 64
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
Change Request: Support MDX linting #316
Comments
Does it aim to replace https://github.com/mdx-js/eslint-mdx? |
I haven’t really looked into You can’t treat each export function someExport() {
// Undefined variable.
props
}
{
// This is fine.
props.someProp
}
export function someOtherExport() {
// This is in scope.
someOther
// This is also in scope. The compiler adds it.
MDXContent
} You have to compile the MDX to some point where the ESM and expressions make sense. In the end it compiles to something where only one If I would re-implement ESLint MDX support from scratch, I would start investigating if ESLint can work with the AST MDX exposes after compilation. This is just an ESTree, where authored nodes have positional info, but generated nodes do not. If this works, it would also have support for plugins. For example, ---
meta: data
---
{
// This is fine.
{frontmatter.meta.data}
} Another interesting case is ```py filename={props.base + '/file.py'}
print('This is a code block')
``` |
@remcohaszing thanks, yes, this is what I was implying in my OP. It's what prelint plugins are designed to do: allow you to say "for this file, also consider this virtual file I've created an overlay the results." |
Environment
ESLint version: HEAD
@eslint/markdown version: HEAD
Node version: n/a
npm version: n/a
Operating System: n/a
What problem do you want to solve?
Currently, the Markdown plugin does not formally parse MDX (it just becomes text but doesn't throw an error). Ideally, we could provide smart parsing for MDX so it can be linted just like anything else.
I investigated using https://github.com/syntax-tree/mdast-util-mdx, but it outputs an AST that is difficult to work with. Any JavaScript code produces a
Program
node (it uses Acorn under the hood), and there can be multipleProgram
nodes in a single document. This makes the AST difficult to navigate and make sense of.Further, because actual JavaScript code can be embedded inside of MDX, we'd need to recreate all of the JavaScript-specific rules just to work in MDX. That seems like wasted effort. I don't think the out-of-the-box approach to MDX from mdast works for our case.
What do you think is the correct solution?
I think ultimately what we want is a way to run existing JavaScript rules on an MDX file in addition to MDX-specific rules. The only way I can think to do that using something like a prelint plugin to extract the JavaScript into a separate virtual file to be linted.
In order to do that, I think we'll need to create a custom mdast plugin that extracts the JavaScript code without creating an AST, basically treating it like a code block.
Participation
Additional comments
I'm just documenting this for now as I have other, higher-priority work to complete. I'd like to consider this on the roadmap for this plugin.
The text was updated successfully, but these errors were encountered: