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

deprecate rule prettier since eslint-plugin-prettier itself supports virtual filenames correctly now #395

Closed
JounQin opened this issue Apr 22, 2021 · 7 comments · Fixed by #397
Labels
kind/enhancement New feature or request stage/5-alpha-release-testing The pull request is merged, an alpha release is available, to be tested

Comments

@JounQin
Copy link
Contributor

JounQin commented Apr 22, 2021

See

https://github.com/prettier/eslint-plugin-prettier/pull/401/files#diff-e9a90fe7b2d3420b8e5f64e24d5f56ffe6383e47b07461172cc29ed21b768905R197-R199

and

https://github.com/prettier/eslint-plugin-prettier/blob/master/eslint-plugin-prettier.js#L192-L209


Besides, actually it is supported previously too. All the user need to do is like the following:

module.exports = {
  // ...
  overrides: [
    {
      files: '*.graphql', // it includes virtual filenames like `*.js/0_document.graphql` already
      rules: {
        'prettier/prettier': [
          2,
          {
            parser: 'graphql',
          },
        ],
      },
    },
  ],
}
After `eslint-plugin-prettier@3.4.0`, the above configuration is not needed any more too!

Maybe in eslint-plugin-pretter@3.5.0 after prettier/eslint-plugin-prettier#413 and prettier/eslint-plugin-prettier#415 been merged and released.

@JounQin JounQin changed the title deprecate rule prettier since it support virtual filenames correctly now deprecate rule prettier since eslint-plugin-prettier itself supports virtual filenames correctly now Apr 22, 2021
@dotansimha dotansimha added the kind/enhancement New feature or request label Apr 22, 2021
@Urigo
Copy link
Collaborator

Urigo commented Apr 22, 2021

@JounQin that's awesome, thank you for pointing that out!

Would you be willing to send a PR for that?
Maybe also worth updating the docs? because it might be breaking for someone who uses the existing rule?
(and thank you again for your previous PR!)

@JounQin
Copy link
Contributor Author

JounQin commented Apr 22, 2021

I'll raise a PR for it.

But I just found the eslint-plugin-prettier decided to be compatible with eslint-plugin-graphql which is not compatible with this plugin before. So the option for prettier and non-virtual *.graphql filenames is still required, I think we can add some configs which includes this option automatically.

And it seems eslint-plugin-grahql is out of maintainace, so I'm going to ask eslint-plugin-prettiet to remove the related codes.

@JounQin
Copy link
Contributor Author

JounQin commented Apr 22, 2021

I have an idea to support the both two ESLint grahql plugins for eslint-plugin-prettier just now, I'll PR there first.

@Urigo
Copy link
Collaborator

Urigo commented Apr 22, 2021

@JounQin yes I agree they should remove eslint-plugin-grahql, it's unmaintained for many years now

@Urigo
Copy link
Collaborator

Urigo commented Apr 22, 2021

reference for why we chose not to go with eslint-plugin-grahql: https://the-guild.dev/blog/introducing-graphql-eslint

@JounQin
Copy link
Contributor Author

JounQin commented Apr 22, 2021

I've raised prettier/eslint-plugin-prettier#413 to make sure eslint-plugin-prettier will be compatible with this plugin out of the box.

I'll raise another PR to remove the prettier rule in this repo in the next step.

@Urigo Urigo added the stage/4-pull-request A pull request has been opened that aims to solve the issue label Apr 26, 2021
@dotansimha dotansimha reopened this May 18, 2021
@dotansimha dotansimha added stage/5-alpha-release-testing The pull request is merged, an alpha release is available, to be tested and removed stage/4-pull-request A pull request has been opened that aims to solve the issue labels May 18, 2021
@dotansimha
Copy link
Collaborator

Available in v1.0.0! 🎉

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind/enhancement New feature or request stage/5-alpha-release-testing The pull request is merged, an alpha release is available, to be tested
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants