Eslint-plugin-react: destructuring-assignment should not trigger when a dynamic property name is accessed

[ThiefMaster]

Example:

    getChildContext() {
        return {getProp: (prop) => this.props[prop]};
    }

There's no way to change this to use destructuring, so the rule should not apply unless it's this.props.foo or this.props['foo'].

[ljharb]

const { [prop]: foo } = this.props;

But you shouldn’t be dynamically reaching into props; exposing that kind of unfettered prop access to children totally violates the data model of React.

[ThiefMaster]

I agree that in most cases it's not a good idea. In this particular case it's for something like this:

<Translate name={name} link={link}>
    Hey <Param name="name" />, you want to <Param name="link">click me</Param> and you know it!
</Translate>

I think this is a pretty valid case for children accessing parent props.

[ljharb]

I’m not sure that it is; we use string interpolation since translations always eventually need to be statically analyzeable.

In that case, why wouldn’t the user just pass name directly into the Param? What’s the point of the indirection?

[ThiefMaster]

I'm using a custom babel plugin that extracts this piece of JSX to a plain string, which can then be translated without having to deal with JSX (Hey {name}, you want to {link}click me{/link} and you know it!), while still being able to have elements in the translated string that can be wrapped by some tag. (link could be <a onclick={doStuff} /> for example)

Anyway, maybe it's special enough to just disable the rule for that particular line.

---

Actually, now that I'm thinking about it again, I could probably change this to

<Translate>
    Hey <Param name="name" value={name} />, you want to <Param name="link" value={name}>click me</Param> and you know it!
</Translate>

[ljharb]

fwiw, the way we do it is basically

<T
  t="Hey %{name}, you want to do something %{bold_tag_start}bold%{bold_tag_end}?"
  html
  bold_tag={<em />}
/>

which works with our many non-JS systems that also do translation (and with mobile, that doesn't necessarily use HTML to style things)

[ThiefMaster]

Just found a different example during a code review where a developer ended up writing rather ugly code due to the warning (and not just disabling it) - see the XXX lines:

handleFiltersChange(filter, value) {
    const {setTextParamFilter, setAdvancedParamFilter, filters: {text}} = this.props;
    const stateUpdates = {[filter]: value};

    if (filter === 'building') {
        stateUpdates.floor = '';
    } else if (filter === 'text') {
        const parsedValues = parseRoomListFiltersText(value);

        stateUpdates.building = parsedValues.building;
        stateUpdates.floor = parsedValues.floor;
    }

    this.setState(stateUpdates, () => {
        let textValue;

        if (filter === 'text') {
            textValue = value;
        } else {
            const stateToKeys = {building: 'building', floor: 'floor'};
            const textParts = Object.entries(stateToKeys).filter(([stateKey]) => {
                const {[stateKey]: stateValue} = this.state;  // XXX
                return !!stateValue;
            }).map(([stateKey, searchKey]) => {
                const {[stateKey]: stateValue} = this.state;  // XXX
                return `${searchKey}:${stateValue}`;
            });

            textValue = textParts.join(' ');
        }

        this.setState({text: textValue});
        setAdvancedParamFilter('text', textValue);
    });

    if (filter === 'text') {
        if (!text || value.trim() !== text.trim()) {
            setTextParamFilter(value);
        }
    }
}

I think this is a less obscure usecase where destructuring is ugly for dynamic props/state.

[ljharb]

In that case I think the dynamic keys are the ugly part; I think destructuring is a fine way to do it.

That said, it might be ok to have an option that relaxes the requirement when the property is computed from a variable name - I just think the whole problem can be avoided entirely.

[CristianoYL]

Hi guys, I have a similar question. I'm quite new to React, so please correct me if I err.

I'm using React and Redux, and Redux allows me to expose the action creator functions as props inside my React component (using the bindActionCreators() function). I've attached my relevant code below, it's a bit verbose.

The problem is that I'm fine with the destructuring pattern, but since queryVariants is already imported, I shall not declare another prop using this name. Any suggestions? Thank you!

import React, { Component } from 'react';
import PropTypes from 'prop-types';
import { connect } from 'react-redux';
import { bindActionCreators } from 'redux';
import { queryVariants } from '../../actions';

// class-based component
class SearchBar extends Component {

  render() {
    return (
      <div>
           <button 
              onClick={() => { this.props.selectVariant(someData); }} 
           />
      </div>
    );
  }
}

// this function allows queryVariant function show up as a prop
function mapDispatchToProps(dispatch) {
  return bindActionCreators({ queryVariants }, dispatch);
}

export default withStyles(styles)(connect(null, mapDispatchToProps)(SearchBar));

[UPDATE]
I removed some of the irrelavent code just to show the high-level structure of the component. It may be psuedo-code. The error comes from this line: onClick={() => { this.props.selectVariant(someData); }} where selectVariant() is exposed as props by redux.