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

Describing an overridden but incompatible method signature in an ambient class #6094

Closed
saschanaz opened this issue Dec 14, 2015 · 5 comments
Labels
Declined The issue was declined as something which matches the TypeScript vision Suggestion An idea for TypeScript

Comments

@saschanaz
Copy link
Contributor

From SO: http://stackoverflow.com/questions/34087631

/** Represents an iterable collection of Pnp device objects. */
abstract class PnpObjectCollection extends Array {
    /** Returns the iterator for iteration over the items in the collection. */
    first(): Windows.Foundation.Collections.IIterator<Windows.Devices.Enumeration.Pnp.PnpObject>;
    /** Returns the PnpObject located at the specified index. */
    getAt(index: number): Windows.Devices.Enumeration.Pnp.PnpObject;
    /** Retrieves multiple elements in a single pass through the iterator. */
    getMany(startIndex: number): { /** Provides the destination for the result. Size the initial array size as a "capacity" in order to specify how many results should be retrieved. */ items: Windows.Devices.Enumeration.Pnp.PnpObject; /** The number of items retrieved. */ returnValue: number; };
    /** Retrieves the index of the specified item. */
    indexOf(value: Windows.Devices.Enumeration.Pnp.PnpObject): { /** The index of the item to find, if found. */ index: number; /** True if an item with the specified value was found; otherwise, False. */ returnValue: boolean; };
    /** Returns the number of items in the collection. */
    size: number;
}

This is the current generated code by expressing UWP PnpObjectCollection. PnpObjectCollection.prototype.__proto__ === Array is true here so I added extends Array but as you see the signature of indexOf method conflicts.

TS2415  Class 'PnpObjectCollection' incorrectly extends base class 'any[]'.
  Types of property 'indexOf' are incompatible.
    Type '(value: PnpObject) => { index: number; returnValue: boolean; }' is not assignable to type '(searchElement: any, fromIndex?: number) => number'.
      Type '{ index: number; returnValue: boolean; }' is not assignable to type 'number'.

30+ classes in entire d.ts file have this problem, so how can I express this class without errors? I cannot add indexOf(value: T, fromIndex: number): number line because calling indexof(value, 0) returns an object, not a number.

@saschanaz
Copy link
Contributor Author

Current winrt.d.ts in DefinitelyTyped have manually added ES5 compatible Array methods on IVectorView interface but that will:

  1. force the class declarations to have all the interface members copied (Question: Why must ambient interface declaration members be redeclared on every class? #5468)
  2. make them lack ES6 methods (adding ES6 ones will break ES5 compatibility when with symbols)

@DanielRosenwasser
Copy link
Member

I've answered on http://stackoverflow.com/a/34299582/4386952.

In general, it seems strange that the library authors disregarded the implementation - but that's the thing. We require all derived signatures to be compatible in an ambient class even if a library author is doing shenanigans, and we should discuss whether that should be the case.

@DanielRosenwasser DanielRosenwasser added the In Discussion Not yet reached consensus label Dec 15, 2015
@DanielRosenwasser DanielRosenwasser changed the title How can UWP IVectorView classes be expressed in TS? Describing an overridden but incompatible method signature in an ambient class Dec 15, 2015
@RyanCavanaugh RyanCavanaugh added the Suggestion An idea for TypeScript label Dec 15, 2015
@saschanaz
Copy link
Contributor Author

@DanielRosenwasser It helps me a lot, thanks! 👍

My thought:

abstract class PnpObjectCollection extends Array<PnpObject> {
    /* suppress signature confliction error */
    override indexOf(value: Windows.Devices.Enumeration.Pnp.PnpObject): { index: number; returnValue: boolean; };
}
let collection: PnpObjectCollection;
let ary: Array<PnpObject>
ary = collection; // error
collection = ary; // error

@RyanCavanaugh RyanCavanaugh added Declined The issue was declined as something which matches the TypeScript vision and removed In Discussion Not yet reached consensus labels Mar 15, 2016
@RyanCavanaugh
Copy link
Member

Answer on SO is generally correct, as well as the observation that you don't really need to extend Array in an ambient class except to save typing (which is sort of a red herring since the type is nonsubstitutable anyway so you should be checking each method individually)

@saschanaz
Copy link
Contributor Author

@RyanCavanaugh I think I need to extend Array as the class have Array in its prototype chain and the declaration should not require users to manually add those new ES6 methods, no?

@microsoft microsoft locked and limited conversation to collaborators Jun 19, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Declined The issue was declined as something which matches the TypeScript vision Suggestion An idea for TypeScript
Projects
None yet
Development

No branches or pull requests

3 participants