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

Async function return type annotations seem to break a few TS design goals #5945

Closed
yortus opened this issue Dec 5, 2015 · 5 comments
Closed
Labels
Discussion Issues which may not have code impact

Comments

@yortus
Copy link
Contributor

yortus commented Dec 5, 2015

TL;DR (clarified): Up until v1.7 type annotations have been used only for type checking (goal 9), and never for modifying JavaScript behaviour (goal 7 & non-goal 5). This seems no longer true with async functions, whose return type annotations modify JavaScript behaviour, and give compile errors when used purely for type checking (eg #5911).

Design Decision

I was surprised to discover via #5911 that different code is emitted for async functions depending on the type annotations encountered. Specifically, the return type annotation on an async function, if present, is used to communicate the custom Promise constructor function that should be used to create the async function's return values. So given the example:

async function foo(): MyPromise { /*...*/ }

The return type annotation MyPromise is used in the emitted code as a value that is passed to a call to the __awaiter helper function. The rationale for this is to support 'bring your own promise implementation' scenarios.

Consequences

That seems peculiar behaviour for TypeScript for a few reasons:

  • The return type annotation is used for its value rather than just its type. If MyPromise is just an interface describing a promise, the code won't compile. MyPromise must be a value. This is contrary to how return type annotations work elsewhere in the language, where their type is used, not their value. This can be surprising for users. async / await and PromiseLike #5911 is an example of that. Even annotating the return type as any generates an error, also surprising given this is otherwise a valid annotation for any function.
  • The return type annotation is used in the emitted code, so changing or erasing the return type annotation may change the emitted code. This seems to go against the TypeScript design goals such as goal 9: "Use a consistent, fully erasable, structural type system.", goal 7: "Preserve runtime behavior of all JavaScript code.", and non-goal 5 "Add or rely on run-time type information in programs, or emit different code based on the results of the type system.".
  • Soon when there's a target: ES7 flag and async functions can just be emitted as-is minus the type annotations, how will the custom promise constructor be retained in the emit? It seems extra code will have to be emitted to preserve the effect of the type annotation. This is contrary to most other features which tsc can just emit as-is minus type annotations when they are supported by the target.

Alternatives

In most 'bring your own promise implementation' code I've seen, the usual approach is to polyfill the value of Promise in scope either at the module level (commonJS/node) or globally (for browsers). (It could even be scoped to a function or block to handle multiple promise implementations in one project, although I've never seen need for this.)

Question

Why couldn't the emit for async functions just pass the in-scope Promise value to __awaiter? It supports custom promises and (a) preserves the otherwise consistent behaviour of the type system; (b) is probably closer to how it would typically be done in JavaScript; and (c) it could allow tsc with eventual target: ES7 to just pass async functions through minus type annotations, consistent with how it treats most other features that are not downleveled.

I understand the still-evolving spec hasn't yet finalised how to support custom promise implementations, but obviously it won't be using type annotations. It could be to just use the value of Promise in scope where an async function is declared, or it could be something else, but it will have to be a run-time mechanism.

I understand it would be a breaking change to do anything about this now, but given that the current mechanism is sure not to match whatever ES7 decides, and it introduces a number of inconsistencies, I thought it might be worth at least discussing.

@Arnavion
Copy link
Contributor

Arnavion commented Dec 6, 2015

(I just want to point out that if you don't have any return type annotation, the generated code uses Promise and thus assumes it's in scope. Whether that's a good thing or a bad thing I don't know.)

Soon when there's a target: ES7 flag and async functions can just be emitted as-is minus the type annotations, how will the custom promise constructor be retained in the emit?

Could it not be ignored? Would there be a reason not to assume native Promise exists in an ES7 environment?

In most 'bring your own promise implementation' code I've seen, the usual approach is to polyfill the value of Promise in scope either at the module level (commonJS/node) or globally (for browsers).

Why couldn't the emit for async functions just pass the in-scope Promise value to __awaiter?

Are you sure? The implementations I know of do not polyfill a global (by default). Instead they're imported explicitly, such as var Promise = require("bluebird");

As for assuming Promise exists in the module scope, it's certainly possible for someone to import it with a different name than Promise. It may be uncommon, but if there's absolutely a reason then it'll help if TS accomodates it as it does now.

@yortus
Copy link
Contributor Author

yortus commented Dec 6, 2015

Could it not be ignored? Would there be a reason not to assume native Promise exists in an ES7 environment?

The point of the return type annotation is to tell the compiler to emit code that does not use the native Promise type, whether it exists or not. It's not safe for tsc to assume it can silently change this behaviour and start emitting native promises for the same source code that previously emitted custom promises as directed by type annotations.

The implementations I know of do not polyfill a global (by default). Instead they're imported explicitly, such as var Promise = require("bluebird");

Yes, that's what I mean by "the usual approach is to polyfill the value of Promise in scope either at the module level (commonJS/node)".

it's certainly possible for someone to import it with a different name than Promise.

Sure they do, and it's no problem to bikeshed different solutions and their trade-offs. What do you think of the trade-offs of the current approach? I think before v1.7 TypeScript's type annotations were fully erasable (as per goal 9), but with this approach they can no longer be erased without possibly changing the emitted code and the program's runtime behahour. That's a pretty big trade-off IMO.

@Arnavion
Copy link
Contributor

Arnavion commented Dec 6, 2015

Yeah, it's not possible to ignore the type and start using native promises. The type annotation allows you to use properties of the returned promise, so it can't be substituted with native Promise at runtime.

interface Foo<T> {
    then<U>(onFulfilled: (value: T) => U, onRejected: (reason: any) => U): Foo<U>;
    bar: string;
}

declare var Foo: {
    new<T>(resolver: (fulfill: (value: T) => void, reject: (reason: any) => void) => void): Foo<T>;
};

async function foo(): Foo<number> {
  return null;
}

foo().bar; // Compiles

@yortus yortus changed the title Async functions and 'bring your own promise implementation' Async function return type annotations seem to break a few TS design goals Dec 7, 2015
@yortus
Copy link
Contributor Author

yortus commented Dec 7, 2015

Can someone from the team weigh in on this? Ping @rbuckton, @mhegazy, @ahejlsberg

@yortus
Copy link
Contributor Author

yortus commented Dec 9, 2015

* crickets *

Closing in favour of the less-wordy #6007

@yortus yortus closed this as completed Dec 9, 2015
@DanielRosenwasser DanielRosenwasser added the Discussion Issues which may not have code impact label Dec 9, 2015
@microsoft microsoft locked and limited conversation to collaborators Jun 19, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Discussion Issues which may not have code impact
Projects
None yet
Development

No branches or pull requests

3 participants