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

Enable no-use-before-define rule #13606

Merged
merged 1 commit into from Sep 10, 2018
Merged

Enable no-use-before-define rule #13606

merged 1 commit into from Sep 10, 2018

Conversation

gaearon
Copy link
Collaborator

@gaearon gaearon commented Sep 9, 2018

This would have caught #13582 (comment).

For now, I had to add suppressions for cyclical Flow references but we can remove this after babel/babel-eslint#584 is merged and released.

I also enabled it for classes. Maybe this is excessive but I figured it doesn't hurt since classes aren't hoisted. I had to reorder two places because of that but it's just copy paste.

@@ -37,6 +37,7 @@ module.exports = {
'no-shadow': ERROR,
'no-unused-expressions': ERROR,
'no-unused-vars': [ERROR, {args: 'none'}],
'no-use-before-define': [ERROR, {functions: false, variables: false}],
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Naming of these options is confusing. It basically means "still allow to rely on function hoisting, and don't complain about variables unless they are in the same scope".

Copy link
Contributor

Choose a reason for hiding this comment

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

Naming of these options is confusing. It basically means "still allow to rely on function hoisting, and don't complain about variables unless they are in the same scope".

If .eslintrc.js is a .js would it make sense to put this explanation in a comment next to the added option?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Maybe, but then we almost never change the config so I don't think it's that helpful. Can always blame to a PR if necessary.

@pull-bot
Copy link

pull-bot commented Sep 9, 2018

Details of bundled changes.

Comparing: 8a8d973...7e52e2f

schedule

File Filesize Diff Gzip Diff Prev Size Current Size Prev Gzip Current Gzip ENV
schedule.development.js n/a n/a 0 B 19.17 KB 0 B 5.74 KB UMD_DEV
schedule.production.min.js n/a n/a 0 B 3.16 KB 0 B 1.53 KB UMD_PROD

Generated by 🚫 dangerJS

@gaearon gaearon merged commit 144328f into facebook:master Sep 10, 2018
@gaearon gaearon deleted the no-unused branch September 10, 2018 15:15
Simek pushed a commit to Simek/react that referenced this pull request Oct 25, 2018
jetoneza pushed a commit to jetoneza/react that referenced this pull request Jan 23, 2019
NMinhNguyen referenced this pull request in enzymejs/react-shallow-renderer Jan 29, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants