Skip to content

Partial rewrite of the destructuring rule #1740

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

Closed
wants to merge 8 commits into from
Closed

Partial rewrite of the destructuring rule #1740

wants to merge 8 commits into from

Conversation

HenriBeck
Copy link

@HenriBeck HenriBeck commented Mar 23, 2018

Pull Request for #1709

This introduces an object as an option for specifying different behavior for different component types.

I also added a check if we are in a render method for class components (don't know if this is wanted but the docs only showed examples with render methods).

What is still missing:

  • Checking for destructuring props inside the functional component body
  • Update tests
  • Update Docs
  • Add an option for allowing the props inside class properties (Future PR)
  • Add an option for also validating class methods (Future PR)

This also fixes a few bugs:

Fixes when the props aren't named props:

function Baz(properties) {
  return (<div>{properties.children}</div>); // Didn' report here before
}

@HenriBeck
Copy link
Author

@ljharb could you take a look?

@HenriBeck HenriBeck closed this Jul 14, 2018
@ljharb
Copy link
Member

ljharb commented Jul 14, 2018

@HenriBeck i know it's been awhile; but did you have to delete the fork? This still seems useful, and I'd prefer not to have to have a different PR.

@HenriBeck
Copy link
Author

@ljharb yeah, I just cleaned up my forks and forgot that I still had a PR open here.
I can recreate it when I have time, you can still see the changes so it shouldn't be too big of an effort.

@HenriBeck
Copy link
Author

I also have the changes on my laptop.

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

Successfully merging this pull request may close these issues.

2 participants