Skip to content
This repository has been archived by the owner on Aug 19, 2022. It is now read-only.

Flow errors #781

Closed
ffxsam opened this issue Jul 14, 2016 · 11 comments
Closed

Flow errors #781

ffxsam opened this issue Jul 14, 2016 · 11 comments

Comments

@ffxsam
Copy link

ffxsam commented Jul 14, 2016

I see that Flow annotations were added (recently?). Unfortunately, they don't pass. Here's the output I'm getting when I run npm run flow in my project which uses Radium:

node_modules/radium/src/components/style-root.js:16
 16:     instance._radiumStyleKeeper = new StyleKeeper(userAgent);
                  ^^^^^^^^^^^^^^^^^^ property `_radiumStyleKeeper`. Property not found in
 16:     instance._radiumStyleKeeper = new StyleKeeper(userAgent);
         ^^^^^^^^ StyleRoot

node_modules/radium/src/components/style-sheet.js:8
  8:   static contextTypes = {
       ^ Experimental class static field usage. Class static fields are an active early stage feature proposal that may change. You may opt-in to using them anyway in Flow by putting `esproposal.class_static_fields=enable` into the [options] section of your .flowconfig.

node_modules/radium/src/components/style-sheet.js:15
 15:     this.state = this._getCSSState();
                      ^^^^^^^^^^^^^^^^^^^ object type. This type is incompatible with
  7: export default class StyleSheet extends Component {
                                             ^^^^^^^^^ undefined. Did you forget to declare type parameter `State` of identifier `Component`?

node_modules/radium/src/components/style-sheet.js:17
 17:     this._onChange = this._onChange.bind(this);
              ^^^^^^^^^ property `_onChange`. Property not found in
  7: export default class StyleSheet extends Component {
                          ^^^^^^^^^^ StyleSheet

node_modules/radium/src/components/style-sheet.js:21
 21:     this._isMounted = true;
              ^^^^^^^^^^ property `_isMounted`. Property not found in
  7: export default class StyleSheet extends Component {
                          ^^^^^^^^^^ StyleSheet

node_modules/radium/src/components/style-sheet.js:22
 22:     this._subscription = this.context._radiumStyleKeeper.subscribe(
              ^^^^^^^^^^^^^ property `_subscription`. Property not found in
  7: export default class StyleSheet extends Component {
                          ^^^^^^^^^^ StyleSheet

node_modules/radium/src/components/style-sheet.js:29
 29:     this._isMounted = false;
              ^^^^^^^^^^ property `_isMounted`. Property not found in
  7: export default class StyleSheet extends Component {
                          ^^^^^^^^^^ StyleSheet

node_modules/radium/src/components/style-sheet.js:41
 41:       this._isMounted && this.setState(this._getCSSState());
                              ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ call of method `setState`
 41:       this._isMounted && this.setState(this._getCSSState());
                                            ^^^^^^^^^^^^^^^^^^^ property `css` of object type. Property cannot be assigned on possibly undefined value
  7: export default class StyleSheet extends Component {
                                             ^^^^^^^^^ undefined. Did you forget to declare type parameter `State` of identifier `Component`?

node_modules/radium/src/components/style-sheet.js:45
 45:   render(): ReactElement {
                 ^^^^^^^^^^^^ identifier `ReactElement`. Could not resolve name

node_modules/radium/src/components/style-sheet.js:47
 47:       <style dangerouslySetInnerHTML={{__html: this.state.css}} />
                                                               ^^^ property `css`. Property cannot be accessed on possibly undefined value
 47:       <style dangerouslySetInnerHTML={{__html: this.state.css}} />
                                                    ^^^^^^^^^^ undefined. Did you forget to declare type parameter `State` of identifier `Component`?

node_modules/radium/src/components/style.js:79
 79:   render(): ?ReactElement {
                  ^^^^^^^^^^^^ identifier `ReactElement`. Could not resolve name

node_modules/radium/src/enhancer.js:63
 63:     static _isRadiumEnhanced = true;
         ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Experimental class static field usage. Class static fields are an active early stage feature proposal that may change. You may opt-in to using them anyway in Flow by putting `esproposal.class_static_fields=enable` into the [options] section of your .flowconfig.


Found 12 errors
@ianobermiller
Copy link
Contributor

You should ignore node_modules in your .flowconfig, and consider using the flow-typed definitions for Radium: https://github.com/flowtype/flow-typed/blob/master/definitions/npm/radium_v0.17.x/flow_%3E%3Dv0.23.x/radium_v0.17.x.js

@ffxsam
Copy link
Author

ffxsam commented Jul 15, 2016

No, ignoring the entire node_modules is a bad idea. It can lead to errors.

facebook/flow#869

@ianobermiller
Copy link
Contributor

No, it is a very good idea, because there is no guarantee they are using the same version of flow as you, or, as you are seeing, the same flowconfig as you. You should be using flow-typed or create declaration files to get coverage. If you want, you can also just ignore node_modules/radium, as in facebook/flow#869

@ffxsam
Copy link
Author

ffxsam commented Jul 15, 2016

I'll clarify a couple points:

Putting node_modules/.* in .flowconfig is not a good idea. It results in several "required module not found" errors when you run flow. On a similar note, it's also not a good idea to ignore node_modules/radium/.* either, because it will result in the same error for Radium.

The workaround is to ignore node_modules/radium/src/.*.

However, what I don't understand is why npm run flow passes fine if I run it in the Radium repo, but in my project, Flow chokes on the Radium annotations I pasted above. I do know how to resolve some of these, for example:

node_modules/radium/src/components/style-sheet.js:15
 15:     this.state = this._getCSSState();
                      ^^^^^^^^^^^^^^^^^^^ object type. This type is incompatible with
  7: export default class StyleSheet extends Component {
                                             ^^^^^^^^^ undefined. Did you forget to declare type parameter `State` of identifier `Component`?

This would fix it:

export default class StyleSheet extends Component {
  static contextTypes = {
    _radiumStyleKeeper: React.PropTypes.instanceOf(StyleKeeper)
  };

  state: {
    css: string
  };

Again, I don't know why the Radium repo passes all Flow checks. I'm not experienced enough with Flow to know what's going on, and why my project is being pickier. There's nothing too different in Radium's .flowconfig compared to mine.

@ianobermiller
Copy link
Contributor

@alexlande was looking at upgrading Radium to the latest flow version which might help.

Anyway, I can tell you for certain (I work at Facebook and talk frequently with the Flow team) that ignoring node_modules is the recommended route, for exactly the reason you are running into.

@ffxsam
Copy link
Author

ffxsam commented Jul 15, 2016

Anyway, I can tell you for certain (I work at Facebook and talk frequently with the Flow team) that ignoring node_modules is the recommended route, for exactly the reason you are running into.

Oh! Well I appreciate the insight :) Not to go off-topic, but how do I prevent all these errors from popping up when I ignore the node_modules folder?

imports/features/UserProfile/components/UserProfile.js:12
 12: } from 'material-ui';
            ^^^^^^^^^^^^^ material-ui. Required module not found

imports/features/UserProfile/components/UserProfile.js:13
 13: import Formous from 'formous';
                         ^^^^^^^^^ formous. Required module not found

imports/features/UserProfile/components/user-profile-form.js:3
  3: import validator from 'validator';
                           ^^^^^^^^^^^ validator. Required module not found

imports/helpers.js:3
  3: import invariant from 'invariant';
                           ^^^^^^^^^^^ invariant. Required module not found

@ianobermiller
Copy link
Contributor

You need to either create declaration files for the modules or import them from flow-typed. Basically you add type files that are completely separate from the actual modules.

@ffxsam
Copy link
Author

ffxsam commented Jul 15, 2016

Ok, looks like I have some reading up to do. Thanks for the info.

@sepo-one
Copy link

what i'm thinking why not to update flowtype to mind order of include/ignore.

for example we have around 200 node modules, where only about ~10 - 15 of them
have to be used by flow.

so this would work perfectly fine:

[ignore]
.*/node_modules/.*

[include]
.*/node_modules/react
.*/node_modules/redux-react
...

possible ?

@marianban
Copy link

I tried the following but seems that it doesn't work:

[ignore]
<PROJECT_ROOT>/node_modules/.*

[include]
<PROJECT_ROOT>/node_modules/axios

I'm still receiving axios. Required module not found

@kresli
Copy link

kresli commented Oct 5, 2017

@marianban

I think you need to add flow file to lib.

[libs]
node_modules/mobx/lib/mobx.js.flow

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

No branches or pull requests

5 participants