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

Question&Bug: Does not work well with preset styles #182

Closed
onichandame opened this issue Mar 25, 2020 · 8 comments
Closed

Question&Bug: Does not work well with preset styles #182

onichandame opened this issue Mar 25, 2020 · 8 comments
Labels
👀 no/external This makes more sense somewhere else

Comments

@onichandame
Copy link

onichandame commented Mar 25, 2020

Not Working with Preset Styles

when initiates eslint with one of the preset styles: google, standard or airbnb, eslint-mdx seems to conflict with the preset style.

Your environment

  • OS: CentOS
  • Env: node 10.19.0, yarn 1.22.4 eslint 6.8.0

Steps to reproduce:

  1. create a react app with create-react-app
yarn create react-app app
  1. init eslint in the app folder
cd app
npx eslint --init
  1. add mdx plugin
yarn add -D eslint-plugin-mdx

choose one of Standard, Airbnb, Google style during initiation process.
4. add mdx to config

{
  "extends": [
    "plugin:mdx/recommended"
  ]
}

Now create a valid mdx file with import and jsx syntax and run eslint on the file.

Some weird errors are reported. It seems like something other than eslint-mdx is linting the mdx file.

Expected behaviour

no error is reported as the mdx is valid.

Actual behaviour

some weird errors are reported. The error reported is different for different preset style. Just to give an example, when the Airbnb style is chosen, 5:1 error JSX not allowed in files with extension '.mdx' is reported.

Note

When the style part in the config file is removed,

{
  "extends": [
    "airbnb"
  ]
}

The problem disappeared. Therefore I guess that it's the conflict between the preset style and eslint-mdx that causes the problem.

This must be a very common scenario as all the steps follow the recommendation of eslint and eslint-mdx. But I cannot find a similar question on Google or StackOverflow. This might be helpful to others encountering the very problem in the future.

@onichandame onichandame added 🐛 type/bug This is a problem 🙉 open/needs-info This needs some more info labels Mar 25, 2020
@JounQin
Copy link
Member

JounQin commented Mar 25, 2020

Please provide a runnable online reproduction, I don't use CRA personally.

And the report should reference a specific eslint rule, which I think is actually not part of eslint-mdx at all.

@ChristianMurphy
Copy link
Member

@onichandame this is coming from from the airbnb preset
https://github.com/airbnb/javascript/blob/0375265cbd43635f8062615995a6a86f22fd0fc2/packages/eslint-config-airbnb/rules/react.js#L318-L320
To support mdx, override the "jsx-filename-extension" rule with

'react/jsx-filename-extension': ['error', { extensions: ['.jsx', '.mdx'] }]

in your eslint config.

@ChristianMurphy ChristianMurphy added 🙋 no/question This does not need any changes 🧘 status/waiting and removed 🐛 type/bug This is a problem 🙉 open/needs-info This needs some more info labels Mar 25, 2020
@onichandame
Copy link
Author

@JounQin

Google

Please see https://codesandbox.io/s/lingering-bush-7o3cn.

Basically I ran npx eslint --init and selected Google style during setup. The the linebreak-style is changed to CRLF as the server seems to use CRLF on default.

Now if you run npx eslint test.mdx --fix and reload test.mdx by simply clicking it in the Files list, some extra semicolons are added to the end of line 7, which is definitely wrong.

Standard

https://codesandbox.io/s/holy-fast-734lf
This example is created with the Standard style. if you run npx eslint test.mdx in the terminal you will see the error Expected an assignment or function call and instead saw an expression

The problem when Airbnb style is chosen is described in my initial post, a re-production can be easily made. If you need a reproduction for this please give me a shout.

@onichandame
Copy link
Author

@ChristianMurphy Thanks! One problem solved. Sadly others remain.

https://codesandbox.io/s/vigilant-elion-urmpf
If you run npx eslint test.mdx in the terminal, same problem which is mentioned in the Google style part persists.

@JounQin
Copy link
Member

JounQin commented Mar 25, 2020

I didn't run codebox actually, but the key problem is .mdx files require eslint-mdx as parser while other configs may overrie it.

So you can try to locate plugin:mdx/recommended to last position of configs so that it will handle all files correctly.

Or, you can use overrides feature of ESLint according to README.

@JounQin
Copy link
Member

JounQin commented Mar 25, 2020

And please provide what are the specific error outputs exactly... You comments maybe not so helpful to locate your real problems.

@onichandame
Copy link
Author

@JounQin Thanks but none of your suggestions worked. https://codesandbox.io/s/lingering-bush-7o3cn has been updated according to your advice.

  1. moved plugin:mdx/recommended to the end of the extends section.
  2. added overrides rules according to the README.

Steps to Reproduce

  1. go to https://codesandbox.io/s/lingering-bush-7o3cn
  2. fork so you can access the terminal with your own account
  3. open a new terminal by clicking + button (on the bottom right corner by default)
  4. run npx eslint test.mdx --fix

Output

Expected:

No error

Seen:

  1. the terminal prints 7:14 error Missing semicolon semi
  2. the line 3 of test.mdx has been changed from <div>hi</div>; to <div>hi</div>;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;

@JounQin
Copy link
Member

JounQin commented Mar 26, 2020

@onichandame OK, I just open an issue and track it at #183.

There is no workaround for this issue temporarily, you can disable ESLint rule semi for .mdx files for now.

{
  "overrides": [
    {
      "files": ".mdx",
      "rules": {
        "semi": 0
      }
    }
  ]
}

@JounQin JounQin closed this as completed Mar 26, 2020
@wooorm wooorm added 👀 no/external This makes more sense somewhere else and removed 🙋 no/question This does not need any changes 🧘 status/waiting labels Jul 23, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
👀 no/external This makes more sense somewhere else
Development

No branches or pull requests

4 participants