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

3.6.2: 'react/display-name' throws false-positive in mixin methods that return JSX #256

Closed
lyrebird opened this issue Oct 18, 2015 · 10 comments

Comments

@lyrebird
Copy link

In the latest release, I'm hitting what appears to be a false-positive on display-name in a mixin. Here's a simplified example of the code that triggers it.

const Mixin = {
  renderButton() {
    return (
      <button />
    );
  }
};

It wasn't triggered prior to 3.6.0 -- I suspect it may be related to the changes to support stateless function components here: #237

A temporary work-around is to assign the JSX to a variable, and then return that. It only seems to trigger when a block of JSX is directly returned from the method.

@mmrko
Copy link

mmrko commented Oct 18, 2015

Ditto. I wonder if it were enough to just apply this rule on functions that are declared at the top level and return JSX...

@yannickcr
Copy link
Member

Actually your mixin is a valid stateless function component, you can totally render it with <Mixin.renderButton />. So I'm afraid there is no way for us to know if it's a "real" stateless component or not.

(Even so, I've found a few issues with stateless functions and display-name, I'll try to fixes that asap)

@iam4x
Copy link

iam4x commented Oct 19, 2015

@yannickcr I don't think display-name should be used on plain function component or did I miss something?

Can we just disable checking for display-name on component function?

(ps: Thank's for the plugin 💯)

@epmatsw
Copy link
Contributor

epmatsw commented Oct 19, 2015

This is a pretty frustrating bug actually. I'm getting multiple display-name errors per file now, because functions like _renderRow that return JSX and are used inside of a component's render are being detected as their own components. I don't know how to fix it, but I feel like sub-renderers like my case are a reasonably common pattern...

@epmatsw
Copy link
Contributor

epmatsw commented Oct 19, 2015

@iam4x That's an interesting question too. There's no mention of it in the 0.14 release notes, but it seems like displayName should be supported. It's a semi-defacto-standard not-React-specific thing after all.

@yannickcr
Copy link
Member

@iam4x @epmatsw Thanks for your feedback. I plan to release a few bugfixes for theses issues this evening.

@jsg2021
Copy link
Contributor

jsg2021 commented Oct 19, 2015

I believe the displayName of function components is the function's name... so if it's anonymous, the component is also anonymous... i agree with @iam4x , function components shouldn't be bound to this lint rule. It's a different animal.

@epmatsw
Copy link
Contributor

epmatsw commented Oct 19, 2015

The displayName can be calculated implicitly by using the function name, or specified explicitly using displayName. The react dev tools will use displayName for function components if provided (http://codepen.io/epmatsw/pen/dYZxwB), which makes me think that that's an intentional feature of function components.

You can write "anonymous" traditional components, but it's generally agreed as a bad practice, as evidenced by this rule. Since from a debugging perspective the two types of component are indistinguishable, I don't think it makes sense to treat them differently. Perhaps a reasonable compromise would be to add an opt-out flag to allow function components to not have a displayName?

@jsg2021
Copy link
Contributor

jsg2021 commented Oct 19, 2015

That code pen does not prove that it's an intentional feature. That's just where the property is looked for on the prototype/class. In the case of simple function components, it's the same place.

Adding a Comp.displayName = "foo" after the function's declaration is a bit counter to the spirit of these new style components. imo.

I wasn't advocating writing anonymous components. I was just using it as an example.

@yannickcr
Copy link
Member

I just published v3.6.3 and it should address most of your cases. Please open another issue if I missed one.

Also, if you do not want to set a displayName to your stateless components, I suggest you to set the acceptTranspilerName option to true (IMHO it should already be true by default, probably in the next major release).

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

No branches or pull requests

6 participants