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

Unable to call map on array of variable depth #35045

Closed
danvk opened this issue Nov 11, 2019 · 6 comments
Closed

Unable to call map on array of variable depth #35045

danvk opened this issue Nov 11, 2019 · 6 comments
Labels
In Discussion Not yet reached consensus Suggestion An idea for TypeScript

Comments

@danvk
Copy link
Contributor

danvk commented Nov 11, 2019

I'm trying to write a function which transforms the coordinates of any type of GeoJSON geometry. The coordinates of a Geometry can be either number[] (a Point), number[][] (a LineString), number[][][] (a Polygon) or number[][][][] (a MultiPolygon). I have a recursive implementation which works but does not type check.

TypeScript Version: 3.7.2

Search Terms:

  • 2349

Code

type Position = number[];

function isNestedArray<T extends any[]>(x: T | T[]): x is T[] {
  return Array.isArray(x[0]);
}

export function transformCoordArray(
  coords: Position | Position[] | Position[][],
  fn: (pt: Position) => Position
): any[] {
  if (isNestedArray(coords)) {
    coords;  // type is Position[] | Position[][]
    return coords.map(c => transformCoordArray(c, fn));
    //            ~~~ This expression is not callable.
  } else {
    coords;  // type is Position
    return fn(coords);
  }
}

(playground)

The full error message is:

This expression is not callable.
  Each member of the union type '
   (<U>(callbackfn: (value: Position, index: number, array: Position[]) => U, thisArg?: any) => U[])
 | (<U>(callbackfn: (value: Position[], index: number, array: Position[][]) => U, thisArg?: any) => U[])
' has signatures, but none of those signatures are compatible with each other.(2349)

The call is valid because c's type should be Position | Position[], which is assignable to Position | Position[] | Position[][].

Interestingly, I can make the error go away by assigning to a new variable which has a broader type ((Position | Position[])[]):
(playground)

export function transformCoordArray(
  coords: Position | Position[] | Position[][],
  fn: (pt: Position) => Position
): any[] {
  if (isNestedArray(coords)) {
    coords;  // type is Position[] | Position[][]
    const c2: (Position | Position[])[] = coords;
    return c2.map(c => transformCoordArray(c, fn));  // ok
  } else {
    coords;  // type is Position
    return fn(coords);
  }
}

Expected behavior:

No error.

Actual behavior:

The given error.

Playground Link: playground

Related Issues:

@jcalz
Copy link
Contributor

jcalz commented Nov 12, 2019

Duplicate of #7294 (see comment). Related to #29011.

Mentioned in documentation as caveats for improved behavior for calling union types:

This new behavior only kicks in when at most one type in the union has multiple overloads, and at most one type in the union has a generic signature. That means methods on number[] | string[] like map (which is generic) still won’t be callable.

@thorn0
Copy link

thorn0 commented Nov 12, 2019

While methods like push are obviously problematic for number[] | string[], it looks safe to allow methods like map or every for readonly number[] | readonly string[]. Has this (special treatment of read-only arrays when merging union signatures) already been discussed in some issue?

@RyanCavanaugh RyanCavanaugh added In Discussion Not yet reached consensus Suggestion An idea for TypeScript labels Nov 14, 2019
@thorn0
Copy link

thorn0 commented Nov 15, 2019

We are trying to gradually introduce type checking in the Prettier project and facing this issue.

Prettier works with ASTs that come from different parsers.

E.g. suppose we have an object node of type import('estree').Node | import('@typescript-eslint/typescript-estree').TSESTree.Node and code like this:

if (node.type === "ObjectExpression") {
    return node.properties.every(
      p => ...
    );
  }

Even though node.properties are correctly inferred to be of type ObjectLiteralElementLike[] | Property[] (because both estree and @typescript-eslint/typescript-estree define ObjectExpression, not exactly compatible though), p in the every callback is of type any. 😢

@hinell
Copy link

hinell commented Nov 16, 2019

I think it's inherently bad idea to rely on compiler to figure out the output of the transformCoordArray() meanwhile coords: ... argument varies.
I think much better way either to create overloads (checkout playground):

  ...
  export function transformCoordArray(coords: Position[][], fn: PosTransfCb): Position[][]
  export function transformCoordArray(coords: Position[], fn: PosTransfCb): Position[]
  export function transformCoordArray(coords: Position, fn: PosTransfCb): Position
  ...

or assert coords:

(<any> coords).map(c => transformCoordArray(c, fn)) as Position | Position[] | Position[][];

@jcalz
Copy link
Contributor

jcalz commented May 27, 2021

Fixed by #42620 I think?

@danvk
Copy link
Contributor Author

danvk commented May 27, 2021

Yep! The error in the example in the original issue goes away with 4.3: playground

@danvk danvk closed this as completed May 27, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
In Discussion Not yet reached consensus Suggestion An idea for TypeScript
Projects
None yet
Development

No branches or pull requests

5 participants