Great plugin, I love it! Tho it may be a challenge to fix all the warnings in a big codebase.

On recently upgrading, we found that our Jest/Enzyme tests have a bunch of new ESLint errors. Tests like:

describe('some component', () => {
  it('does the correct thing when color is red', () => {
    const color = 'red';
    const wrapper = shallow(<SomeComponent color={color} />);
    // ...
  });
});

Need to be re-written as:

const color = 'red';

describe('some component', () => {
  it('does the correct thing when color is red', () => {
    const wrapper = shallow(<SomeComponent color={color} />);
    // ...
  });
});

This is pretty far from ideal. I don't think that we will want to hoist all of test prop variables. It would be very difficult to read/reason about large test suites if we obey this rule there.

We're going to disable this plugin for our tests for now, but it seems like this might be something that should be handled out-of-the-box by the plugin in some way.

• Is there a way for findRelevantNodes to determine that the variable is declared in a non-React-component function? I'm not sure this is possible, but maybe there's a way.
• Could we maybe use some kind of callback exclusion list in the vein of #19?
• Should we be dealing with this in some other way? Or is it just preferred to turn the rules off for test files?

There is an issue with refactoring from defaultProps to default values on prop destructuring

reactjs/rfcs#107 (comment)

Since Dan Abramov's tweet https://twitter.com/dan_abramov, it should be common refactoring issue.

I wonder, could this plugin be implemented as a tsserver plugin so that I can write code like prop={{ blah: 123 }} with no problems.. it would just compile to JS with it hoisted.

my .eslintrc:

{ "extends": "eslint-config-airbnb",
  "env": {
    "browser": true,
    "node": true,
    "mocha": true
  },
  "rules": {
    "react/no-multi-comp": 0,
    "import/default": 0,
    "import/no-duplicates": 0,
    "import/named": 0,
    "import/namespace": 0,
    "import/no-unresolved": 0,
    "import/no-named-as-default": 2,
    "comma-dangle": 0,  // not sure why airbnb turned this on. gross!
    "indent": [2, 2, {"SwitchCase": 1}],
    "no-console": 0,
    "no-alert": 0,
    "no-unused-vars": 1, // set to warning instead of error (better for development)
    "id-length": [3, {"min": 2, "properties": "never", "exceptions": ["t", "_", "i", "j", "k"]}], // allow underscore
    "react-perf/jsx-no-new-object-as-prop": 1,
    "react-perf/jsx-no-new-array-as-prop": 1,
    "react-perf/jsx-no-new-function-as-prop": 1,
    "react-perf/jsx-no-jsx-as-prop": 1,
  },
  "plugins": [
    "react", "import", "react-perf",
  ],
  "settings": {
    "import/parser": "babel-eslint",
    "import/resolve": {
      "moduleDirectory": ["node_modules", "src"]
    },
  },
  "globals": {
    "__DEVELOPMENT__": true,
    "__CLIENT__": true,
    "__SERVER__": true,
    "__DISABLE_SSR__": true,
    "__DEVTOOLS__": true,
    "__API_PATH__": true,
    "webpackIsomorphicTools": true
  }
}

Atom throws the following error:

Error: Error while loading rule 'react-perf/jsx-no-new-object-as-prop': ruleCreator is not a function

TypeError: Error while loading rule 'react-perf/jsx-no-new-object-as-prop': ruleCreator is not a function
    at /home/tim/workspace/projects/sapientia-web/src/app/node_modules/eslint/lib/eslint.js:692:28
    at Array.forEach (native)
    at EventEmitter.module.exports.api.verify (/home/tim/workspace/projects/sapientia-web/src/app/node_modules/eslint/lib/eslint.js:671:16)
    at processText (/home/tim/workspace/projects/sapientia-web/src/app/node_modules/eslint/lib/cli-engine.js:230:27)
    at CLIEngine.executeOnText (/home/tim/workspace/projects/sapientia-web/src/app/node_modules/eslint/lib/cli-engine.js:686:26)
    at Object.execute (/home/tim/workspace/projects/sapientia-web/src/app/node_modules/eslint/lib/cli.js:162:36)
    at lintJob (/home/tim/.atom/packages/linter-eslint/lib/worker.js:38:10)
    at /home/tim/.atom/packages/linter-eslint/lib/worker.js:75:20
    at Emitter.emit (/home/tim/.atom/packages/linter-eslint/node_modules/sb-event-kit/lib/emitter.js:70:19)
    at /home/tim/.atom/packages/linter-eslint/node_modules/sb-communication/lib/main.js:22:23

The eslint ecosystem support for v8 is now pretty strong, and this library is the only remaining one in my stack requiring eslint 7.
Could you add support for it ?
Thanks in advance!

eslint recently released version 5.0.0. Would it be possible to add that version as a usable peerDependency in package.json?

I expect the lint to also work inside React hooks functions:

