Not working - Githubissues

cletusw commented 9 years ago

I can't seem to get this to work, and it seems to be because of your use of overloading. According to the docs, overloading is not supported, and union types must be used instead.

faceleg commented 9 years ago

I'm not sure how to convert the example below to use union types:

  css(properties: Object): AngularJSJQueryLite;
  css(propertyName: string): string;
  css(propertyName: string, propertyFunction: AngularJSCallbacks.css): AngularJSJQueryLite;
  css(propertyName: string, value: string): AngularJSJQueryLite;

Would it be, for example:

  css(propertiesOrPropertyName: Object | string, propertyFunctionOrValue: AngularJSCallbacks.css | string): AngularJSJQueryLite | string;
  //css(propertyName: string): string;
  //css(propertyName: string, propertyFunction: AngularJSCallbacks.css): AngularJSJQueryLite;
  //css(propertyName: string, value: string): AngularJSJQueryLite;

cletusw commented 9 years ago

I guess, yeah. Plus the return value would be AngularJSJQueryLite | string. It seems untenable to assume that all use cases can be expressed this way. I wonder why they don't support it. Did they used to? Did these interfaces of yours used to work?

faceleg commented 9 years ago

I agree, it hardly makes the code more readable. I had a lot of trouble getting it to work (on a huge, poorly written codebase, granted). I seem to remember that it was working as I expected, but I gave up due to: facebook/flow/issues/199

faceleg commented 9 years ago

I updated my example with string.

This makes it impossible to know under what circumstances which return value should be expected.

faceleg commented 9 years ago

Our use case is mentioned here: https://github.com/facebook/flow/issues/60#issuecomment-63781216 by @avikchaudhuri

faceleg commented 9 years ago

But then there is this comment https://github.com/facebook/flow/issues/20#issuecomment-63570839 by @gabelevi

cletusw commented 9 years ago

Hmm, that makes it seem like it should work. Maybe I'm doing something wrong on my end.

faceleg commented 9 years ago

What error are you getting?

cletusw commented 9 years ago

I've since moved on to trying to build my own interfaces for other things. Let me see if I can get back to where I was.

Sidenote: Looks like maybe intersection types might work for overloading? I'm a noob at this stuff, so maybe I'm way off track.

faceleg commented 9 years ago

From a fellow noob: I don't know either! I was very happy to "discover the correct" way to construct these interfaces for the overloaded angular functions. My fingers remain crossed in the hope that it is in fact the correct way...

cletusw commented 9 years ago

Assuming I've got things set up correctly, Flow-ing this file:

angular.module('app').directive('myDirective', function (dependencyA, dependencyB) {
  return {
    restrict: 'E',
    scope: {
      myOption: '='
    },
    link: function ($scope, element) {
    }
  };
});

and referencing your interfaces gives this output:

$ flow check --all --strip-root

[LIB] angular.js:4:45,68: function type
Too few arguments (expected default/rest parameters in function)
  main.js:1:48,10:1: function

main.js:1:33,45: string
This type is incompatible with
  [LIB] angular.js:27:19,80: object type

main.js:1:48,10:1: function
This type is incompatible with
  [LIB] angular.js:25:45,96: array type

Found 3 errors

faceleg commented 9 years ago

Looks like: https://github.com/facebook/flow/issues/199

atticoos commented 9 years ago

It looks like some progress may be made? https://github.com/facebook/flow/issues/256

faceleg commented 9 years ago

I've asked the question on facebook/flow#199

faceleg commented 9 years ago

@ajwhite Looks like facebook/flow#199 has been fixed, does this resolve the issue for you?

solnet-aquarium / flow-interfaces-angular

Not working #1