Skip to content

Bloomberg feedback for 5.3 Beta #56213

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

Closed
dragomirtitian opened this issue Oct 24, 2023 · 0 comments
Closed

Bloomberg feedback for 5.3 Beta #56213

dragomirtitian opened this issue Oct 24, 2023 · 0 comments
Labels
Discussion Issues which may not have code impact

Comments

@dragomirtitian
Copy link
Contributor

dragomirtitian commented Oct 24, 2023

By @molisani & @dragomirtitian

We are in the process of evaluating the impact of upgrading to TypeScript 5.3 on our codebase. Here are some preliminary findings up to version 5.3.0-dev.20230928.

This is shaping up to be a low impact release. The following table lists changes that affected our codebase:

(🟢 - Good breaking change, 🔴 - Bad breaking change)

# Change Affects Release notes Packages affected Reported As
1 Unrelated interface causes assignability to change 🔴 Type checking Not Announced <1% #56099
2 Source map improvements 🟢 Source Maps Not Announced ~70%
3 Negative number literals are not allowed 🟢 Compiler API Not Announced Tooling
4 Error on duplicate computed properties 🟢 Type checking Not Announced <1%
5 TS now understands guard(o) === false 🟢 Type checking Announced <1%
6 New error on super.field 🟢 Type checking Announced <1%
7 Bug fix to assignability of Record<string, unknown> in mapped type 🟢 Type checking Not Announced <1%
8 Set accessor parameter name in d.ts from JS is preserved 🟢 Declaration Emit Not Announced 1

1. Unrelated interface causes assignability to change

Reported as #56099

Removing Keyed (unused) bellow causes R to change. The error also sometimes appears and sometimes it does not in VS Code.

export { }
interface Map<V> extends Collection<V> {
    flatMap<VM>(): Map<VM>;
}

interface Collection<V> {
    value: V; // sprinkle some covariance
    map: Map<V>;
    concat(): Collection<unknown>;
    flatMap(): Collection<V>;
    flatMap<VM>(): Collection<VM>;
}

// Comment out and it works like in 5.2
interface Keyed extends Collection<number> {
    concat(): Keyed;
}

type R = Map<never> extends Collection<infer V> ? V : "NO";

const r = null! as R;
const t: "NO" = r;

Both 5.2 and 5.3 don't really seem to do a good job here, R should probably be never (and in 5.3 removing flatMap(): Collection<V>; will make R never)

2. Source map improvements

Some mapping in sourcemaps appear to better point back to the source code (they previously did not include the original line, column and file).

Seems like a net improvement but given the huge diff it's difficult to assess if any other changes occurred.

3. Negative number literals are not allowed

Internal tooling created negative numeric literals, which are not supported (they should be expressed as a minus unary operator and a positive numeric literal). TypeScript 5.3 now asserts that numeric literals are always positive.

4. Error on duplicate computed properties

Typescript now correctly detects more duplicate computed properties. This caught several places in our code base where we had duplicate properties in our object literals.

const c = { V: "B" } as const;
let o = {
    [c.V]: 1,
    [c.V]: 1, // No error in 5.2
}

Playground Link 5.2
Playground Link 5.3 beta

5. TS now understands guard(o) === false

Typescript now correctly uses guard(o) === false in CFA. This surfaced several places in our code where even though the type guard was being used, it's effect on the type of the symbol was basically being ignored.

6. New error on super.field

TypeScript now correctly errors on access to fields on super. We currently use a field in the base class to represent what is actually a method, we were ignoring the previous error on the override:

class B {
    on!: () => void;
}

class D extends B{
    //@ts-ignore
    on() {
        super.on(); // new error
    }
}

Playground Link 5.2
Playground Link 5.3 beta

7. Bug fix to assignability of Record<string, unknown> in mapped type

5.3 includes a bug fix where a nested property of type Record<string, unknown> that passes through a mapped type becomes assignable to an object with required properties.

This caused some existing code to fail, but definitely a good catch.

export type Id<T> = {
    [P in keyof T]: Id<T[P]>;
};

export type Data = {
    action: { // Has to be nested
        commit: {
            category: string
            group: number;
        };
    }
}

declare let x: Id<{ action: { commit: Record<string, unknown> } }>
let u: Id<Data> = x; // should be an error is in 5.3
u.action.commit = x.action.commit; // should be an error is in 5.3

Playground Link 5.2
Playground Link 5.3 beta

8. Set accessor parameter name in d.ts from JS is preserved

This one came up only in internal tests. The original name of the setter parameter is now preserved where the name was previously generated.

class Cls {
    get n() { return  ""; }
    set n(a) { /*a renamed to arg in 5.2 declarations*/ }
}

Playground Link 5.2 Playground Link 5.3 beta

@RyanCavanaugh RyanCavanaugh added the Discussion Issues which may not have code impact label Oct 24, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Discussion Issues which may not have code impact
Projects
None yet
Development

No branches or pull requests

2 participants