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

Allow only formatting code snippets in markdown, not the markdown file itself #612

Closed
antfu opened this issue Dec 2, 2023 · 14 comments
Closed

Comments

@antfu
Copy link

antfu commented Dec 2, 2023

What version of eslint are you using?

8.55.0

What version of prettier are you using?

3.1.0

What version of eslint-plugin-prettier are you using?

5.0.1

Please paste any applicable config files that you're using (e.g. .prettierrc or .eslintrc files)

https://stackblitz.com/edit/node-mhr6sb?file=README.md,eslint.config.js

What source code are you linting?

See the Stackblitz link above

What did you expect to happen?

With fine-grained files option in the flat config, Prettier should be able to format virtual files created by eslint-plugin-markdown to format the code snippet in Markdown.

What actually happened?

There is a hard-coded logic to ignore any virtual file created in markdown as well as a few other extensions:

} else {
// Similar to https://github.com/prettier/stylelint-prettier/pull/22
// In all of the following cases ESLint extracts a part of a file to
// be formatted and there exists a prettier parser for the whole file.
// If you're interested in prettier you'll want a fully formatted file so
// you're about to run prettier over the whole file anyway.
// Therefore running prettier over just the style section is wasteful, so
// skip it.
const parserBlocklist = [
'babel',
'babylon',
'flow',
'typescript',
'vue',
'markdown',
'html',
'mdx',
'angular',
'svelte',
];
if (parserBlocklist.includes(/** @type {string} */ (inferredParser))) {
return;
}
}

I consider it a valid default as it might cause nested formatting when users enable eslint-plugin-prettier to all files.

But with the new Flat Config, you are able to only enable certain rules with certain options for specific types of files. In the case where we don't config Prettier to format the parent markdown, treating the virtual files as normal formattable code should be allowed.

Proposed solutions

I imagine we could introduce a flag to opt-out of this hard-coded logic, and give the control of ignoring a file to users.

I made a simple implantation, adding a new config called fullControl: a333607

Let me know if you have a better idea/naming. Thanks!

@JounQin
Copy link
Member

JounQin commented Dec 2, 2023

I'm not sure to understand, markdown files are formatted by prettier itself already?

You can use https://github.com/mdx-js/eslint-mdx/tree/master/packages/eslint-plugin-mdx to enable linting markdown files via eslint already.

@antfu
Copy link
Author

antfu commented Dec 2, 2023

With Flat Config, you can only enable Prettier to format on specific type,

export default [
{
  files: ['**/*.css'], // <-- this
  plugins: {
    prettier: pluginPrettier,
  },
  languageOptions: {
    parser: parserPlain,
  },
  rules: {
    'prettier/prettier': ['error', { parser: 'css' }],
  },
}
]

In my case I only want Prettier to take care all CSS in the pipeline but not Markdown itself.

@JounQin
Copy link
Member

JounQin commented Dec 2, 2023

In my case I only want Prettier to take care all CSS in the pipeline but not Markdown itself.

I don't think this is how prettier works.

@antfu
Copy link
Author

antfu commented Dec 2, 2023

Prettier is a formatter that can even format a string in the memory, right? I don't see any reason I can't use Prettier to only format a selection of files (or virtual files).

@JounQin
Copy link
Member

JounQin commented Dec 2, 2023

It can do it, but why it should do it?

cc @BPScott

@antfu
Copy link
Author

antfu commented Dec 2, 2023

I don't understand what we are even debating. Do you mean that the only way Prettier should work is to format all files?

@JounQin
Copy link
Member

JounQin commented Dec 2, 2023

Yes. It should format the whole file, not all files, sorry.

Prettier is an opinionated code formatter

I'd like to listen from @BPScott at the same time.

@JounQin JounQin changed the title Allow formatting code snippets in markdown Allow only formatting code snippets in markdown, not the markdown file itself Dec 2, 2023
@antfu
Copy link
Author

antfu commented Dec 2, 2023

Just for context, eslint-plugin-markdown will create a virtual file for each code snippet, and allow other plugins to treat them as a normal file.

For ./README.md

Markdown Test

```css
.code { color: red }
```

