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

Props inside async functions are not looked up #1583

Closed
haikyuu opened this issue Dec 4, 2017 · 12 comments
Closed

Props inside async functions are not looked up #1583

haikyuu opened this issue Dec 4, 2017 · 12 comments

Comments

@haikyuu
Copy link

haikyuu commented Dec 4, 2017

I am having this issue in async functions

class Comp extends Component{
  method1 = async () =>{
    const { propNotInPropTypes } = this.props
  }
  ...
}

This does not show warning missing in props validation. But when i remove async, it works as expected.

@jseminck
Copy link
Contributor

jseminck commented Dec 6, 2017

Duplicate of #1473

I briefly looked into it and it seems it is related to destructuring within async functions that are defined as class properties.

This seems to work:

async componentDidMount() {
  const { propIsDetected } = this.props
}

async componentDidMount() {
  console.log(this.props.propIsDetected)
}

myMethod = async () => {
  console.log(this.props.propIsDetected)
}

But indeed, the example from the OP is failing:

myMethod = async () => {
  const { propIsNOTDetected} = this.props
}

@jseminck
Copy link
Contributor

jseminck commented Dec 6, 2017

Upon further digging, the issue is because myMethod = async () => { } is detected as a component in the Components util, because it is ArrowFunctionExpression + async === true:

https://github.com/yannickcr/eslint-plugin-react/blob/master/lib/util/Components.js#L650

Therefore, the propIsNOTDetected is registered as a prop on that incorrectly detected ArrowFunctionExpression.

Not sure what's the proper fix for this. Removing this logic seems scary, but it only fails one test inside prop-types itself (rest of the suite passes):

var Hello = async (props) => {
  return <div>Hello {props.name}</div>;
}:

Maybe this logic was added as a quick fix to make this specific case work... not sure...

@ljharb
Copy link
Member

ljharb commented Dec 6, 2017

I think arrow function class properties should be categorized as non-components.

@Wildhoney
Copy link

Wildhoney commented Dec 21, 2017

I have a related issue.

In my propTypes I have:

static propTypes = {
    onToggleShortlist: PropTypes.func.isRequired
};

Which is then used in an async method:

async handleToggleShortlist(isShortlisted) {
    // ...
    const type = // ...
    await this.props.onToggleShortlist(type, this.props.model.location.locationId);
    // ...
}

However when running ESLint I get: onToggleShortlist PropType is defined but prop is never used react/no-unused-prop-types.

Without async it finds the prop perfectly fine.

Where package.json is:

"babel-eslint": "~7.2.3",
"eslint-config-xo-react": "~0.12.0",
"eslint-plugin-react": "~7.1.0"

@Wildhoney
Copy link

Wildhoney commented Dec 21, 2017

Updated and all seems to be okay now.

"babel-eslint": "~8.0.3",
"eslint-config-xo-react": "~0.15.0",
"eslint-plugin-react": "~7.5.1"

@ljharb
Copy link
Member

ljharb commented Dec 21, 2017

@haikyuu please let me know if this is still a problem, and i'll reopen

@ljharb ljharb closed this as completed Dec 21, 2017
@trisys3
Copy link

trisys3 commented Jul 13, 2018

I'm still having this issue in an async class method. It's a normal method, e.g.

class MyClass extends PureComponent {
    async myMethod() {
        const {undetectedMethod} = this.props;
    }
}

@ljharb
Copy link
Member

ljharb commented Jul 14, 2018

@trisys3 that class doesn't extend React.Component or React.PureComponent, so I'm not sure why it'd warn.

@trisys3
Copy link

trisys3 commented Jul 14, 2018

Sorry, forgot about that. It extends PureComponent.

It was just an example, anyway. I can try to see if it actually warns, otherwise I can experiment to see how much is actually needed for breakage.

@ljharb
Copy link
Member

ljharb commented Jul 14, 2018

If you could file a new issue, with the full component code, installed eslint* versions, and eslint config, that’d be great.

@trisys3
Copy link

trisys3 commented Jul 14, 2018

OK. I'll try to get a simplified test case that doesn't work.

@trisys3
Copy link

trisys3 commented Jul 15, 2018

I have a test case, and will put it on GitHub.

From what I can determine, the problem comes from a combination of destructuring and async functions. You need to destructure this.props inside of an async function; make the function synchronous and you're warned; don't destructure and you're warned. Hopefully my example repo will explain better.

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

No branches or pull requests

5 participants