ryyppy / flow-guide

The definitive guide for using Flow static JavaScript type checker (https://flowtype.org)
MIT License
483 stars 21 forks source link

Covariance and Contravariance #6

Open ryyppy opened 8 years ago

ryyppy commented 8 years ago

https://twitter.com/ForbesLindesay/status/732610644357337088

SamMatthewsIsACommonName commented 7 years ago

First up thanks a lot for the great guide, it really helped me get up and going. Also I found the redux todos-flow example a great reference as redux and flow is somewhat challenging at first.

I ran into this issue when trying to bind actions in the constructor like this:

constructor(props) { super(props); this.handleNavigate = this.handleNavigate.bind(this); this.toggleMenu = this.toggleMenu.bind(this); }

Which gives me the error:

propertyhandleNavigate: .../headerContainer.js:32 Covariant propertyhandleNavigateincompatible with contravariant use in assignment of propertyhandleNavigate``

ryyppy commented 7 years ago

Hmmm, I never had this issue before... can you post a complete gist or something?

SamMatthewsIsACommonName commented 7 years ago

Thanks for getting back. Yeah sure: https://gist.github.com/sammy6464/cf089669eac1507fa1da2639eab1f47b Which gives the flow error above ^^

ryyppy commented 7 years ago

Okay, I will need to set up a project to test this... not sure when I have time, but I will eventually get back to you ;-)

SamMatthewsIsACommonName commented 7 years ago

OK great thanks a lot! To save you time and headaches I've stripped everything down to it's bare essence, it's just a class that takes no inputs and returns a div, but the error remains. It must be purely due to the constructor / function syntax? Thanks again

https://gist.github.com/sammy6464/e7e74cff566a99e3e68e070f7861e7ad

ryyppy commented 7 years ago

Okay, now I get it... The newest version shows a different error message which I was not familiar with... (probably because of the newest co/contravariance changes)

This is my modified version of making it check:


import React from 'react';

class HeaderContainer extends React.Component {

  handleNavigate: Function;
  toggleMenu: Function;

  constructor(props: Object) {
    super(props);
    this.handleNavigate = this.handleNavigate.bind(this);
    this.toggleMenu = this.toggleMenu.bind(this);
  }

  handleNavigate() {
    console.log('hey');
  }

  toggleMenu() {
    console.log('ho');
  }

  render() {
    return (
      <div />
    );
  }
}

export default HeaderContainer;

You need to add class properties with the proper Function definition... I know that's tedious, so I usually just write Function as a type.

Hope that helps

SamMatthewsIsACommonName commented 7 years ago

Aah great that's it thanks a lot for helping clear that up!

skywalkerlw commented 7 years ago

@ryyppy you have saved my life :) it's strange to have to write that, i hope in future version of flow, this kind of warning could be removed :)

juanibiapina commented 7 years ago

You can also declare the actual type of the function, which helps catch errors in more complex cases. This issue was helpful though.

Nopzen commented 7 years ago

@ryyppy - Great solution, do you know how i could make this compatible with standard js now im getting: Standard code style functionNameis not defined (no-undef) ?

B-Reif commented 7 years ago

Just wanted to chime in that I found the covariance error messaging to be pretty confusing.

fabiomcosta commented 6 years ago

Be careful with the suggested solution, the method type in this case is not properly "forwarded".

See this example:

class X {
  x: Function;
  constructor() {
    super(...arguments);
    this.x = this.x.bind(this);
  }
  x(a: string): number {
    return 3;
  }
}

new X().x(5); // no errors, but should!

https://flow.org/try/#0MYGwhgzhAEAa0G8BQ1oA8Bc0BiBXAdsAC4CWA9vgNwrTAUREBOuxZjAFAJSI2oS4AHAKYcAdOLCMA5rgC2Q-EQidqqVEQAWJCKLTQAvNE3bdogEYl8AE3bHlq6AF8aadmCwNGlqZyz45ZiI8atCMQkS4jPjQAMwOzs5I+EIA7nBcuuwArCrQAPR50Phk0CKMbBAANNBmuETQEBpkuCBWAIRAA

If you want types to be properly handled, you can use the not yet standardized class property (https://babeljs.io/docs/plugins/transform-class-properties/):

class X {
  x = (a: string): number => {
    return 3;
  }
}

new X().x(5); // properly throws an error

https://flow.org/try/#0MYGwhgzhAEAa0G8BQ1oA9oF5oAowC5oIAXAJwEsA7AcwEpDKBXAWwCMBTUrAPkRVWil2xRqUrQAzAG5+AXyTykldgHc4OWgDo0OAKy0p0APRHoAB1IB7M5xABPaMQAWVlTDDjOV0kA