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

Fix ESLint warnings #192

Open
koistya opened this issue Jan 12, 2017 · 16 comments
Open

Fix ESLint warnings #192

koistya opened this issue Jan 12, 2017 · 16 comments

Comments

@koistya
Copy link
Member

koistya commented Jan 12, 2017

Is anyone willing to help? Run yarn lint in a terminal window. See package.json/eslintConfig.

image

@rajpatel507
Copy link
Contributor

I can help if you can provide more info.

@koistya
Copy link
Member Author

koistya commented Jan 12, 2017

@rajpatel507 assuming you have Node.js v6+ and Yarn installed, when you run yarn lint it prints lots of ESLint warnings. It happened after the latest update of eslint and eslint-config-airbnb modules. Some rules can be disabled/tweaked if needed (see package.json/eslintConfig), but ideally we want to reuse AirBnb's ESLint preset as much as possible.

https://github.com/airbnb/javascript
https://github.com/airbnb/javascript/tree/master/packages/eslint-config-airbnb

@ericlau-solid
Copy link

@rajpatel507 you got what you needed and making progress? Checking in because I am interested. Thanks.

@rajpatel507
Copy link
Contributor

@ericlau-solid I am getting around 40 errors and 2 warnings, is it normal or i am missing something?

@ericlau-solid
Copy link

ericlau-solid commented Jan 23, 2017

@rajpatel507 Fixing the errors and warnings is the ask. You know you are done when the package still works and you get no errors and warnings.

@rajpatel507
Copy link
Contributor

@ericlau-solid some errors indicating that package is not in package.json dependency and still it is used but we have all dependency there in package.json, this is usally when eslint is not finding package.json file in source .I need to investigate why .eslintrc file is not pointing to package.json

@ericlau-solid
Copy link

@rajpatel507 there are multiple package.json files. when running eslint, it only looks at the package.json closest to the file in question. All package.json in sub-folders need to have dependencies object correctly set up.

@giantrobotbee
Copy link

Is there any status on this?

@rajpatel507
Copy link
Contributor

@drmanitoba not able to figuring out why error is there even if package is there in dev dependency of package.json

@frenzzy
Copy link
Member

frenzzy commented Feb 2, 2017

@rajpatel507 just add "import/no-extraneous-dependencies": "off" to package.json until the related issue exists (PR)

@rajpatel507
Copy link
Contributor

@frenzzy Thanks for suggestion, I have added "import/no-extraneous-dependencies": "off" and "env": { "browser": true } in package.json now warning comes down to 17 from 42 earlier, remaining warnings are from missing comma somewhere and other cosmetics.

@koistya should I remove all other warning or create pull request for this changes only?

@frenzzy
Copy link
Member

frenzzy commented Feb 2, 2017

This issue about fixing all eslint errors and warnings

@rajpatel507
Copy link
Contributor

I changed code according to ESLint rule and some places i have disabled ESLint for a line. it solved all error message from ESLint.

Still i am getting these two warning can anybody help me how to solve these.
image

@frenzzy
Copy link
Member

frenzzy commented Feb 2, 2017

I think you can just add inline comment to make it clear for developers that this was done deliberately and not by mistake, for example:

<div
  // eslint-disable-next-line react/no-danger
  dangerouslySetInnerHTML={{ __html: html }}
/>

ref react/no-danger

@rajpatel507
Copy link
Contributor

I have created pull request for same.

@frenzzy
Copy link
Member

frenzzy commented Feb 2, 2017

@rajpatel507 Awesome! I just reviewed it #199

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

5 participants