Eslint - enforces many more rules. #139

2015-10-05T05:23:25-07:00

ColinEberhardt commented

2015-10-05 05:23:25 -07:00

(Migrated from github.com)

Based on the review comments I saw for #138 I thought the linting rules could do with being 'tightened up'.

I checked the .eslintrc and found that the only thing being checked was the quotes style, and that was only being applied to Layout.js!

I've auto-configured ESLint based on the existing codebase, then added a few rules that I think are most important. Let me know what you think.

Based on the review comments I saw for #138 I thought the linting rules could do with being 'tightened up'. I checked the `.eslintrc` and found that the only thing being checked was the quotes style, and that was only being applied to `Layout.js`! I've auto-configured ESLint based on the existing codebase, then added a few rules that I think are most important. Let me know what you think.

ColinEberhardt commented

2015-10-06 23:59:00 -07:00

(Migrated from github.com)

@vjeux @devongovett - I've relaxed the eqeqeq rule to permit eqeq checks against null, as @devongovett rightly points out this allows you to check for both null and undefined with a single test.

ColinEberhardt commented

2015-10-06 23:59:58 -07:00

(Migrated from github.com)

As an side, it does feel like the getLeadingMargin function, and its neighbours, share a lot of common code. I'll have a look at refactoring as a separate PR.

As an side, it does feel like the `getLeadingMargin` function, and its neighbours, share a lot of common code. I'll have a look at refactoring as a separate PR.

sophiebits commented

2015-10-07 00:01:07 -07:00

(Migrated from github.com)

See how React and Relay have eslint set up – they use a common config from the fbjs-scripts package.

ColinEberhardt commented

2015-10-07 13:53:29 -07:00

(Migrated from github.com)

@spicyj thanks for the heads up, didn't realise there was a centralised lint configuration. I've updated the PR to use the fbjs-scripts eslint configuration and fixed all errors / warnings.

@spicyj thanks for the heads up, didn't realise there was a centralised lint configuration. I've updated the PR to use the `fbjs-scripts` eslint configuration and fixed all errors / warnings.

vjeux commented

2015-10-07 14:05:16 -07:00

(Migrated from github.com)

Sweet! Outside of the typeof undefined and null, looks very good! Thanks for doing that

ColinEberhardt commented

2015-10-07 14:19:36 -07:00

(Migrated from github.com)

please use x === undefined instead of typeof x === 'undefined'

Fair point, guarding against redefinition of undefined is a little unnecessary, and does make the code less readable. I've made these updates.

Thanks for doing that

No problems :-)

> please use x === undefined instead of typeof x === 'undefined' Fair point, guarding against redefinition of `undefined` is a little unnecessary, and does make the code less readable. I've made these updates. > Thanks for doing that No problems :-)

vjeux commented

2015-10-07 14:20:35 -07:00

(Migrated from github.com)

Feel free to land if travis passes :)

ColinEberhardt commented

2015-10-07 14:26:02 -07:00

(Migrated from github.com)

Travis is happy - all done 👍

Travis is happy - all done :+1:

Sign in to join this conversation.

1 Participants

Notifications

Due Date

No due date set.

Dependencies

No dependencies set.

Reference: DaddyFrosty/yoga#139