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

Future-proof always-aliasing of mapped/intersection/union/etc. types #35616

Closed
AnyhowStep opened this issue Dec 10, 2019 · 9 comments
Closed
Labels
Unactionable There isn't something we can do with this issue

Comments

@AnyhowStep
Copy link
Contributor

AnyhowStep commented Dec 10, 2019

TypeScript Version: 3.5.1, 3.7.2

Search Terms:

Force TS to alias type, optional property, type expand

Code

type Identity<T> = T;

type PleaseDoNotExpand<T> =
    | T
    | { x : T }
;

type MappedType<ObjT> =
    Identity<
        & {
            readonly [k in Exclude<keyof ObjT, "x">]: (
                PleaseDoNotExpand<ObjT[k]>
            )
        }
        & {
            readonly [k in Extract<keyof ObjT, "x">]?: (
                PleaseDoNotExpand<ObjT[k]>
            )
        }
    >
;

declare function mappedType<ObjT>(obj: ObjT): MappedType<ObjT>;

/*
    Expected:
    const x: {
        readonly y: PleaseDoNotExpand<string>;
        readonly z: PleaseDoNotExpand<boolean>;
    } & {
        readonly x?: PleaseDoNotExpand<number>|undefined;
    }
*/
/*
    Actual:
    const x: {
        readonly y: PleaseDoNotExpand<string>;
        readonly z: PleaseDoNotExpand<boolean>;
    } & {
        readonly x?: number | {
            x: number;
        } | undefined;
    }
*/
const x = mappedType({
    x: 1,
    y: "hi",
    z: true,
});

Expected behavior:

Expected x to resolve to,

    const x: {
        readonly y: PleaseDoNotExpand<string>;
        readonly z: PleaseDoNotExpand<boolean>;
    } & {
        readonly x?: PleaseDoNotExpand<number>|undefined;
    }

Actual behavior:

Actually resolves to,

    const x: {
        readonly y: PleaseDoNotExpand<string>;
        readonly z: PleaseDoNotExpand<boolean>;
    } & {
        readonly x?: number | {
            x: number;
        } | undefined;
    }

This behaviour is caused by the ? optional property modifier. It seems to do something that messes up emit, and makes it inconsistent with required properties.

Since required properties do not expand the PleaseDoNotExpand<> type,
optional properties should also not expand the PleaseDoNotExpand<> type.

I believe this is a bug in emit.

Playground Link:

Playground

Related Issues:

Kind of related to this,
#34556

That issue wants a way to future-proof ways of forcing TS to not alias a type.
The Identity<> trick works nicely at the moment.


Also related to this,
#34777

#34777 (comment)

That issue wants a way to force TS to always alias a type.
There is a workaround using interface when the properties are always known and you do not have union types.

It does not apply in this case, because I have union types.


This mini-rant probably belongs in a different issue but...

I would like a way to have TS never expand a type alias for hover tooltip and emit.
Not by default, anyway.

My actual PleaseDoNotExpand<> type is easily 100+ lines long. It makes everything unreadable, when TS fully expands it.