Every hook function (which is a function named use* should not constantly return a new reference (which is an object/function/array/JSX which is not created outside the hook nor inside a useMemo/useCallback).

Emotion adds the possibility to pass a css prop to all components that takes an object (of styling). It gets hoisted away with their babel plugin.

It would be nice to not have to globally disable the jsx-no-new-object-as-prop and jsx-no-new-array-as-prop (css prop can take an array also for style composition) rules or add a disable comment everywhere I add css 😅.

I am thinking something like this:

rules: {
  'jsx-no-new-object-as-prop': [
    'error', 
    { 
      exceptProps: ['css'], 
    }
  ],
}

or

rules: {
  'jsx-no-new-object-as-prop': ['error', 'css'],
}

or

rules: {
  'jsx-no-new-object-as-prop': ['error', ['css']],
}

I see errors in terminal console on yarn install:

I've been having problems with the Route component of react-router-dom. I saw a couple of comments on Reddit and some internet forums about a way to solve the warning/error that the jsx-no-jsx-as-prop rule throws, but honestly, it makes the code too complicated just to not get that error. It would be a good idea to ignore this component, since to keep a cleaner code, at least in this case this rule does not help much. Maybe it would be even better to have a function that gives us the possibility to put something like:

{
ignoreList: [“Route”],
}

I am not an expert, but I think that something like this would work better, since I don't know if there are other components out there that need something like this and instead of helping it would hurt the developer, but disabling this rule is not an option, because it is very useful in most cases.

Hi guys,

thanks for this great plugin! :-). I recently updated the library to version 3.1.0 and first of all, I was pleased with the new rules. But then I had an issue while refactoring the code:

I easily can refactor the "new objects" which are not related to the props:

render() {
   const style = {color: red};
    return (<div style={style} />)
}

No Problem. But I can't remove the "new objects" if they're related to the props because it has to be a new object in this case :

render() {
  const style = {left: this.props.left};
   return (<div style={style} />)
}

In our application, we have to calculate many inline styles (which can not be replaced by pure CSS). I don't want to disable the rule (by comment) above all style attributes, neither I want to disable the rule completely because it makes total sense in our project.
Is there a solution to this?

thank you very much

Hey there. I was just checking out the changes in 3.3.0.
I'm wondering what the motivation is for the nativeAllowList.
Are there no performance costs of passing objects/arrays to native attributes, or is there another reason one might want to allow them?

So, "JSX attribute values should not contain functions created in the same scope"

How I can use function toggleById if it uses open and close functions, passed from ContainerComponent props.

If I create this function out of ContainerComponent scope, how open and close will be passed into it?

const ContainerComponent = ({ open, close }) => {
    const toggleById = (id) => {
        if (id === 'some_id') open()
        else close()
    }

    return <OtherComponent toggleById={toggleById} />
}

The following patterns are not considered warnings:

<div style={{display: 'none'}} />

This creates a new style object on every render. Better to initialize and reference, and because this is a static object, do so outside of render.

const displayNone = {display: 'none'};

render() {
    return (<div style={displayNone} />)
}

See also: https://gist.github.com/ryanflorence/70f9ca97de0606212424694a3629428d#file-oh-no-jsx

Hi!

I just wondered if you had thought of also supporting non-JSX usages of React, where object/function identity is still a performance concern?

Thanks,
Oliver

emotion allows css={[aRules, bRules]} and does the right thing for re-rendering, so it should not be flagged as a performance issue. We can turn this off for native elements like div, but css is allowed on any element, including custom elements.

There are other examples like this, for example, I can accept style on a custom element and pass it unchanged to the native one, and so would make sense to allow style as an exception on custom elements too.

Commit 2d81814 added the new jsx-no-jsx-as-prop rule and added it immediately to the recommended configuration. However, commit 8f121da then removed it from recommended, notably without updating the README.md.

I could submit a pull request to update the README.md to match the code. Before I do that, though, I was wondering if there's any context on why this was not a recommended rule? Commit 8f121da doesn't provide any context

Hello,
the plugin currently not supports the newest version of Eslint. Are you able to fix this?

npm ERR! peer dep missing: eslint@^2.0.0 || ^3.0.0, required by [email protected]

Thanks!

Please write release notes to explain what changed from version to version. Either a commented git tag, or a changelog file.
Thank you!

So, considering the code below, which is supposedly popular in react-native, how should I re-design my code to remove the warning?

<Text
              style={[
                styles.smallBoldText,
                styles.greenText,
                styles.greenBackground,
              ]}>
              Some Text
</Text>

new Array is a legal alternate to new Array()

When we add this rules in a old project with dozen of class components, we need to handle with dozen of lint warnings.

@cvazac
In yarn install logs in ci/cd I see the warning:

warning " > [email protected]" has incorrect peer dependency "eslint@^3.0.0 || ^4.0.0 || ^5.0.0 || ^6.0.0".

We have eslint 7 installed and most of other plugins already support eslint 7

We report on:

  render() {
    return <Tag obj={{}} />
  }

But it's easily defeated with:

  render() {
    var o = {}
    return <Tag obj={o} />
  }

#6 (comment)