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

Use a custom ESLint rule list #27

Merged
merged 3 commits into from
Jul 19, 2016
Merged

Use a custom ESLint rule list #27

merged 3 commits into from
Jul 19, 2016

Conversation

gaearon
Copy link
Contributor

@gaearon gaearon commented Jul 19, 2016

Work in progress.
I think this also fixes #19.

@ghost ghost added the CLA Signed label Jul 19, 2016

// http://eslint.org/docs/rules/

'array-callback-return': ERROR,
Copy link
Contributor

Choose a reason for hiding this comment

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

What do you think about sorting these? Just kind of annoying to find things otherwise

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yeah, totally. I was mostly looking at airbnb's configs, and they are split into multiple files, so I ended up with several sorted “regions”. I wouldn’t spend time on this right now because there are more pressing issues, but this would be nice.

'object-property-newline': [WARNING, {
allowMultiplePropertiesPerLine: true,
}],
'one-var': [WARNING, 'never'],
Copy link
Contributor

Choose a reason for hiding this comment

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

It seems like it should be ok for people to do multiple declarations in a line? this config and one-var-declaration-per-line are relevant

@lacker
Copy link
Contributor

lacker commented Jul 19, 2016

Doh, sorting the rules might have rendered most of my comments confused. Hrmm

@lacker
Copy link
Contributor

lacker commented Jul 19, 2016

OK summarizing because the sorting borked a bunch of comments. To me these rules seem like they do not catch bugs, they just enforce one style over another equally-ok style:

indent camelcase one-var one-var-declaration-per-line no-mixed-spaces-and-tabs react/jsx-indent prefer-rest-params import/no-mutable-exports prefer-arrow-callback no-var

@gaearon
Copy link
Contributor Author

gaearon commented Jul 19, 2016

It’s fine, thanks! I relaxed per your suggestions but I’m sticking with no-var.
There are many ways to screw up with vars and there is no reason to keep using them in new apps.

@gaearon
Copy link
Contributor Author

gaearon commented Jul 19, 2016

(It may be possible that people will complain, and if enough people do, we’ll turn no-var off. But I’m going to be optimistic for now.)

@lacker
Copy link
Contributor

lacker commented Jul 19, 2016

OK, we'll see how it goes ;-) I personally am anti-var but it's still in tons and tons of sample code.

Same goes for "require" - "import" is obviously better, but tons of sample code still uses "require".

@gaearon
Copy link
Contributor Author

gaearon commented Jul 19, 2016

One thing we could do is add an autofix script. I think @ForbesLindesay wrote an autofix rule for let/const, so we could offer that as part of the package.

@ghost ghost added the CLA Signed label Jul 19, 2016
@gaearon
Copy link
Contributor Author

gaearon commented Jul 19, 2016

I’ll get it in so we don’t rebase hell each other

@gaearon gaearon merged commit 3d77f5b into master Jul 19, 2016
@gaearon
Copy link
Contributor Author

gaearon commented Jul 19, 2016

Note: travis probably failed because of node_modules cache. I just cleared it for the future.

stayradiated pushed a commit to stayradiated/create-react-app that referenced this pull request Nov 13, 2016
fix jest moduleNameMapper to handle other styling libraries
@lock lock bot locked and limited conversation to collaborators Jan 22, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

unejected version does not work for a subdirectory of another npm module
3 participants