Fix require-default-props rule when using Flow type from assignment

[wyze]

This fixes the issue I reported in #1017.

The condition I added is hit when code like the following is used:

type Props = Message

This just takes the right side of the annotation (Message) and tries to find that variable in scope and then continues on.

[ljharb]

Seems legit

[EvHaus]

Could you also add a failing test, such as in the event that Flow type is used but the variable name can't be found?

[wyze]

Hey @EvNaverniouk, can you check the failing case I added? Seems a bit off as I needed to provide errors: [] because it is under invalid key in the test suite, but it doesn't actually output any errors.

[EvHaus]

We should update this to return errors so we can test for it, perhaps in the event that the type isn't defined. Maybe something like this:

code: [
   'function Hello (props: UndefinedVariable) {',
   '    return <div>Hello {props.name.firstname}</div>;'
   '}'
]

[wyze]

When you say "update this", are you referring to the rule, to throw an error if it can't find a variable? If so, I think that should be a separate PR.

If you are referring to the test case when you say "update this", then when making the requested changes, the invalid test still passes with errors: [] so I am not sure there is much difference between what you are suggesting and what I added.

[EvHaus]

Perhaps I'm misunderstanding what the scope of the changes in this PR. I was under the impression that it adds support for Flow types as valid prop assignments via variables. Your original test case covers the fact the the rule properly catches such valid Flow type assignments without error.

What I'm asking for is to add a test case that when a Flow type is defined, but the Flow type is variable is unassigned. I believe this ESLint rule should still throw an error in that case.

[ljharb]

That's covered by the no-undef rules, is it not?

[yannickcr]

Yes, this should be covered by no-undef. The require-default-props rule should just try its bests to resolve the props and the defaultProps, and give up if it cannot do it.

If this test case do not return any error then it should be in the "valid" array.

[yannickcr]

Yes, this should be covered by no-undef. The require-default-props rule should just try its bests to resolve the props and the defaultProps, and give up if it cannot do it.

If this test case do not return any error then it should be in the "valid" array.

[yannickcr]

You should also update the next line to check for annotation.properties existence, or the rule will keep crashing if there is multiple reassignments

properties = annotation && annotation.properties ? annotation.properties : [];

[ljharb]

What would be a test case that would trigger that?

[wyze]

Well, technically, my fix prevented this, but if for some reason you do multiple assignments, it would trigger the original error again:

import type ImportedProps from "fake";
type NestedProps = ImportedProps;
type Props = NestedProps;
function Hello(props: Props) {
  return <div>Hello {props.name.firstname}</div>;
}

[cr0cK]

up!