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-config-react): Add static-variables to react/sort-comp #70

Merged
merged 1 commit into from Oct 24, 2019

Conversation

koggdal
Copy link
Contributor

@koggdal koggdal commented Oct 24, 2019

eslint-plugin-react was updated (jsx-eslint/eslint-plugin-react#2408) to support a new static-variables option to react/sort-comp. With this change, our linting started breaking. After some investigation, it turns out that statics seems to refer to some old React.createClass option, so static variables actually fell into the check for static-methods. Now that is fixed and static-methods only check for methods and static-variables only check for variables. So it doesn't seem like a breaking change, but we seem to have misused the options a bit.

By only adding static-variables to our list we should still support older apps using React.createClass (if any), and everything will work again like before.

`eslint-plugin-react` was updated (jsx-eslint/eslint-plugin-react#2408)
to support a new `static-variables` option to `react/sort-comp`. With this
change, our linting started breaking. After some investigation, it turns out
that `statics` seems to refer to some old `React.createClass` option, so
static variables actually fell into the check for `static-methods`. Now that is
fixed and `static-methods` only check for methods and `static-variables` only
check for variables. So it doesn't seem like a breaking change, but we seem to
have misused the options a bit.

By only adding `static-variables` to our list we should still support older
apps using `React.createClass` (if any), and everything will work again like
before.
Copy link
Collaborator

@alde alde left a comment

Choose a reason for hiding this comment

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

sounds good to me

@alde alde merged commit b2c38e7 into spotify:master Oct 24, 2019
@koggdal koggdal deleted the eslint-config-react-static-variables branch October 24, 2019 15:57
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants