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

Breaking changes associated with Union Types #868

Closed
danquirk opened this issue Oct 10, 2014 · 4 comments
Closed

Breaking changes associated with Union Types #868

danquirk opened this issue Oct 10, 2014 · 4 comments
Labels
Breaking Change Would introduce errors in existing code By Design Deprecated - use "Working as Intended" or "Design Limitation" instead

Comments

@danquirk
Copy link
Member

This issue tracks the set of breaking changes that result from the change to union types #824 to implement the suggestion #805. The issues here will be formalized further once an official spec update is done, until then it will mostly serve as an easy reference for the type of coding patterns that have broken for various reasons.

Multiple Best Common Type Candidates

Given multiple viable candidates from a Best Common Type computation we now choose an item (depending on the compiler's implementation) rather than the first item.

var a: { x: number; y?: number };
var b: { x: number; z?: number };

// was { x: number; z?: number; }[]
// now { x: number; y?: number; }[]
var bs = [b, a]; 

This can happen in a variety of circumstances. A shared set of required properties and a disjoint set of other properties (optional or otherwise), empty types, compatible signature types (including generic and non-generic signatures when type parameters are stamped out with any).

Recommendation
Provide a type annotation if you need a specific type to be chosen

var bs: { x: number; y?: number; z?: number }[] = [b, a];

Generic Type Inference

Using different types for multiple arguments of type T is now an error, even with constraints involved:

declare function foo<T>(x: T, y:T): T;
var r = foo(1, ""); // r used to be {}, now this is an error

With constraints:

interface Animal { x }
interface Giraffe extends Animal { y }
interface Elephant extends Animal { z }
function f<T extends Animal>(x: T, y: T): T { return undefined; }
var g: Giraffe;
var e: Elephant;
f(g, e);

See #824 (comment) for explanation.

Recommendations
Specify an explicit type parameter if the mismatch was intentional:

var r = foo<{}>(1, ""); // Emulates 1.0 behavior
var r = foo<string|number>(1, ""); // Most useful
var r = foo<any>(1, ""); // Easiest
f<Animal>(g, e);

or rewrite the function definition to specify that mismatches are OK:

declare function foo<T,U>(x: T, y:U): T|U;
function f<T extends Animal, U extends Animal>(x: T, y: U): T|U { return undefined; }

Generic Rest Parameters

You cannot use hetergeneous argument types anymore:

function makeArray<T>(...items: T[]): T[] { return items; }
var r = makeArray(1, ""); // used to return {}[], now an error

Likewise for new Array(...)

Recommendations
Declare a back-compat signature if the 1.0 behavior was desired:

function makeArray<T>(...items: T[]): T[];
function makeArray(...items: {}[]): {}[];
function makeArray<T>(...items: T[]): T[] { return items; }

Overload Resolution with Type Argument Inference

var f10: <T>(x: T, b: () => (a: T) => void, y: T) => T;
var r9 = f10('', () => (a => a.foo), 1); // r9 was any, now this is an error

Recommendations
Manually specify a type parameter

var r9 = f10<any>('', () => (a => a.foo), 1);

Promises

This used to not have an error:

interface Promise<T> {
    then<U>(success?: (value: T) => Promise<U>, error?: (error: any) => Promise<U>, progress?: (progress: any) => void): Promise<U>;
    then<U>(success?: (value: T) => Promise<U>, error?: (error: any) => U, progress?: (progress: any) => void): Promise<U>;
    then<U>(success?: (value: T) => U, error?: (error: any) => Promise<U>, progress?: (progress: any) => void): Promise<U>;
    then<U>(success?: (value: T) => U, error?: (error: any) => U, progress?: (progress: any) => void): Promise<U>;
    done<U>(success?: (value: T) => any, error?: (error: any) => any, progress?: (progress: any) => void): void;
}

interface IPromise<T> {
    then<U>(success?: (value: T) => IPromise<U>, error?: (error: any) => IPromise<U>, progress?: (progress: any) => void): IPromise<U>;
    then<U>(success?: (value: T) => IPromise<U>, error?: (error: any) => U, progress?: (progress: any) => void): IPromise<U>;
    then<U>(success?: (value: T) => U, error?: (error: any) => IPromise<U>, progress?: (progress: any) => void): IPromise<U>;
    then<U>(success?: (value: T) => U, error?: (error: any) => U, progress?: (progress: any) => void): IPromise<U>;
    done? <U>(success?: (value: T) => any, error?: (error: any) => any, progress?: (progress: any) => void): void;
}

declare function f1(x: number): IPromise<number>;
declare function f2(x: number): Promise<number>;
var p: Promise<number>;
var r = p.then(f2, f1, f1).then(f1, f1, f1);
//                                 ^^
// error TS2345: Argument of type '(x: number) => IPromise<number>' is not assignable to parameter of type '(value: IPromise<number>) => IPromise<number>'.
@danquirk danquirk added By Design Deprecated - use "Working as Intended" or "Design Limitation" instead Breaking Change Would introduce errors in existing code labels Oct 10, 2014
@danquirk danquirk mentioned this issue Oct 10, 2014
@RyanCavanaugh
Copy link
Member

Added "Recommendations" to each section

@JsonFreeman
Copy link
Contributor

For the homogeneity constraint in type argument inference (the example with the Giraffe and Elephant), I do not see why we have to give an error if there is a constraint. Intuitively, if Giraffe and Elephant are candidates, we want to pick Animal if we have a feasible way to do so. All we would have to do is get the best common type for T, see that it fails, and then fall back to the constraint. We already do this if we successfully compute a BCT incompatible with the constraint, so why not on failure too?

I looked into the Promises break, and it was never really working anyway. The old behavior was that the first call to then returned Promise<{}>, and the second returned Promise<IPromise<number>>. Now, we return Promise<IPromise<number>> on the first call, and subsequently error, but it doesn't really matter. Union types and our change to type argument inference are red herrings here. The real problem is that as written, IPromise will never be a Promise because of its optional done member. And I'm not sure there is anything we can do about that.

@danquirk
Copy link
Member Author

For the first: the reason to give an error is more one about user expectations than what is technically possible. What are the type relationships that signature is trying to represent? To me it reads that the function expects 2 arguments of the same type which are both subtypes of Animal. So even if both are arguments are subtypes of Animal but of different types then an error is expected. If an Elephant and Giraffe were intended to work then the function would take a T and U both constrained by Animal.

And yeah I looked at all the promises examples closely and essentially all of them were previously Promise<{}> cases.

@JsonFreeman
Copy link
Contributor

I am personally in favor of being gentler when there is a constraint. I think we can do so without losing correctness. That said, the philosophy you explained makes sense.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Breaking Change Would introduce errors in existing code By Design Deprecated - use "Working as Intended" or "Design Limitation" instead
Projects
None yet
Development

No branches or pull requests

4 participants