The virtual file will be ./README.md/0.css (which matches the **/*.css glob).

.code { color: red }

Which from the plugin point of view, it's a single file not a part of the file.

eslint-plugin-prettier's current behavior is a bit coupled with the filesystem and is not 100% agnostic (like you can turn off prettierrc, but .getFileInfo is mandatory with hard-coded logic).

From your words, I feel that currently this plugin is more like an "ESLint integration for Prettier, the opinionated formatting CLI Tools" over an "ESLint plugin using Prettier as the opinionated code formatter" -- Which is also fair, I'd respect that if you position it that way. "Opinionated" is an easy word to close the door I guess, where I see there might still be many interesting possibilities we could explore.

@JounQin
Copy link
Member

JounQin commented Dec 2, 2023

I was referencing eslint-plugin-mdx, it supports markdown at the same time, not eslint-plugin-markdown.


"Opinionated" is an easy word to close the door I guess, where I see there might still be many interesting possibilities we could explore.

That's why I said:

I'd like to listen from @BPScott at the same time.

@antfu
Copy link
Author

antfu commented Dec 2, 2023

eslint-plugin-mdx looks cool and I'd love to give it a try. Tho I feel it's not really related to this issue. What I want to do is fill the gap that ESLint lacks support for some file formats like CSS. I imagine with eslint-plugin-mdx I'll be able to format markdown but the CSS code snippet will still remain untouched right?

@JounQin
Copy link
Member

JounQin commented Dec 2, 2023

I'll be able to format markdown but the CSS code snippet will still remain untouched right?

Code snippets linting by eslint-plugin-mdx is powered by eslint-plugin-markdown.

What means you can already achieve what you want already.

@JounQin
Copy link
Member

JounQin commented Dec 2, 2023

What I want to do is fill the gap that ESLint lacks support for some file formats like CSS.

Maybe you want https://github.com/ota-meshi/eslint-plugin-css

In the feature, ESLint will not only focus on JavaScript ecosystem, it would be very easy to create a language plugin for ESLint, see aslo stylelint/stylelint#6593

@BPScott
Copy link
Member

BPScott commented Dec 2, 2023

I'm strongly opposed to adding new options that allow users to do customizations that diverge from the standard "configure prettier using the .prettierrc file" behaviour.

The aim is that you should get identical output if you format a file through prettier's CLI or through eslint with eslint-plugin-prettier. Adding options to eslint-plugin-prettier is a vector for allowing divergences to happen, which moves us away from that goal, and increases our support burden as people can misuse these options and confuse themselves.

Ideally eslint-plugin-prettier should have no options at all. The only reason why I've not removed the current ones is because I inherited this plugin with those options in place and I don't want the maintenance burden of removing them.


In my case I only want Prettier to take care all CSS in the pipeline but not Markdown itself.

If I'm understanding this right, you've got a markdown file with a css fenced codeblock inside it, and you want to use ESLint to format the css fenced codeblock content using prettier (as prettier can handle non-js languages). At no point in this process are you trying to format javascript.

ESLint concerns itself with formatting JS content, and with plugins like eslint-plugin-markdown can handle JS embedded within other file types. It is not currently in the business of formatting non-JS content. As @JounQin has noted ESLint is looking at becoming more amenable to becoming a linter of many languages but that work is currently theoretical.

This feels tremendously complicated and like you're trying to push square pegs into round holes. I consider this to use-case to be very convoluted and not something I'd want to support.

Prettier does not act in this way - it formats the whole file at a time - if you've got a markdown file with a js/css codeblock in it, then prettier will format both the markdown and the codeblock. There is no way to make it just format just the codeblocks within the markdown file while leaving the markdown content untouched. Because prettier does not act in this way I do not want eslint-plugin-prettier to act in this way either.

If you want to format your markdown files I'd recommend running the whole markdown file through the prettier CLI (prettier './**/*.md') and that will format both the markdown and codeblock content within it.

What I want to do is fill the gap that ESLint lacks support for some file formats like CSS.

This is a noble goal, and the ESLint team are working towards this in eslint/rfcs#99. However I do not feel that this plugin is the place to experiment with such work.

I am grateful that you're proactive in coming up with solutions, however I would not merge #613, as I feel that what it is trying to do is directly opposed to the aims of this plugin (i.e. match eslint and prettier's behaviours). It tries to allow formatting of non-js content, which is not what ESLint is currently designed for, and it results in formating only a subset of a markdown file, which prettier does not support.


Random aside:

But with the new Flat Config, you are able to only enable certain rules with certain options for specific types of files.

FYI, this isn't a new feature, overrides have been a standard part of eslint's existing config for a long time.

@antfu
Copy link
Author

antfu commented Dec 3, 2023

I am quite new to Prettier, I didn't know the "Opinionated" in Prettier expanded into so many aspects. Thanks for your time and detailed explanation, I think they make sense and you made them very clear. I guess Prettier might not be the "formatter" I am looking for, I'll use my fork for now and consider other alternatives. Thank you :)

@antfu antfu closed this as not planned Won't fix, can't repro, duplicate, stale Dec 3, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
3 participants