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: introduce option to force format extracted fragements #561

Closed

Conversation

paescuj
Copy link

@paescuj paescuj commented Jun 20, 2023

The default behavior skipping the formatting of extracted fragments (e.g. code blocks in Markdown files, parsed via eslint-plugin-markdown) is usually fine.

} 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;
}
}

However, there are cases where you want to have such fragments formatted anyways.
This pull request introduces the new option forceFormatExtracted (happy to change the naming) which turns this particular check off.

@changeset-bot
Copy link

changeset-bot bot commented Jun 20, 2023

🦋 Changeset detected

Latest commit: 4c7077b

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 1 package
Name Type
eslint-plugin-prettier Minor

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

@github-actions
Copy link
Contributor

github-actions bot commented Jun 20, 2023

📊 Package size report   No changes

File Before After
Total (Includes all files) 24.5 kB 24.5 kB
Tarball size 8.2 kB 8.2 kB
Unchanged files
File Size
eslint-plugin-prettier.d.ts 117 B
eslint-plugin-prettier.js 7.4 kB
LICENSE.md 1.1 kB
package.json 2.4 kB
README.md 7.9 kB
worker.js 5.7 kB

🤖 This report was automatically generated by pkg-size-action

@@ -153,6 +153,22 @@ If you’re fixing large of amounts of previously unformatted code, consider tem
}
```

- `forceFormatExtracted`: Enables formatting of extracted fragements, (default: `false`).
ESLint supports processors that let you extract and lint JS fragments within a non-JS language, for example using `eslint-plugin-markdown` to extract and lint a code block inside a Markdown file. Usually, you don't want to have the fragement itself formatted by Prettier as the whole file
might already be formatted anyway. This is why `eslint-plugin-prettier` ignores such fragements by default. However, for cases you want to have such extracted fragements formatted by Prettier you can use this option to force it.
Copy link
Member

Choose a reason for hiding this comment

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

However, for cases you want to have such extracted fragements formatted by Prettier you can use this option to force it.

So, the case is, you want to format js in markdown via prettier, but not the markdown file itself? cc @BPScott

Copy link
Author

Choose a reason for hiding this comment

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

Yeah, sort of. Actually I'd like to be able to apply a different set of Prettier rules to code blocks in Markdown files resp. be able to individually handle those blocks (for example disabling Prettier for JS code blocks, while formatting other code blocks like JSON). This can be achieved via ESLint rules on Markdown files processed with eslint-plugin-markdown. This set-up is working fine, but only if I'm able to disable the default behavior of eslint-plugin-prettier with this newly introduced option to not skip those code blocks.

Copy link
Member

Choose a reason for hiding this comment

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

Commented down below. I think there's ways of doing this without introducing a new option.

@BPScott
Copy link
Member

BPScott commented Jun 22, 2023

However, there are cases where you want to have such fragments formatted anyways.

I was about to ask for an exact example of this case. Fortunately you've provided one in a comment already:

Yeah, sort of. Actually I'd like to be able to apply a different set of Prettier rules to code blocks in Markdown files resp. be able to individually handle those blocks (for example disabling Prettier for JS code blocks, while formatting other code blocks like JSON). This can be achieved via ESLint rules on Markdown files processed with eslint-plugin-markdown. This set-up is working fine, but only if I'm able to disable the default behavior of eslint-plugin-prettier with this newly introduced option to not skip those code blocks.


This change feels like a really really big footgun. A big aim of eslint-plugin-prettier is that you should get non-conflicting results regardless of if you format a file through prettier or eslint with eslint-plugin-prettier enabled. In order to do this, we must leverage existing eslint/prettier configuration - usually through overrides - as much as possible instead of adding our own.

When we diverge we introduce the support burden of having to answer a bunch of support questions asking why formatting a file with prettier does one thing but eslint does another. This gets extra fun inside editors if you've got eslint and prettier plugins installed at the same time because now we have to explain potential for editor misconfiguration too.

eslint-plugin-prettier should strive to be glue between two other things that are configurable. Every option that we add (including the two existing usePrettierrc, and fileInfoOptions options) is a step away from that aim of prettier and eslint producing non-conflicting results, and thus I default to being against config option additions without a very very very good reason. In an ideal world I'd like to nuke those two existing options too, but I don't want to have to deal with the fallout of that either - we're in a nice place where the codebase is feature-complete and stable and doesn't need to be tinkered with for he most part.

If you're trying to do something particularly funky that truly can not be resolved by leveraging existing config, honestly I'd rather you fork eslint-plugin-prettier for your specific use-case instead of having to deal with the maintenance burden of supporting that weird use-case as I think additional options are ripe for misunderstanding.


I suspect we might be able to get your desired state another way without adding a config option.

eslint-plugin-markdown uses processors to generate custom virtual filenames for each of the fenced code blocks it finds in a markdown file, per its readme it sounds like they're in the format the-source-file.md/0.js - with 0 increasing for each fenced codeblock it finds in that source file.

You should be able to add the following override to your eslint config to disable prettier in those js blocks within markdown files:

This would mean that those blocks would be not prettiered when you run the file through eslint, but if you ever ran the file through prettier then you get a different result.

 overrides: [
  {
    // 1. Target ```js code blocks in .md files.
    files: ["**/*.md/*.js"],
    rules: {
      "prettier/prettier": "off",
    }
  }
]

Alternatively you can prefix the fenced codeblocks that you don't want to format with prettier with a ignore comment to get prettier to skip it:

# here is my markdown file
   
Here is some text

<!-- prettier-ignore -->
```js
ugly   ( codeThatIsIgnoredByPrettier ) ;
```

Here is some more text

This is almost certainly the most idiomatic approach, which would result in the same behaviour when you run the file through both prettier and eslint.
You could probably write some kind of markdown linter that says "if you find a fenced codeblock with the js language, then it must be immediately preceded by a <!-- prettier-ignore --> comment.


Please try one of the two above approaches. The latter certainly will work, and I believe that you should go with that approach instead of instead of us adding a new option to eslint-plugin-prettier.

@JounQin
Copy link
Member

JounQin commented Oct 14, 2023

close as above comment

@JounQin JounQin closed this Oct 14, 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
Development

Successfully merging this pull request may close these issues.

None yet

3 participants