angular / tsickle

Tsickle — TypeScript to Closure Translator
MIT License
896 stars 110 forks source link

Feature Request: Support resolved mapped types #888

Open SLaks opened 6 years ago

SLaks commented 6 years ago

Mapped types themselves are fundamentally incompatible with Closure Compiler. However, the results of mapped types (once all type parameters have been specified) are normal object types that can work fine.

Currently, mapped types are always emitted as ?.

This FR is to emit any mapped type whose properties are known to Tsickle (IOW, if the in clause is a resolved type, not a type parameter) as a record literal type ({foo: !Type, ...}).

This gets interesting with named mapped types (type Foo = { [name in ...]: .... }); where possible, we should reference the name for brevity / clarity / (for classes or interfaces) to avoid property invalidations. There are (in general) three categories of type references:

  1. Anonymous types (a {}, not hidden behind an alias)
    • These can only be emitted as a record literal, or ? if not expressible
  2. Named types whose definitions are expressible (they don't involve keys or intersections from a type parameter).
    • These should be emitted as a reference to the name.
  3. Named types whose definitions are not expressible
    • These should be emitted as a record literal (if possible).

Note that these guidelines apply to all type features that are not supported by JSCompiler but can produce object types; this also includes intersection types (#886) & conditional types.

Note that computing whether a type is expressible is a recursive operation that depends on the type parameters passed (examples coming tomorrow).

SLaks commented 6 years ago

Some examples, with expected Tsickle annotations:

// This is fully expressible
/** @type {{a: number, b: number}} */
let x: Record<'a'|'b', number>;

class ServiceId<T>{}
// Only expand the inner type; the outer type is expressible.
/** @type {!ServiceId<{a: number, b: number}>} */
let x: ServiceId<Record<'a'|'b', number>>;
// This type is only partially expressible
/** @typedef {{name: string, metadata: ?}} */
type Spec<T> = {
    name: string;
    metadata: Record<keyof T, boolean>;
}

// So this must re-specify the entire type to fill in the mapped type
/** @type {{name: string, metadata: {a: boolean}}} */
let x: Spec<{ a: number }>;
// In contrast, this type is fully expressible (the mapped type's keys don't use T)
// Ignore my condensed syntax; this would be a full @record type.
/** @record @template T class { ctor() {name: string, samples: {a: T, b: T}}} */
type Options<T> = {
    name: string;
    samples: Record<'a'|'b', T>;
}

/** @type {!Options<{a: number>}} */
let x: Options<{ a: number }>;
// Simpler version of that:
/** @typedef {{name: string, samples: {a: boolean, b: boolean}}} */
type Options = {
    name: string;
    samples: Record<'a'|'b', boolean>;
};

/** @type {!Options} */
let x: Options;
SLaks commented 6 years ago

The practical use-cases of these distinctions are builders that use advanced generics to build constants with static types. Such builders cannot be called safely from Closure code (because it doesn't support the generics used in their parameters), but their results can be. This allows a mixed codebase to use TypeScript to create DRY, strongly-typed objects, then consume them from legacy Closure code.

Simplified example (I have much more complex uses which work with advanced optimizations) coming tomorrow.

evmar commented 5 years ago

One thing I haven't been able to figure out is when we see code like

class Foo implements SomeMappedType<Bar> {

We can evaluate SomeMappedType<Bar> as a record literal, but we lose the relationship between Foo and Bar. For example if Bar defines some bar field, and then Foo wants to reference it, with disambiguate properties, Closure won't know that Foo and Bar are part of an inheritance hierarchy, and so it will name the supertype's one something like Bar$bar and the subtype Foo$bar and they won't line up.

mprobst commented 5 years ago

Maybe @shicks or @blickly have a suggestion?

shicks commented 5 years ago

I'm not coming up with anything off-hand. To make the issue clear, let me propose another example:

class Bar {
    foo: string;
    constructor(foo: string) { this.foo = foo;}
}

type Mapped<T> = {[K in keyof T]: number};

type Baz = Mapped<Bar>;

let baz: Baz = { foo: 42 }; // {foo: 'x'} would be an error

The only thing the types Bar and Baz share is the keys - but the properties themselves are unrelated (one must be a string, one must be a number). It could be possible to force a type mismatch between these types when we detect that a property key is shared between them, but I'm wary of doing this too much as it will cause lots of backoff - it basically invalidates that entire property name on all types IIUC, but really we only want to link the property name on two types. There are some Closure Library primitives in goog.reflect that could possibly help (though I haven't tested it), or maybe @lends or Object.defineProperties (in an if (false), of course) could help somehow?