babel-plugin-flow-react-proptypes icon indicating copy to clipboard operation
babel-plugin-flow-react-proptypes copied to clipboard

0.53 changes and weak mode for React.Component

Open rosskevin opened this issue 8 years ago • 6 comments

flow-upgrade effectively turned weak components into

class Foo extends React.Component<$FlowFixMeProps, $FlowFixMeState>

This causes this plugin to fail with:

Did not find type annotation for UnmountTransition
    at annotate (/Users/kross/projects/material-ui/node_modules/babel-plugin-flow-react-proptypes/lib/index.js:89:13)
    at PluginPass.ClassDeclaration (/Users/kross/projects/material-ui/node_modules/babel-plugin-flow-react-proptypes/lib/index.js:202:18)
    at newFn (/Users/kross/projects/material-ui/node_modules/babel-traverse/lib/visitors.js:276:21)
    at NodePath._call (/Users/kross/projects/material-ui/node_modules/babel-traverse/lib/path/context.js:76:18)
    at NodePath.call (/Users/kross/projects/material-ui/node_modules/babel-traverse/lib/path/context.js:48:17)
    at NodePath.visit (/Users/kross/projects/material-ui/node_modules/babel-traverse/lib/path/context.js:105:12)
    at TraversalContext.visitQueue (/Users/kross/projects/material-ui/node_modules/babel-traverse/lib/context.js:150:16)
    at TraversalContext.visitMultiple (/Users/kross/projects/material-ui/node_modules/babel-traverse/lib/context.js:103:17)
    at TraversalContext.visit (/Users/kross/projects/material-ui/node_modules/babel-traverse/lib/context.js:190:19)
    at Function.traverse.node (/Users/kross/projects/material-ui/node_modules/babel-traverse/lib/index.js:114:17)

I'm not sure what to do, but we need to ignore suppressed annotated types. Removing the <$FlowFixMeProps, $FlowFixMeState> leads to a slew of other errors requiring $FlowFixMe suppressions.

rosskevin avatar Aug 23 '17 00:08 rosskevin

BTW, weak mode still works, but the built-in react libdef requires the type parameters, so it is a little stricter than before.

rosskevin avatar Aug 23 '17 00:08 rosskevin

This can be worked around with extends React.Component<any, any>, so is this just new information or do we need to change anything in the plugin?

rosskevin avatar Aug 23 '17 00:08 rosskevin

Should we test for the type identifier starting with $FlowFixMe and interpret it as any?

brigand avatar Aug 23 '17 00:08 brigand

I think that is a good idea. It's not foolproof but it's going to work for the majority.

rosskevin avatar Aug 23 '17 00:08 rosskevin

Should we test for the type identifier starting with $FlowFixMe and interpret it as any?

Maybe it should do this for all suppress_types in the .flowconfig?

robwise avatar Sep 21 '17 11:09 robwise

I'd rather not be reading files within a babel plugin. We could expose an option mapping types to any in the babel config. Open to having my mind changed.

brigand avatar Sep 21 '17 11:09 brigand