jsx-eslint / eslint-plugin-react

React-specific linting rules for ESLint
MIT License
9k stars 2.77k forks source link

"Component definition is missing display name" with React.createElement #597

Closed nihey closed 8 years ago

nihey commented 8 years ago

Hello eslint-plugin-react maintainers, your plugins has surely been of great help so far :)

But I recently had problems with react/display-name rule, I have several functions that return React.createElement instances on my code, and they seem to be triggering react/display-name rule, even though I'm not creating a Component.

I have isolated the problem on the following code:

var Utils = {
  label: function(settings) {
    return <span className={'label ' + settings.klass}>{ settings.text }</span>;
  },
};

export default Utils;

The minimal .eslintrc code I need to make that work is:

{
  "rules": {
    "react/display-name": [2],
  },
  "parserOptions": {
    "sourceType": "module",
    "ecmaFeatures": {
      "jsx": true
    }
  },
  "plugins": [
    "react",
  ]
}

As I am seeing this, the Utils.label function should not be triggering react/display-name rule. A <span></span> instance will generate a React.createElement instance, so no displayName should be required for it, right?

Edit:

This may be related to: https://github.com/yannickcr/eslint-plugin-react/issues/512

ljharb commented 8 years ago

A function that returns jsx (ie, that generates a React.createElement) is in fact a stateless functional component - so the rule would expect it to have a display name, propTypes, etc.

You're also correct that any solution to #512 would also work here. However, for now I'd recommend just disabling the rule on that line/file.

Jessidhia commented 8 years ago

It seems this is also triggered when using thunks that return ReactElements (...which look very similar to a SFC, but are not quite the same).

Here's an example, where t is a function that, for the given arguments, returns ['String with ', <a href='/'>{'replacement'}</a>, '']:

<span>
  {t('String with %(a:replacement)', {
    a: text => <a href='/'>{text}</a> /* react/display-name warns here */
  })}
</span>
mjackson commented 8 years ago

A function that returns jsx (ie, that generates a React.createElement) is in fact a stateless functional component

While such functions may be used as components (i.e. passed as the first arg to React.createElement), I think it's too aggressive to assume that all such functions are components. As render props become more and more common (ala react-motion and react-router) this is going to be a larger concern.

I'd say we can close this issue as a dup of #512

ljharb commented 8 years ago

I agree this is a duplicate of #512.

sturmenta commented 6 years ago
//warn
const wrapWithToastProvider = Comp => props => (
    <ToastProvider>
        <Comp {...props} />
    </ToastProvider>
);

//warn
function wrapWithToastProvider(Comp) {
  return function (props) {
    return (
        <ToastProvider>
            <Comp {...props} />
        </ToastProvider>
    );
  };
}

Any help?

ljharb commented 6 years ago

@sturmenta neither of those is a named function:

function wrapWithToastProvider(Comp) {
  return function WrappedWithToast(props) {
    return (
        <ToastProvider>
            <Comp {...props} />
        </ToastProvider>
    );
  };
}
nikolaisdfsd commented 5 years ago

@ljharb is it possible to do something similar with arrow functions?

const AppWrapper => Comp => props => <div><Comp {...props} /></div>

I thought that it should automatically pass AppWrapper as component's name

ljharb commented 5 years ago

@nikolai-s absolutely not; the language’s inference will never extend beyond your outer HOC. If you want your components to reliably have names, do not use arrow functions for them, ever.

S4elkyn4eGGG commented 4 years ago

"react/display-name": "off"

phiresky commented 4 years ago

The example in the original post here:

var Utils = {
  label: function(settings) {
    return <span className={'label ' + settings.klass}>{ settings.text }</span>;
  },
};

Is in fact a false positive of this rule, since the display name of the function is correctly set to the key in literal objects since ES6: https://2ality.com/2015/09/function-names-es6.html

For example:

const x = { foo: () => {} }

x.foo.name // === "foo" even though the function is an anonymous lambda
ljharb commented 4 years ago

@phiresky you have to enable ignoreTranspilerNames to be able to rely on inference (which i don’t suggest doing)

phiresky commented 4 years ago

Ah, I didn't see there was a configurable option for that (link). Docs don't listing anything about it not being recommended. Why would you not suggest it?

ljharb commented 4 years ago

Function name inference is specified, but not intuitive and will not always apply when one thinks it does. It's far, far better for debuggability to always and only use explicit names.