I can read PleaseDoNotExpand<Date> better than I can read the following,

    readonly createdAt?: Date | tsql.IExpr<{
        mapper: Mapper<unknown, Date>;
        usedRef: tsql.IUsedRef<{}>;
    }> | (tsql.IQueryBase<{
        fromClause: tsql.IFromClause<tsql.FromClauseData>;
        selectClause: [tsql.IColumn<{
            tableAlias: string;
            columnAlias: string;
            mapper: Mapper<unknown, Date>;
        }> | tsql.IExprSelectItem<{
            mapper: Mapper<unknown, Date>;
            tableAlias: string;
            alias: string;
            usedRef: tsql.IUsedRef<never>;
        }>];
        limitClause: tsql.LimitClause | undefined;
        compoundQueryClause: readonly tsql.CompoundQuery[] | undefined;
        compoundQueryLimitClause: tsql.LimitClause | undefined;
        mapDelegate: tsql.MapDelegate<never, never, unknown> | undefined;
    }> & tsql.IQueryBase<{
        fromClause: tsql.IFromClause<{
            outerQueryJoins: readonly tsql.IJoin<tsql.JoinData>[] | undefined;
            currentJoins: undefined;
        }>;
        selectClause: readonly (tsql.ColumnMap | tsql.IColumn<tsql.ColumnData> | tsql.IExprSelectItem<tsql.ExprSelectItemData> | tsql.ColumnRef)[] | undefined;
        limitClause: tsql.LimitClause | undefined;
        compoundQueryClause: undefined;
        compoundQueryLimitClause: tsql.LimitClause | undefined;
        mapDelegate: tsql.MapDelegate<never, never, unknown> | undefined;
    }> & tsql.IQueryBase<{
        fromClause: tsql.IFromClause<{
            outerQueryJoins ...
@AnyhowStep
Copy link
Contributor Author

Smaller repro,

type PleaseDoNotExpand<T> =
    | T
    | { x : T }
;

/*
Expected: const a: PleaseDoNotExpand<number>
Actual  : const a: PleaseDoNotExpand<number>
*/
declare const a: PleaseDoNotExpand<number>;


/*
Expected: const b: PleaseDoNotExpand<number>|undefined
Actual  : const b: number | {x: number;} | undefined
*/
declare const b: PleaseDoNotExpand<number>|undefined;

Playground


So, it might not be the ? modifier.

It's probably because I'm union'ing PleaseDoNotExpand<T> with a type that is not a subtype of PleaseDoNotExpand<T>

Playground

@AnyhowStep
Copy link
Contributor Author

AnyhowStep commented Dec 10, 2019

So... I guess this is a feature request?

Suggestion

I would like the opposite of #34556 , which asks for a way to force TS to always expand a type.

I would like a way to force TS to never expand a type.

Use Cases

More readable emit! For declaration files, for tooltip hover, for error messages, etc.

At some point, with complex projects, emit for certain types end up being hundreds of lines long.
Either because of large objects, or large unions, or large intersections, etc.

Some of these large types actually have very short and intuitive names (their type alias identifier).
But TS seems to always expand the type once you put it in a union,
and we lose the readable type.

Right now, there is a general workaround for always expanding a type. (The Identity<> trick).

However, there is no general workaround for never expanding a type.
There is a special case described here: #34777 (comment)

Examples

Look at how unreadable the expanded type is for tooltip hover (and also .d.ts emit) =(

type PleaseDoNotExpand<T> =
    | { a : T }
    | { b : T }
    | { c : T }
    | { d : T }
    | { e : T }
    | { f : T }
    | { g : T }
    | { h : T }
    | { i : T }
    | { j : T }
    | { k : T }
    | { l : T }
    | { m : T }
    | { n : T }
    | { o : T }
    | { p : T }
    | { q : T }
    | { r : T }
    | { s : T }
    | { t : T }
    | { u : T }
    | { v : T }
    | { w : T }
    | { x : T }
    | { y : T }
    | { z : T }
;

/*
const a: PleaseDoNotExpand<number>
*/
declare const a: PleaseDoNotExpand<number>;


/*
const b: {
    a: number;
} | {
    b: number;
} | {
    c: number;
} | {
    d: number;
} | {
    e: number;
} | {
    f: number;
} | {
    g: number;
} | {
    h: number;
} | {
    i: number;
} | {
    j: number;
} | {
    k: number;
} | {
    l: number;
} | {
    m: number;
} | {
    n: number;
} | ... 12 more ... | undefined

*/
declare const b: PleaseDoNotExpand<number> | undefined;

Playground


Look at how unreadable the expanded type is for error messages =(

type PleaseDoNotExpand<T> =
    | { a : T }
    | { b : T }
    | { c : T }
    | { d : T }
    | { e : T }
    | { f : T }
    | { g : T }
    | { h : T }
    | { i : T }
    | { j : T }
    | { k : T }
    | { l : T }
    | { m : T }
    | { n : T }
    | { o : T }
    | { p : T }
    | { q : T }
    | { r : T }
    | { s : T }
    | { t : T }
    | { u : T }
    | { v : T }
    | { w : T }
    | { x : T }
    | { y : T }
    | { z : T }
;

/*
Type 'true' is not assignable to type 'PleaseDoNotExpand<number>'.(2322)
*/
const a: PleaseDoNotExpand<number> = true;


/*
Type 'true' is not assignable to type '{ a: number; } | { b: number; } | { c: number; } | { d: number; } | { e: number; } | { f: number; } | { g: number; } | { h: number; } | { i: number; } | { j: number; } | { k: number; } | { l: number; } | { m: number; } | { n: number; } | ... 12 more ... | undefined'.(2322)
*/
const b: PleaseDoNotExpand<number> | undefined = true;

Playground

Checklist

My suggestion meets these guidelines:

  • This wouldn't be a breaking change in existing TypeScript/JavaScript code
  • This wouldn't change the runtime behavior of existing JavaScript code
  • This could be implemented without emitting different JS based on the types of the expressions
  • This isn't a runtime feature (e.g. library functionality, non-ECMAScript syntax with JavaScript output, etc.)
  • This feature would agree with the rest of TypeScript's Design Goals.

@AnyhowStep AnyhowStep changed the title Optional property modifier causes emit bug; overly eager type expansion Future-proof always-aliasing of mapped/intersection/union/etc. types Dec 10, 2019
@AnyhowStep
Copy link
Contributor Author

I guess I want the kind of aliasing behavior that interface and class have, since their identifiers are always used, but for type.

I wonder if the following syntax would make any sense at all,

type interface PleaseDoNotExpand<T> = /*complex type*/;

In all aspects, the above is a type alias, except for the fact that its type will not expand in emit

@AnyhowStep
Copy link
Contributor Author

Here's a workaround (for my specific case),

type Identity<T> = T;

type PleaseDoNotExpand<T> =
    | T
    | { x : T }
;

type PleaseDoNotExpandOrUndefined<T> =
    | PleaseDoNotExpand<T>
    | undefined
;

type MappedType<ObjT> =
    Identity<
        & {
            readonly [k in Exclude<keyof ObjT, "x">]: (
                PleaseDoNotExpand<ObjT[k]>
            )
        }
        & {
            readonly [k in Extract<keyof ObjT, "x">]?: (
                PleaseDoNotExpandOrUndefined<ObjT[k]>
            )
        }
    >
;

declare function mappedType<ObjT>(obj: ObjT): MappedType<ObjT>;

/*
    const x: {
        readonly y: PleaseDoNotExpand<string>;
        readonly z: PleaseDoNotExpand<boolean>;
    } & {
        readonly x?: PleaseDoNotExpandOrUndefined<number>;
    }
*/
const x = mappedType({
    x: 1,
    y: "hi",
    z: true,
});

Playground

The idea is to introduce a new type alias that is a union of undefined and the type I originally had.

The emit isn't as nice as I would like it to be, but so much better than before.

@AnyhowStep
Copy link
Contributor Author

AnyhowStep commented Dec 12, 2019

The workaround outlined above does not always work.

It works for the tooltip,
image

It does not work for .d.ts files,
image

But it seems to only break for boolean as a type parameter.

Is this a bug?

@RyanCavanaugh
Copy link
Member

@AnyhowStep I can't work with reports this big. Please, please, please post something succinct once you've boiled it down to something actionable.

@RyanCavanaugh RyanCavanaugh added the Unactionable There isn't something we can do with this issue label Dec 12, 2019
@AnyhowStep
Copy link
Contributor Author

AnyhowStep commented Dec 12, 2019

The original comment is a repro of the problem for my use case, which is pretty small, imo.

The second comment is an even smaller repro. Writing the original comment helped me realize there might be a different thing causing problems for me. I experimented and realized that it was unions (not optional properties) that were removing the type alias from emit. Optional properties just happen to always union with undefined.

The third comment is me realizing this might be better as a feature request, since unions seem to have exhibited this behavior forever. However, such behaviour is undesirable for optional properties (in my opinion).

The fourth comment is a half-hearted proposal, mostly thinking out loud.

The last comment is a workaround for my specific use case, which might be useful to others who encounter this problem, in the future. (And also screenshots to show that the workaround doesn't always work)

Which parts are irrelevant? Which parts should I cull? Which parts do I keep quietly to myself?

I've noticed that you hate it a lot when I make chains of comments. But, to me, these comments help me think, and serve as a guide to others (and future me) who may run into these exact problems some time in the future.

I don't like it when I see threads where people just post stuff without explaining what their thought process is. So, I try to provide my thought process, where I can.


I just gave it some thought.

If I were to create an entirely new issue for this feature request, and rearrange my comments here into the template, I would basically still have the content of all five comments... Just combined into one massive post.

Would that be better?

@RyanCavanaugh
Copy link
Member

The issue tracker is for bug reports or feature requests. I can assign these units of work to people and determine whether or not they're complete.

I can't take a stream of conscious blog about your thoughts on a topic and (in any reasonable length of time) determine what needs to be done, whether it's a bug or not, and schedule it. I can't even tell when the stream of comments is going to stop, so it feels constantly premature to try to do anything with your issues.

The vast, vast majority of things we deal with here can be reasonably described in 1 or 2 screenfulls of text. If you think you need 8 pages of text to define something, it's probably not something that's in a state for us to be making decisions on. It seems like you need to workshop these things in some other forum first until you have a more concise way of communicating what you'd like to happen. Does that make sense?

@AnyhowStep
Copy link
Contributor Author

I opened a new issue that is shorter

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Unactionable There isn't something we can do with this issue
Projects
None yet
Development

No branches or pull requests

2 participants