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

false positives with react/no-multi-comp in v7.0.0 #1181

Closed
nickbouton opened this issue May 8, 2017 · 11 comments
Closed

false positives with react/no-multi-comp in v7.0.0 #1181

nickbouton opened this issue May 8, 2017 · 11 comments
Labels

Comments

@nickbouton
Copy link

nickbouton commented May 8, 2017

Following up on #1088, I seem to be running into false positives problem again with react/no-multi-comp in v7.0.0.

Any component method (in ES5, at least) that has a name starting with render... and returns something non-empty appears to falsely trigger this rule.

e.g.:
image

If I roll back to v6.10.3, the above warning does not occur.

@yannickcr
Copy link
Member

Can you provide a complete code sample ?

I tested with:

var Hello = createReactClass({
  renderSomething: function() {
    return (
      <div />
    );
  },
  render: function() {
    return (
      <div />
    );
  }
});

But I was not able to reproduce the bug.

@nickbouton
Copy link
Author

I'll post an example tomorrow, this is using ES5 / React.createClass syntax mind you.

@paulruescher
Copy link

paulruescher commented May 9, 2017

I'm getting the same while updating React from 15.4.1 to 15.5.4. Only changes I really see are migrating from React.createClass and React.PropTypes to their respective standalone packages create-react-class and prop-types.

I'll circle back with more info as I dig deeper.

@nickbouton
Copy link
Author

nickbouton commented May 9, 2017

@paulruescher @yannickcr This was happening to me when upgrading to React 15.5.4 as well, FWIW.

Edit: After converting the same file I was having issues with to using ES6 class syntax, the react/no-multi-comp warning no longer shows up.

@jomasti
Copy link
Contributor

jomasti commented May 18, 2017

This can happen with the latest version if one imports create-react-class as something other than createReactClass. With the proper setting for createClass, it doesn't trigger the rule.

@artyomtrityak
Copy link

Same issue if create-react-class defined by ProvidePlugin

new webpack.ProvidePlugin({
    createReactClass: 'create-react-class'
  })

@matt5784
Copy link

I am also having this problem using ES6, eslint ^4.19.x and eslint-plugin-react ^7.7.x

I was able to make up a relatively simple example that triggers it:

File SubComponent.jsx (this one doesn't trigger the false positive, just needed to have something to pass a function to):

import PropTypes from "prop-types";
import React from "react";

function SubComponent() {
  return this.props.returnFunction();
}

SubComponent.propTypes = {
  returnFunction: PropTypes.func.isRequired
};

export default SubComponent;

File ItemView.jsx (this one triggers the false positive on the internal function "renderView":

import React from "react";
import { Button } from "react-bootstrap";
import { Link } from "react-router-dom";
import SubComponent from "./SubComponent";

function ItemView() {
  function renderView() {
    return (
        <Link to="/create">
          <Button variant="primary">Create</Button>
        </Link>
    );
  }

  return (
      <div>
        <SubComponent returnFunction={renderView} />
      </div>
  );
}

export default ItemView;

It seems that it doesn't like internal functions which return components, as that is always what the eslint warning is pointing at.

However, I have a very similar component that does not trigger it. It's not entirely clear why this one does. They both define an internal function which returns a component, and they both pass that function as a prop into a different component in their actual return value. In both cases (the component which triggers a false positive and the component which does not) there is only a single export in the whole file.

@ljharb
Copy link
Member

ljharb commented Sep 25, 2019

@matt5784 in that example, renderView is a component - it's a function that returns jsx. In that specific pattern, you're also recreating that component on every render, which seems quite inefficient.

@matt5784
Copy link

Is that how that works? My understanding is that my (functional) component is ItemView. renderView is just an inner function that happens to return a node. I'm only exporting one thing. Would it still be considered a component if I inline it in the return value? Maybe I just don't understand what this rule does, what I was trying to prevent was exporting multiple components from a single file.

As for the inefficiency, do you have any suggestions for improvements? In the actual usage the generic sub component needs to show a bunch of different types of components so it's difficult to model as anything other than a function that returns a node

@ljharb
Copy link
Member

ljharb commented Sep 25, 2019

The rule doesn’t filter to only exported components (altho that’d be a great option to have) - the rule is currently for multiple components defined in the file.

I’d suggest extracting it out to be an actual separate component, that takes props and renders the right thing.

@ljharb
Copy link
Member

ljharb commented Feb 4, 2022

This seems answered.

A PR to add an option to only warn on more than one exported component would be amazing. Short of that, I think the rule's working.

@ljharb ljharb closed this as completed Feb 4, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Development

No branches or pull requests

7 participants