90

I have the following component that triggers a no-shadow ESlint error on the FilterButton props.

import { setFilter } from '../actions/filter';


function FilterButton({ setFilter }) {
  return (
    <button onClick={setFilter}>Click</button>
  );
}

export default connect(null, { setFilter })(FilterButton);

How can I avoid the warning while keeping both the concise syntax of mapDispatchToProps and the ESlint rule?

I know I can add a comment to suppress the warning but doing it for every components seems redundant and tedious.

8
  • You can rename setFilter (FilterButton({ setFilter }) to FilterButton({ setFilter })). It makes sense (sort of) because the functions that's in FilterButton's props is actually the original setFilter with the dispatch function bound to it. Jun 7, 2016 at 17:59
  • Before and after rename are the same.
    – Kerumen
    Jun 8, 2016 at 9:01
  • I meant renaming only in function FilterButton({ setFilter }) { and <button onClick={setFilter}>Click</button>. Can you update your question with the edited code? Jun 8, 2016 at 9:21
  • I can't rename it in function FilterButton({ setFilter }) because it has to match the name of the prop which is setFilter actually.
    – Kerumen
    Jun 8, 2016 at 9:58
  • 3
    Can't you just reassign when passing it in to the function on the export line? So, export default connect(null, {filter: setFilter})(FilterButton); and then above that just function FilterButton ({filter}) { (or whatever new variable name you prefer). This way you're not shadowing the variable in the upper scope, and that's clear when looking at the code. Jun 12, 2016 at 18:55

6 Answers 6

205

There are four options here:

1. Disable the rule.

Why?

It's the easiest way to avoid the ESLint error.

Why Not?

The no-shadow rule helps to prevent a very common bug when using react-redux. That is, attempting to invoke the raw, unconnected action (which does not automatically get dispatched).

In other words, if you were not using destructuring and grabbing the action from props, setFilter() would not dispatch the action (because you'd be invoking the imported action directly, as opposed to invoking the connected action through props via props.setFilter(), which react-redux automatically dispatches for you).

By cleaning up variable shadowing, you and/or your IDE are more likely to pick up on the error.

How?

Adding a eslintConfig property to your package.json file is one way to do this.

"eslintConfig": {
    "rules": {
      "no-shadow": "off",
    }
  }

2. Reassign the variable when passing it into connect().

Why?

You benefit from the safety of the no-shadow rule, and, if you choose to adhere to a naming convention, it's very explicit.

Why Not?

It introduces boilerplate.

If you do not use a naming convention, you now have to come up with alternate names (that still make sense) for every action. And chances are that the same actions will be named differently across components, making it harder to become familiar with the actions themselves.

If you do use a naming convention, names become long and repetitive.

How?

Without naming convention:

import { setFilter } from '../actions/filter';

function FilterButton({ filter }) {
  return (
    <button onClick={filter}>Click</button>
  );
}

export default connect(null, { filter: setFilter })(FilterButton);

With naming convention:

import { setFilter, clearFilter } from '../actions/filter';

function FilterButton({ setFilterConnect, clearFilterConnect }) {
  return (
    <button onClick={setFilterConnect} onBlur={clearFilterConnect}>Click</button>
  );
}

export default connect(null, {
  setFilterConnect: setFilter,
  clearFilterConnect: clearFilter,
})(FilterButton);

3. Don't destructure actions off of props.

Why?

By explicitly using the method off of the props object, you don't need to worry about shadowing to begin with.

Why Not?

Prepending all of your actions with props/this.props is repetitive (and inconsistent if you're destructuring all of your other non-action props).

How?

import { setFilter } from '../actions/filter';

function FilterButton(props) {
  return (
    <button onClick={props.setFilter}>Click</button>
  );
}

export default connect(null, { setFilter })(FilterButton);

4. Import the entire module.

Why?

It's concise.

Why Not?

Other developers (or your future self) may have trouble understanding what's going on. And depending on the style guide you're following, you might be breaking the no-wildcard-imports rule.

How?

If you're simply passing in action creators from one module:

import * as actions from '../actions/filter';

function FilterButton({ setFilter }) {
  return (
    <button onClick={setFilter}>Click</button>
  );
}

export default connect(null, actions)(FilterButton);

If you're passing in multiple modules, use object destructuring with rest syntax:

import * as filterActions from '../actions/filter';
import * as otherActions from '../actions/other';

// all exported actions from the two imported files are now available as props
function FilterButton({ setFilter, clearFilter, setOther, clearOther }) {
  return (
    <button onClick={setFilter}>Click</button>
  );
}

export default connect(null, { ...filterActions, ...otherActions })(FilterButton);

And since you mentioned a preference for ES6's concise syntax in the comments, might as well throw in the arrow function with an implicit return:

import * as actions from '../actions/filter';

const FilterButton = ({ setFilter }) => <button onClick={setFilter}>Click</button>;

export default connect(null, actions)(FilterButton);
11
  • 1
    Thank you! This should be the accepted answer for sure. Mar 23, 2017 at 11:13
  • 5
    Very well written answer. Although, one should stop reading after Disable the rule :)
    – Mrchief
    Nov 15, 2017 at 15:38
  • 3
    How exactly does no-shadow help prevent invoking the raw action? It throws an error when you get it right (assuming destructured props) and does NOT when you call the raw action. The answer by @GollyJer is a great addition and should be appended to this answer's first part. It is very likely to be the best practice, all things considered.
    – basse
    Oct 12, 2018 at 12:19
  • 1
    As a convention, I'm tempted to just prepend $ to the names of the actions and use the mapDispatchToProps shorthand, but I've always found $ a little jarring, even with jQuery.
    – Kevin Dice
    Nov 29, 2018 at 19:35
  • 2
    I'm using the #2 Reassign the variable when passing it into connect(). I'm adding an Action suffix (i.e., setFilter is setFilterAction). Thank you for other options and your insights to each of them.
    – kent
    Jun 1, 2019 at 9:40
16

A fifth option:

5. Allow a specific exception via eslintrc rules.

module.exports = {
  rules: {
    'no-shadow': [
      'error',
      {
        allow: ['setFilter'],
      },
    ],
  }
}

Why?

You don't want variable shadowing but can't get around it in certain cases.

Why Not?

You really don't want variable shadowing in your code base. 😝

1
  • I like this option, but it is not working for me I still see the error
    – saas_joel
    Jan 4, 2021 at 18:52
9

Option number six.

6. Disable the es-lint rule for the specific line(s) of code

import { setFilter } from '../actions/filter';


// eslint-disable-next-line no-shadow
function FilterButton({ setFilter }) {
  return (
    <button onClick={setFilter}>Click</button>
  );
}

export default connect(null, { setFilter })(FilterButton);

or

import { setFilter } from '../actions/filter';


/* eslint-disable no-shadow */
function FilterButton({ setFilter }) {
/* es-lint-enable */
  return (
    <button onClick={setFilter}>Click</button>
  );
}

export default connect(null, { setFilter })(FilterButton);

The second way to temporarily disable the es-lint rule can be used for multiple lines of code, unlike the first one. It can be useful if you pass more arguments and divide them into multiple lines of code.

Why?

This is an easy and suitable option for some use-cases (for example, your team/organization uses specific es-lint settings and it is discouraged/forbidden to modify those settings). It disables the es-lint error in the line(s) of code but does not influence on mapDispatchToProps syntax and the rule is still completely active outside the line(s) of code.

Why not?

You do not want or you are forbidden to bloat your code with such kind of comments. You do not want or you are forbidden to influence es-lint behaviour.

6

I tweaked 4. and achieved what I would like to call option 8.

8. Import methods under a different name

Why?

It has the same benefits as importing the entire module but without conflicting with other rules, e.g. Do not use wildcard imports(airbnb).

Why not?

It's adding an unnecessary variable declaration that might cause confusion.

How?

For the single method case

import { setFilter as setFilterConnect } from '../actions/filter';

function FilterButton({ setFilter }) {
  return <button onClick={setFilter}>Click</button>;
}

export default connect(
  null,
  { setFilter: setFilterConnect }
)(FilterButton);
6

With the new Hooks API added in v7.1.0, you can get rid of the variable and mapDispatchToProps altogether:

import { useDispatch } from 'react-redux'
import { setFilter } from '../actions/filter';

function FilterButton() {
  const dispatch = useDispatch()
  return (
    <button onClick={() => dispatch(setFilter())}>Click</button>
  );
}

export default FilterButton;
3

Option 7...

7. Use container components

Why?

It's a known pattern and you get the added benefit of decoupling your components from the redux store, making them easier to be reused.

Why not?

You now need two files per component.

How?

// FilterButton.jsx

export default function FilterButton({ setFilter }) {
  return (
    <button onClick={setFilter}>Click</button>
  );
}
// FilterButtonRedux.jsx

import FilterButton from './FilterButton';
import { setFilter } from '../actions/filter';

export default connect(null, { setFilter })(FilterButton);
3
  • Another point to "why not": it's now possible to import a component with a very similar name to the intended one that is not connected to redux.
    – tao_oat
    Jan 15, 2020 at 13:31
  • @tao_oat put these files inside a FilterButton/ directory with an index.js exporting only the connected component. It makes for a clear public API, and it's then easier to unit test the isolated unconnected component, e.g. inside FilterButton/FilterButton.test.jsx Apr 22, 2021 at 2:35
  • This works well for simple components (Yes, I hear you all that all of the components should be simple). For more complex components (= more props) you will get a lot of repetition in prop types Mar 10 at 4:10

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge that you have read and understand our privacy policy and code of conduct.

Not the answer you're looking for? Browse other questions tagged or ask your own question.