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 ignorePureComponents when using class expressions. #1122

Merged
merged 17 commits into from May 15, 2017
Merged

Fix ignorePureComponents when using class expressions. #1122

merged 17 commits into from May 15, 2017

Conversation

dreid
Copy link
Contributor

@dreid dreid commented Mar 23, 2017

This allows the use of a React.PureComponent when used in an expression context for prefer-stateless-function with ignorePureComponents.

Example:

const Foo = class extends React.PureComponent {
  render() {
    return <div>{this.props.foo}</div>;
  }
};

benstepp and others added 3 commits Feb 23, 2017
Currently prefer-stateless-function warns when using a decorated class
over a stateless function. The decorator syntax only works with classes,
so it makes sense not to warn in this case.

Fixes #1034
@ljharb
Copy link
Member

ljharb commented Mar 23, 2017

Should this only apply when the react version setting is >= v15.3?

@ljharb
Copy link
Member

ljharb commented Mar 23, 2017

(also, why use an expression in that example and not class Foo extends …?

@dreid
Copy link
Contributor Author

dreid commented Mar 23, 2017

(also, why use an expression in that example and not class Foo extends …?

It's just a minimal obviously correct syntax where ignorePureComponents doesn't work right.

This also fixes usages of non-decorator wrapper functions (and maybe export default class extends…?)

In practice this example probably shouldn't be allowed but that should probably be enforced by a more general eslint rule. :)

@dreid
Copy link
Contributor Author

dreid commented Mar 23, 2017

I'm not sure about the react version setting.

@ljharb
Copy link
Member

ljharb commented Mar 23, 2017

I'm not super concerned about people on v15.0, 15.1, or 15.2, but someone on v0.14 shouldn't get a pass from using PureComponent.

@dreid
Copy link
Contributor Author

dreid commented Mar 23, 2017

Sorry, I might be misunderstanding you, are you proposing that ignorePureComponents shouldn't do anything on < 0.15.3? There do not appear to be (but maybe I missed them) any existing version constraints on ignorePureComponents.

If so, should that be done in this PR? It seems like it's a larger change that would require more discussion.

ljharb
ljharb approved these changes Mar 23, 2017
Copy link
Member

@ljharb ljharb left a comment

You're right, since ignorePureComponents is already an option, dealing with versions isn't the job of this PR.

LGTM.

dreid and others added 13 commits Mar 28, 2017
Fix nextProps false positive in no-unused-prop-types
{Doc} fix indentation in no-unused-prop-types example
[New] Add no-will-update-set-state rule
Prepend "react/" to rule options documentation code
This fixes a link regarding ReactDOM.render. Previously these docs
were part of the Top Level API docs for React, but have since been
split into their own documentation.
…-value

Link fix: ReactDOM docs were broken out
prefer-stateless-function w/ decorators
@dreid
Copy link
Contributor Author

dreid commented Apr 18, 2017

Is there anything else I need to do for this? I noticed there were some conflicts so I resolved them.

This was approved almost a month ago and hasn't been merged so I just wanted to make sure this wasn't waiting on some action from me.

Remove trailing spaces.
@ljharb ljharb requested review from yannickcr, lencioni and EvHaus Apr 18, 2017
@yannickcr yannickcr merged commit 0dd4685 into jsx-eslint:master May 15, 2017
3 checks passed
@yannickcr
Copy link
Member

yannickcr commented May 15, 2017

Thanks for your work! And sorry for the delay :)

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

Successfully merging this pull request may close these issues.

None yet

8 participants