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

Prevent non-defined props from getting set in connect? #888

Closed
dandean opened this issue Oct 5, 2016 · 7 comments
Closed

Prevent non-defined props from getting set in connect? #888

dandean opened this issue Oct 5, 2016 · 7 comments

Comments

@dandean
Copy link

dandean commented Oct 5, 2016

Given this basic example:

import React from 'react';
import { connect } from 'react-redux';

class Foo extends React.Component {
  static propTypes = {
    foo: React.PropTypes.string
  };
}

export default connect((state) => {
  return {
    // Legit prop is set:
    foo: state.foo,

    // Non-existent prop is set. I want a lint error here:
    bar: state.bar
  };
})(Foo);

Is there a way to get eslint to report the invalid bar prop as an error?

@xjamundx
Copy link
Contributor

xjamundx commented Oct 5, 2016

I think this is a good idea

@ljharb
Copy link
Member

ljharb commented Oct 5, 2016

I'm not sure I understand. How could anyone statically determine that in this IIFE that returns an object, the object is meant to represent props?

@xjamundx
Copy link
Contributor

xjamundx commented Oct 5, 2016

because of this line import { connect } from 'react-redux' and also the connect()

@ljharb
Copy link
Member

ljharb commented Oct 5, 2016

That's some really really specific deduction for one single library. That seems like something that some sort of eslint-config-redux would handle, but not generic React.

I'm particularly concerned that usage of https://www.npmjs.com/package/react-with-styles would trigger this, short of the very specific import detection.

@xjamundx
Copy link
Contributor

xjamundx commented Oct 5, 2016

@ljharb I'd argue redux is pretty huge https://www.npmjs.com/package/redux, but you're right a redux specific plugin may make the most sense. Wondering how it would work with the existing propTypes validation if it was separate though 🤔

@lencioni
Copy link
Collaborator

lencioni commented Oct 5, 2016

Agreed, this seems like a useful rule but it would be best for a redux or react-redux specific eslint plugin.

Can you elaborate on your concerns about existing propTypes validation?

@dandean
Copy link
Author

dandean commented Oct 5, 2016

Thanks for the discussion, y'all. At the very least, it sounds like this is the wrong repository for this rule, so I'm going to close the issue. Please feel free to keep commenting on the issue – it's all really valuable input from my perspective :)

@dandean dandean closed this as completed Oct 5, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

No branches or pull requests

4 participants