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

[Feature Request] Future-proof always-aliasing/never-expanding of mapped/intersection/union/etc. types #35654

Closed
5 tasks done
AnyhowStep opened this issue Dec 12, 2019 · 7 comments · Fixed by #42149
Closed
5 tasks done
Assignees
Labels
Suggestion An idea for TypeScript

Comments

@AnyhowStep
Copy link
Contributor

AnyhowStep commented Dec 12, 2019

Search Terms

Force TS to always alias type, optional property, never expand type

Suggestion

It would be nice to have a way to annotate a type alias and tell TS, "As much as possible, do not expand this type alias in emit". I don't know what syntax it should have. But maybe,

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 (as much as possible).

class and interface types have this behaviour. Their identifiers are used as much as possible in emit.

The "readability" of a particular emit is almost always subjective. So, giving developers some control over the emit can make code easier to understand.


Here's an example, where using a type alias in a union causes the type alias' identifier to not be used in emit,

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;

If PleaseDoNotExpand<> were a class or interface, we would have const b: PleaseDoNotExpand<number>|undefined

Use Cases

What do you want to use this for?

  • Better emit for .d.ts files
  • Better emit for tooltip hover
  • Better emit for error messages

Some type aliases are hundreds of lines long, after expansion. These type aliases usually have short, intuitive identifiers. But those identifiers tend to get lost when used in union types (and optional properties). See #35616 for more examples.

What shortcomings exist with current approaches?

There is no "general purpose" workaround for the current problem.

So far, I've thought of two workarounds. But they only work for very specific use cases.

Workaround 1: The type alias has statically known members

#34777

#34777 (comment)

The idea is to use an interface to extend the type alias. From that point, only the interface's identifier is used in emit. This has the most desirable behaviour. If it could be extended to work for all use cases, then I wouldn't have this feature request.

Workdaround 2: The type alias is being removed by unions/optional properties

#35616

#35616 (comment)

The idea is to create a new type alias that is a union of the original type and the new union elements. However, it does not always work and I don't know why. But this is better than always expanding.

Examples

//No idea about syntax
type interface PleaseDoNotExpand<T> =
    | T
    | { x : T }
;

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


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

/*
//It makes sense to lose the identifier at this point
const c: number
*/
declare const c: Extract<PleaseDoNotExpand<number>, number>;

/*
//It makes sense to lose the identifier at this point
const c: { x : number }
*/
declare const d: Exclude<PleaseDoNotExpand<number>, number>;

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.

Related

#34556 asks to never alias a type. This asks to always alias a type.

@EdwardDrapkin
Copy link

We're a little more than a year into our adoption of TS at work, and at our end-of-year retrospective, TS was one of the topics of discussion. Everyone was generally quite happy with the transition, except one thing kept coming up, particularly from the juniors: reading and understanding the tsc error messages was very difficult and overwhelming. When I introduce a new junior to TS, I've taken to copy and pasting the error message into a text editor, and then gradually revealing it line-by-line to the junior because the sheer size of it tends to overwhelm people to the point of frustration, where they're defensive and less open to learning.

All of this is to say I think this is a great suggestion, but it may just be a band-aid on a deeper issue. This would certainly treat a symptom, but perhaps this suggestion works best if it's included as part of a larger overhaul in the diagnostic ergonomics of the language?

One thing I thought about was taking an error message that contains an expanded type and showing its relevant alias in the erroneous frame, and then having a footnote that shows its expansion below the error, but I haven't had time to mock it up, let alone think it all the way through and open an issue/PR. Another idea I had was to have some kind of pattern matching in errors so I could specify error-time aliases, but I'm not sure what that would look like, so that's an even more raw idea.

Point is, there are probably a ton of great ideas out there to improve error reporting, and if I'm not the only one who's noticed diagnostic messages could use some work, why not see what else is out there and come up with a long term solution? Or maybe that's just letting "great" be the enemy of "good."

@AnyhowStep
Copy link
Contributor Author

AnyhowStep commented Feb 26, 2020

may just be a band-aid on a deeper issue.

Not just a bandaid over error messages. But I agree.
Certainly, error messages could use other improvements, besides allowing more control from devs.

My main personal use case is,

Better emit for .d.ts files

Sometimes, you just want to look at the emitted .d.ts file and not the emitted .js or source .ts files.
Because we can't control when and how TS expands aliases (or not), the emitted .d.ts files can look very ugly for projects that are generic-heavy.


Also, regarding error messages, this issue is still very important.

When TS expands types in emit, it causes TS to lose the "connection" to the original type alias.

Downstream users of the library will just see the expanded type. Then, the code that generates error messages can't show the original type alias because that information no longer exists.

