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

[React] Flow loses the inferred type of imported functions when using typeof inside Component<Props> #6797

Closed
ggregoire opened this issue Aug 24, 2018 · 12 comments

Comments

@ggregoire
Copy link

ggregoire commented Aug 24, 2018

Edit: last update on this: it appears only when 'action' is imported.

--

I don't know if I'm doing something wrong or if it's a bug, but it looks pretty straightforward:

import { action } from 'somewhere'

// action: (param1: string) => {| type: string, param1: string |}

const actions = {
  action
}

type Props = typeof actions

class MyComponent extends Component<Props> {
  componentDidMount() {
    this.props.action(1, 2, 3) // No errors when I run 'flow check'!
  }
}

Flow correctly infers the type of the function outside of the component. But, once it is passed to the component (this.props.action), it becomes any.

I also tried, without success:

type Actions = typeof actions

type Props = Actions
type Props = {} & Actions
type Props = { ...Actions }
type Props = {| ...Actions |}
@ghost
Copy link

ghost commented Aug 25, 2018

Seems like flow is marking action as any. If you are importing action from a third-party module, in order to have flow mark it the correct type make sure you have the libdef for the module. Here is the link to the docs that provide instruction on how to that.
If you are importing action from a file within your project make sure that the file has flow initialized.

@ggregoire
Copy link
Author

ggregoire commented Aug 27, 2018

Maybe it was not clear, so here is another example with some comments:

// index.js
// @flow

// Let's import a function I've written in my project, with the '// @flow' annotation:

import { action } from '../state/actions'
// action: (param1: string) => {| type: string, param1: string |}
// This is the type inferred by Flow. I can see it in my editor.

// Now let's call it with some unexpected params:

action(1, 2, 3)
// Error 1: Cannot call `action` with `1` bound to `param1` because number [1] is incompatible with string [2]
// Error 2: Cannot call `action` because no more than 1 argument is expected by function [1]
// In this first case, Flow is catching all the errors as expected.

// Now let's pass 'action' to a component through its props:

const mapActionsToProps = {
  action
}

type Actions = typeof mapActionsToProps

type Props = {|
  ...Actions,
  otherProp: string, 
|}

class MyComponent extends Component<Props> {
  componentDidMount() {
    this.props.action(1, 2, 3)
    // In this case, no errors! Flow is not catching anything.
    // Yet it's the exact same function, passed through the Props
  }
}

connect(null, mapActionsToProps)(MyComponent)

So, either I'm not typing the Props correctly, either there is a bug somewhere.

@AlicanC
Copy link
Contributor

AlicanC commented Aug 27, 2018

I'd try doing:

type Props = {|
  ...$Exact<Actions>,
  otherProp: string, 
|}

@ggregoire
Copy link
Author

@AlicanC Just tried and it doesn't fix it. Actually

const mapActionsToProps = {
  action
}

type Actions = typeof mapActionsToProps

already defines Actions as an exact object.

@ggregoire
Copy link
Author

ggregoire commented Aug 28, 2018

So I just discovered that it's actually because action is imported.

I was trying to reproduce the bug on flow.org/try but it works as expected when it is in the same file: https://flow.org/try/#0JYWwDg9gTgLgBAJQKYEMDGMA0cDecDCE4EAdkifAL5wBmURcA5FKhowFDtqkDO86MYKTgBeOAAowKKChABGAFxw+UYCQDmASlEA+CXhgBPMEiWMAgvgAqASQDyAOUbYpM+XEqbOAoSXFzsACZsAGYvdiMTOAAFejAeUVwAHzgAOnTzDF8EpMpONAAbFB4EgDEICDgkAA8YcgATBMJiMgoAHliIeL0cdjg4bhbyGAARYHqAWQgAVwpxbV7+-pgAC2AeVLA4jZ9SfyDQr368vK5efizeRMW4XZIlh8en55el9lPIpDhMwSuxT4gNFulxIPCAA

I also confirm that the type of the imported action is correctly inferred out of the component's props:

import { action } from '../state/actions'
// action: (param1: string) => {| type: string, param1: string |}

const mapActionsToProps = {
  action
}

type Actions = typeof mapActionsToProps

type Props = {| ...Actions |}

class MyComponent extends Component<Props> {
  componentDidMount() {
    action(1, 2, 3) // Error 1: Cannot call `action` with `1` bound to `param1` because number [1] is incompatible with string [2]
                    // Error 2: Cannot call `action` because no more than 1 argument is expected by function [1]

    this.props.action(1, 2, 3) // No errors!
  }
}

And if in the same file, it works as expected:

const action = (param1: string) => ({ type: 'ACTION', param1 })
// action: (param1: string) => {| type: string, param1: string |}

const mapActionsToProps = {
  action
}

type Actions = typeof mapActionsToProps

type Props = {| ...Actions |}

class MyComponent extends Component<Props> {
  componentDidMount() {
    action(1, 2, 3) // Error 1: Cannot call `action` with `1` bound to `param1` because number [1] is incompatible with string [2]
                    // Error 2: Cannot call `action` because no more than 1 argument is expected by function [1]

    this.props.action(1, 2, 3) // Error 1: Cannot call `action` with `1` bound to `param1` because number [1] is incompatible with string [2]
                               // Error 2: Cannot call `action` because no more than 1 argument is expected by function [1]
  }
}

So there is a bug when combining import, typeof (?) and Component<Props>.

@ggregoire ggregoire changed the title React Component<Props>: if Props = typeof object_of_functions, then the functions type is 'any' [React] Flow loses the inferred type of imported functions when inside Component<Props> Aug 28, 2018
@ggregoire ggregoire changed the title [React] Flow loses the inferred type of imported functions when inside Component<Props> [React] Flow loses the inferred type of imported functions when using typeof inside Component<Props> Aug 28, 2018
@kangax
Copy link

kangax commented Oct 16, 2018

@ggregoire have you found a workaround for this or what did you end up doing?

@ggregoire
Copy link
Author

ggregoire commented Oct 16, 2018

@kangax No I didn't find any workaround. I write the type of every action that I pass to my components…

type Props = {|
  action: (param1: string) => {| type: string, param1: string |},
  action2: () => void,
  // and so on…
|}

class MyComponent extends Component<Props> { ... }

@kangax
Copy link

kangax commented Oct 16, 2018

@ggregoire I'm seeing a similar issue where props lose their types:

The shape seems to be determined properly:

screen shot 2018-10-16 at 12 55 42

But the 2nd level props are any:

screen shot 2018-10-16 at 12 53 05

I also checked with type-at-pos and it's consistent with what I'm seeing in VSCode:

➜  git:(master) ✗ flow type-at-pos app/scripts/xxx.jsx 10 27
{byUuid: {[string]: Array<Ticket>}, error: mixed, loaded: boolean, loading: boolean}

➜  git:(master) ✗ flow type-at-pos app/scripts/xxx.jsx 10 37
any

It's pretty bizarre how Flow recognizes this shape correctly but then reports property's value in the same expression as any. Perhaps some kind of internal shape caching?

@mroch any hints on what this could be?

@ggregoire
Copy link
Author

ggregoire commented Oct 16, 2018

@kangax I don't know if this is related to my issue. In your screenshot, zendeskTickets is not a "prop", it's just a nested object inside another object that you manually typed (state: GlobalState).

If you remove ...ReduxProps from Props, is state.zendeskTickets.loader still inferred as any? If yes then it's definitely not related to my issue.

@kangax
Copy link

kangax commented Oct 16, 2018

@ggregoire I felt it might be the same issue because:

  • if I remove connect, loaded is determined correctly
  • if I remove spreading (type Props = ReduxProps) loaded is determined correctly

In my case, however, importing GlobalState or defining it locally doesn't make a difference.

@MartinCerny-awin
Copy link

I think that this bug is similar to issues #7071 and #4312

@SamChou19815
Copy link
Contributor

These import related issues should be resolved a long time ago since types first

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

No branches or pull requests

5 participants