Skip to content

[prop-types] It crash eslint when using spread operator in a flow type #1178

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
lgraziani2712 opened this issue May 7, 2017 · 10 comments
Closed

Comments

@lgraziani2712
Copy link

lgraziani2712 commented May 7, 2017

Hello! This code crashs eslint:

type RawMazeData = {|
  ...MazeData,
  path: Array<[string, ActivePathBorders]>,
|};
Cannot read property 'type' of undefined
TypeError: Cannot read property 'type' of undefined
    at buildTypeAnnotationDeclarationTypes (C:\dev\repos\codimo\node_modules\eslint-plugin-react\lib\rules\prop-types.js:452:25)
    at C:\dev\repos\codimo\node_modules\eslint-plugin-react\lib\rules\prop-types.js:464:54
    at iterateProperties (C:\dev\repos\codimo\node_modules\eslint-plugin-react\lib\rules\prop-types.js:333:11)
    at buildTypeAnnotationDeclarationTypes (C:\dev\repos\codimo\node_modules\eslint-plugin-react\lib\rules\prop-types.js:463:11)
    at buildTypeAnnotationDeclarationTypes (C:\dev\repos\codimo\node_modules\eslint-plugin-react\lib\rules\prop-types.js:455:20)
    at C:\dev\repos\codimo\node_modules\eslint-plugin-react\lib\rules\prop-types.js:464:54
    at iterateProperties (C:\dev\repos\codimo\node_modules\eslint-plugin-react\lib\rules\prop-types.js:333:11)
    at buildTypeAnnotationDeclarationTypes (C:\dev\repos\codimo\node_modules\eslint-plugin-react\lib\rules\prop-types.js:463:11)
    at C:\dev\repos\codimo\node_modules\eslint-plugin-react\lib\rules\prop-types.js:681:38
    at iterateProperties (C:\dev\repos\codimo\node_modules\eslint-plugin-react\lib\rules\prop-types.js:333:11)

Edit: disabling this rules does not happen.

Thank you!

@AndreyKozlov1984
Copy link

AndreyKozlov1984 commented May 7, 2017

I can also say that babel itself could not run that code until the 6.24.1 (or a bit earlier) version of babel-core

@AndreyKozlov1984
Copy link

Wow, so fast! Fantastic!

@AndreyKozlov1984
Copy link

Sadly I still get that issue:

TypeError: Cannot read property 'type' of undefined
    at buildTypeAnnotationDeclarationTypes (/Users/zeus/Documents/redux_demo/webapp/node_modules/eslint-plugin-react/lib/rules/no-unused-prop-types.js:426:25)
    at /Users/zeus/Documents/redux_demo/webapp/node_modules/eslint-plugin-react/lib/rules/no-unused-prop-types.js:681:25
    at iterateProperties (/Users/zeus/Documents/redux_demo/webapp/node_modules/eslint-plugin-react/lib/rules/no-unused-prop-types.js:287:11)
    at markPropTypesAsDeclared (/Users/zeus/Documents/redux_demo/webapp/node_modules/eslint-plugin-react/lib/rules/no-unused-prop-types.js:680:11)
    at markAnnotatedFunctionArgumentsAsDeclared (/Users/zeus/Documents/redux_demo/webapp/node_modules/eslint-plugin-react/lib/rules/no-unused-prop-types.js:814:7)
    at Object.handleStatelessComponent (/Users/zeus/Documents/redux_demo/webapp/node_modules/eslint-plugin-react/lib/rules/no-unused-prop-types.js:823:7)
    at EventEmitter.updatedRuleInstructions.(anonymous function) (/Users/zeus/Documents/redux_demo/webapp/node_modules/eslint-plugin-react/lib/util/Components.js:624:75)
    at emitOne (events.js:101:20)
    at EventEmitter.emit (events.js:189:7)
    at NodeEventGenerator.applySelector (/Users/zeus/Documents/redux_demo/webapp/node_modules/eslint/lib/util/node-event-generator.js:265:26)

@yannickcr
Copy link
Member

Different rule, same issue. I will fix this one too.

@rosskevin
Copy link
Contributor

rosskevin commented Jun 13, 2017

This should be reopened, still a problem with the same stack on version 7.0.1. I'll see if I can add a test PR that demonstrates it, I suspect it is due to the use of $Exact<> or shorthand {||}.