As an (imaginary) example,

//my-lib.ts
export type MyAlias<T> = //snip complicated stuff
export function myFunc<T> (t : T, myAlias : MyAlias<T>) : void {}

After transpiling,

//my-lib.d.ts
export type MyAlias<T> = //snip complicated stuff
export function myFunc<T> (t : T, myAlias : /* snip complicated stuff */) : void {}

Then, when downstream users try to use myFunc,

//downstream user
import {myFunc} from "my-lib";
myFunc("test-t", "trigger-some-error-here");

The error message they receive will be about /* snip complicated stuff */, and will have no link to MyAlias<T>, since the emit completely expanded the type alias and erased it from myFunc<T>().

So, not a bandaid, but necessary to solve the problem of bad error messages for library authors.

@EdwardDrapkin
Copy link

Maybe my choice of the word "band-aid" was poor, and if there was any implication I'm anything but fully supportive of this idea, that wasn't my intent. I only meant to suggest that error reporting in general needs some love, and while fixing displayed aliases is absolutely a part of that, there may be opportunity to improve things in other ways as well. No argument from me that diagnostic messages need to have better control over how and when aliases are expanded, just there's more work to be done too[1] :)

1: stuff like omitting intermediary expansions would be nice (type Foo expanded from /* stuff */ ... 4 layers omitted ... from /* basest stuff */), or being able to control specific error messages at specific code locations (which is probably a pipe dream), etc.

@AnyhowStep
Copy link
Contributor Author

AnyhowStep commented Oct 8, 2020

So, I know not many people deal with types hundreds of lines long but I figured I'd share a workable solution for some cases.

In workaround 1 listed above, we had a type alias already handy.
However, this workaround is for when we do not have a type alias, but have a variable of the type, instead.


It is hacky and forces the responsibility of readability onto downstream users, rather than library authors, but it's better than nothing...

The solution has the following form,

//Imagine `foo()` is a generic function returning a type hundreds of lines long.
const _x = foo(/*args*/);

type Id<T> = T;

export interface X extends Id<typeof _x> {
}

export const x : X = _x;

Every reference to x will have the very nice and readable type X.


Here is an example of the benefits of this approach,

Before extends Id<> Hack

image

This type wasn't even hundreds of lines long, but you can already see that the error output is so large that we have to scroll a lot to read the whole thing.

After extends Id<> Hack

image

This is SO MUCH more readable. It's like night and day.


Before the hack, compiling this 15k LoC project took 48s.
After the hack, it now takes 33s.
That's like a 30% speed up.
I had to use this hack for about 50 different variables.

I have another project that is wayyyy larger and I'm pretty sure it'll get the same benefits.


Pros:

  • Better emit for .d.ts
  • Better emit for errors
  • Better compile times

Cons:

  • Requires boilerplate to implement

Would be nice to have syntax like,

export const x = foo(/*args*/) as export interface X;

@ClaireNeveu
Copy link

This is a huge issue for any library that deals with large union-types. I have a library which exposes an AST which is—naturally—a union type. Throughout the library functions are generally overloaded to take either an AST node or a shorthand, but the implementation of the AST node ends up being 99% of the error.

See this method:

where(clause: { [K in keyof Table]?: Table[K] } | Expr<Ext>)

which produces this rather unwieldy error message when the user has a typo:

src/builder/index.ts:547:65 - error TS2345: Argument of type '{ ifd: number; }' is not assignable to parameter of type '({ _tag: "Ident"; } & string & { __schemaType: MySchema; __returnType: any; }) | (Wildcard & { __schemaType: MySchema; __returnType: any; }) | (QualifiedWildcard & { __schemaType: MySchema; __returnType: any; }) | (CompoundIdentifier & { __schemaType: MySchema; __returnType: any; }) | (Lit & { __schemaType: MySchema; __returnType: any; }) | (Between & { __schemaType: MySchema; __returnType: any; }) | (BinaryApp & { __schemaType: MySchema; __returnType: any; }) | (Case & { __schemaType: MySchema; __returnType: any; }) | (Cast & { __schemaType: MySchema; __returnType: any; }) | (Collate & { __schemaType: MySchema; __returnType: any; }) | (Exists & { __schemaType: MySchema; __returnType: any; }) | (Extract & { __schemaType: MySchema; __returnType: any; }) | (FunctionApp & { __schemaType: MySchema; __returnType: any; }) | (IsNull & { __schemaType: MySchema; __returnType: any; }) | (InList & { __schemaType: MySchema; __returnType: any; }) | (InSubQuery & { __schemaType: MySchema; __returnType: any; }) | (Parenthesized & { __schemaType: MySchema; __returnType: any; }) | (SubQuery & { __schemaType: MySchema; __returnType: any; }) | (UnaryApp & { __schemaType: MySchema; __returnType: any; }) | (ExprExtension & { __schemaType: MySchema; __returnType: any; }) | { name?: string | undefined; id?: number | undefined; "employee.name"?: string | undefined; "employee.id"?: number | undefined; }'.
Object literal may only specify known properties, and 'ifd' does not exist in type '(Wildcard & { __schemaType: MySchema; __returnType: any; }) | (QualifiedWildcard & { __schemaType: MySchema; __returnType: any; }) | (CompoundIdentifier & { __schemaType: MySchema; __returnType: any; }) | (Lit & { __schemaType: MySchema; __returnType: any; }) | (Between & { __schemaType: MySchema; __returnType: any; }) | (BinaryApp & { __schemaType: MySchema; __returnType: any; }) | (Case & { __schemaType: MySchema; __returnType: any; }) | (Cast & { __schemaType: MySchema; __returnType: any; }) | (Collate & { __schemaType: MySchema; __returnType: any; }) | (Exists & { __schemaType: MySchema; __returnType: any; }) | (Extract & { __schemaType: MySchema; __returnType: any; }) | (FunctionApp & { __schemaType: MySchema; __returnType: any; }) | (IsNull & { __schemaType: MySchema; __returnType: any; }) | (InList & { __schemaType: MySchema; __returnType: any; }) | (InSubQuery & { __schemaType: MySchema; __returnType: any; }) | (Parenthesized & { __schemaType: MySchema; __returnType: any; }) | (SubQuery & { __schemaType: MySchema; __returnType: any; }) | (UnaryApp & { __schemaType: MySchema; __returnType: any; }) | (ExprExtension & { __schemaType: MySchema; __returnType: any; }) | { name?: string | undefined; id?: number | undefined; "employee.name"?: string | undefined; "employee.id"?: number | undefined; }'.

547 const blaahhh = b.from('employee').select('id', 'name').where({ ifd: 5 })

The only part actually relevant to the user is the last part where it lists the allowed properties. If there were a way to stop TypedAst from expanding the error message would be much more useful:

src/builder/index.ts:547:65 - error TS2345: Argument of type 'TypedAst<MySchema, any, Expr> | { name?: string | undefined; id?: number | undefined; "employee.name"?: string | undefined; "employee.id"?: number | undefined; }'.

547 const blaahhh = b.from('employee').select('id', 'name').where({ ifd: 5 })

This one is particularly bad because I'd carrying around "phantom" type information but even excluding that it's still not great:

src/builder/index.ts:547:65 - error TS2345: Argument of type '{ fid: number; }' is not assignable to parameter of type 'Ident | Wildcard | QualifiedWildcard | CompoundIdentifier | Lit | Between | BinaryApp | Case | Cast | Collate | Exists | Extract | FunctionApp | IsNull | InList | InSubQuery | Parenthesized | SubQuery | UnaryApp | ExprExtension | { name?: string | undefined; id?: number | undefined; "employee.name"?: string | undefined; "employee.id"?: number | undefined; }'.
Object literal may only specify known properties, and 'fid' does not exist in type 'Wildcard | QualifiedWildcard | CompoundIdentifier | Lit | Between | BinaryApp | Case | Cast | Collate | Exists | Extract | FunctionApp | IsNull | InList | InSubQuery | Parenthesized | SubQuery | UnaryApp | ExprExtension | { name?: string | undefined; id?: number | undefined; "employee.name"?: string | undefined; "employee.id"?: number | undefined; }'.

547 const blaahhh = b.from('employee').select('id', 'name').where({ fid: 5 })

@AnyhowStep AnyhowStep changed the title [Feature Request] Future-proof always-aliasing of mapped/intersection/union/etc. types [Feature Request] Future-proof always-aliasing/never-expanding of mapped/intersection/union/etc. types Nov 10, 2020
@marijnh
Copy link

marijnh commented Dec 23, 2020

Merging #42075, which asks more specifically about not flattening nested union/intersection types, into this.

@ahejlsberg ahejlsberg self-assigned this Dec 29, 2020
@ahejlsberg ahejlsberg removed the Awaiting More Feedback This means we'd like to hear from more people who would be helped by this feature label Jan 9, 2021
@ahejlsberg ahejlsberg added this to the TypeScript 4.2.0 milestone Jan 9, 2021
@AnyhowStep
Copy link
Contributor Author

I just wanted to say that I love you all for all the work you all do on TS <3 Happy New Year!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Suggestion An idea for TypeScript
Projects
None yet
Development

Successfully merging a pull request may close this issue.

6 participants