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

react/prefer-stateless-function with PureComponent. #1773

Closed
ghost opened this issue Apr 19, 2018 · 15 comments
Closed

react/prefer-stateless-function with PureComponent. #1773

ghost opened this issue Apr 19, 2018 · 15 comments

Comments

@ghost
Copy link

ghost commented Apr 19, 2018

I'm using:

"react/prefer-stateless-function": [1, { "ignorePureComponents": true }]

but I'm also using this new Query component by react-apollo:

import React, { PureComponent } from "react";
import gql from "graphql-tag";
import { Query } from "react-apollo";

const GET_DOGS = gql`
  {
    dogs {
      id
      breed
    }
  }
`;

class DogsLove extends PureComponent {
    render() {
        return (
            <Query query={GET_DOGS}>
                {({ loading, error, data }) => {
                if (loading) return "Loading...";
                if (error) return `Error! ${error.message}`;

                return (
                    <select name="dog" onChange={onDogSelected}>
                    {data.dogs.map(dog => (
                        <option key={dog.id} value={dog.breed}>
                        {dog.breed}
                        </option>
                    ))}
                    </select>
                );
                }}
            </Query>
        )
    }
}

So, why this is a warning? (Or an error if I disable ignorePureComponents?)

I just need PureComponent because I don't need to re-render DogsLove component every time I switch Route. I need it to render just one time, the first time. Then Query component is making an observable and nothing more.

Where am I wrong?

@ghost
Copy link
Author

ghost commented Apr 19, 2018

Scenario and codesandbox here: apollographql/react-apollo#1938

@ljharb
Copy link
Member

ljharb commented Apr 20, 2018

Can you paste the actual warning message (and denote the line it points to)?

@ljharb
Copy link
Member

ljharb commented Apr 20, 2018

Separately, if you truly only want to render once, you’d want a normal class component with a shouldComponentUpdate that returns false.

@ghost
Copy link
Author

ghost commented Apr 20, 2018

Can you paste the actual warning message (and denote the line it points to)?

All the component is in warning:

Component should be written as a pure function (react/prefer-stateless-function)

@ljharb
Copy link
Member

ljharb commented Apr 20, 2018

@johnunclesam what if you extends React.PureComponent (instead of bringing it in as a named import)?

@deemoding
Copy link

In lib/rules/prefer-stateless-function.js line 374,I find this code

if (list[component].hasSCU && list[component].usePropsOrContext) {
    continue;
}

But,I find that list[component].usePropsOrContext === undefined ,so that the next code isn't skiped.
So what's the usage of list[component].usePropsOrContext?Can I delete it? @yannickcr

@callumlocke
Copy link

I have the same problem. ignorePureComponents: true doesn't seem to do anything.

Doesn't matter if I extend PureComponent or React.PureComponent.

@JavierLaguna
Copy link

I have the same problem. ignorePureComponents: true doesn't seem to do anything.

Doesn't matter if I extend PureComponent or React.PureComponent.

@nickbouton
Copy link

Same problem here as well. If you add props or anything beyond a simple return to render, though, it stops complaining.

@dimpiax
Copy link

dimpiax commented Jan 8, 2019

As the option, if you are experimenting with views you can avoid this error writing:

render() {
  this.props; // declare this!

  return (<h1>Hello</h1>);
}

@FlaviooLima
Copy link

This bug need some attention :)

@golopot
Copy link
Contributor

golopot commented Apr 29, 2019

@ljharb ljharb closed this as completed Apr 30, 2019
@JavierLaguna
Copy link

The fix is merged on the current version v7.12.4 ?

@golopot
Copy link
Contributor

golopot commented Apr 30, 2019

No, the pr was merged 18 days ago while v7.12.4 is released 3 months ago.

@JavierLaguna
Copy link

No, the pr was merged 18 days ago while v7.12.4 is released 3 months ago.

Thank you!

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

No branches or pull requests

8 participants