Cannot read property 'type' of undefined
TypeError: Cannot read property 'type' of undefined
    at buildTypeAnnotationDeclarationTypes (/Users/kross/projects/ui/node_modules/eslint-plugin-react/lib/rules/prop-types.js:452:25)
    at /Users/kross/projects/ui/node_modules/eslint-plugin-react/lib/rules/prop-types.js:464:54
    at iterateProperties (/Users/kross/projects/ui/node_modules/eslint-plugin-react/lib/rules/prop-types.js:333:11)
    at buildTypeAnnotationDeclarationTypes (/Users/kross/projects/ui/node_modules/eslint-plugin-react/lib/rules/prop-types.js:463:11)
    at /Users/kross/projects/ui/node_modules/eslint-plugin-react/lib/rules/prop-types.js:686:38
    at iterateProperties (/Users/kross/projects/ui/node_modules/eslint-plugin-react/lib/rules/prop-types.js:333:11)
    at markPropTypesAsDeclared (/Users/kross/projects/ui/node_modules/eslint-plugin-react/lib/rules/prop-types.js:681:11)
    at Object.ClassProperty (/Users/kross/projects/ui/node_modules/eslint-plugin-react/lib/rules/prop-types.js:833:11)
    at Linter.updatedRuleInstructions.(anonymous function) (/Users/kross/projects/ui/node_modules/eslint-plugin-react/lib/util/Components.js:621:75)
    at emitOne (events.js:101:20)
    at Linter.emit (events.js:191:7)
error Command failed with exit code 1.
~/p/ui ❯❯❯ yarn list eslint-plugin-react                                                                                                                                                                                        ⏎ develop ✭ ✱
yarn list v0.24.6
└─ eslint-plugin-react@7.0.1
✨  Done in 1.83s

@rosskevin
Copy link
Contributor

rosskevin commented Jun 13, 2017

I've pulled master, I see {...data}, {|...data|} are working, I added {...$Exact<data>} and it too works, so my hunch was wrong. I'll keep looking.

EDIT: See below, I can't make tests fail, so...

@rosskevin
Copy link
Contributor

I've reproduced, it is the same thing I've found with flow plugins: I am spreading a type imported from a libdef (like flow-typed) and it is unresolved, therefore there are no children from the imported shape:

prop-types_js_-_ui_-____projects_ui__-_webstorm_2017_2_eap

@rosskevin
Copy link
Contributor

rosskevin commented Jun 13, 2017

I've discovered that this problem is not isolated to libdefs, code that is present inside my project is failing to be validated using the object type spread, it appears to be because iterateProperties() makes no attempt to flatten the type, and it calls fn(undefined, undefined) as the key/value.

I think this method will need to check if it is indeed an object type spread and flatten it in-line.

prop-types_js_-ui-__projects_ui-_webstorm_2017_2_eap

Unfortunately, it doesn't seem to matter what I put in the tests, they all pass, leading me to believe there is a problem in the tests. e.g.:

{
      code: [
        'import type {Data} from \'./Data\'',
        'type Person = {',
        '  ...Data,',
        '  lastname: string',
        '};',
        'class Hello extends React.Component {',
        '  props: Person;',
        '  render () {',
        '    return <div>Hello {this.props.bar}</div>;',
        '  }',
        '}'
      ].join('\n'),
      parser: 'babel-eslint'
    }, {
      code: [
        'import type {Data} from \'some-libdef-like-flow-typed-provides\'',
        'type Person = {',
        '  ...Data,',
        '  lastname: string',
        '};',
        'class Hello extends React.Component {',
        '  props: Person;',
        '  render () {',
        '    return <div>Hello {this.props.bar}</div>;',
        '  }',
        '}'
      ].join('\n'),
      parser: 'babel-eslint'
    }

Both report as passing, and I cannot see how that could be true.

If @yannickcr will reopen this and advise me on the bad tests succeeding, I'm willing to take a shot at this.

@rosskevin
Copy link
Contributor

I finally debugged my case close enough to add a test that breaks the current master. It's a nested spread that breaks this code and proves this should be reopened.

{
      code: [
        'import type {BasePerson} from \'./types\'',
        'type Props = {',
        '  person: {',
        '   ...$Exact<BasePerson>',
        '   lastname: string',
        '  }',
        '};',
        'class Hello extends React.Component {',
        '  props: Props;',
        '  render () {',
        '    return <div>Hello {this.props.person.firstname}</div>;',
        '  }',
        '}'
      ].join('\n'),
      parser: 'babel-eslint'
    }

@ljharb
Copy link
Member

ljharb commented Jun 13, 2017

@rosskevin would you open a new issue for that, that links back to this one? The OP is fixed; your issue is slightly different :-)

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