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

Flow choosing incorrect overload. #2891

Closed
JohnLouderback opened this issue Nov 24, 2016 · 3 comments
Closed

Flow choosing incorrect overload. #2891

JohnLouderback opened this issue Nov 24, 2016 · 3 comments

Comments

@JohnLouderback
Copy link

Given this code defined in a library:

  /**
   * Get the value of style properties for the first element in the set of matched elements.
   *
   * @param propertyName A CSS property.
   */
  css(propertyName: string): string;
  /**
   * Set one or more CSS properties for the set of matched elements.
   *
   * @param propertyName A CSS property name.
   * @param value A value to set for the property.
   */
  css(propertyName: string, value: string | number): JQuery;
  /**
   * Set one or more CSS properties for the set of matched elements.
   *
   * @param propertyName A CSS property name.
   * @param value A function returning the value to set. this is the current element. Receives the index position of the element in the set and the old value as arguments.
   */
  css(propertyName: string, value: (index: number, value: string) => string | number): JQuery;
  /**
   * Set one or more CSS properties for the set of matched elements.
   *
   * @param properties An object of property-value pairs to set.
   */
  css(properties: Object): JQuery;

And this code leveraging that method:

$this.css('max-height', folderClosedMaxHeight + 'px')
     .addClass('is-folder')
     .after('<div class="show-more">Show More</div>');

I get the error: property addClass Property not found in String

capturfiles-24-33-2016_01 33 49

Since there is two arguments being passed, there's no good reason it should choose an overload that only has one parameter.

@gcanti
Copy link

gcanti commented Nov 25, 2016

You can either

  1. sort the overloadings by the number of arguments desc

Bad

declare function f(a: string): number;
declare function f(a: string, b: string): string

f('a', 'b').trim() // <= error property `trim`. Property not found in Number

Good

declare function f(a: string, b: string): string
declare function f(a: string): number;

f('a', 'b').trim() // <= ok
  1. add a fake rest argument
declare function f(a: string, ...rest: Array<void>): number;
declare function f(a: string, b: string, ...rest: Array<void>): string

f('a', 'b').trim() // <= ok

@JohnLouderback
Copy link
Author

Option 1 seems to do the trick. I understand why Flow does this (kind of), but wouldn't it make more sense to need to explicitly define rest parameters rather than "block" them as above? It seems to defeat the purpose of strict typing and other specifications.

@SamChou19815
Copy link
Contributor

The issue seemed to be fixed a long time ago.

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

No branches or pull requests

4 participants