Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
90 changes: 82 additions & 8 deletions src/compiler/checker.ts
Original file line numberDiff line numberDiff line change
Expand Up@@ -2473,10 +2473,21 @@ namespace ts {

function getDeclarationContainer(node: Node): Node {
node = getRootDeclaration(node);
while (node) {
switch (node.kind) {
case SyntaxKind.VariableDeclaration:
case SyntaxKind.VariableDeclarationList:
case SyntaxKind.ImportSpecifier:
case SyntaxKind.NamedImports:
case SyntaxKind.NamespaceImport:
case SyntaxKind.ImportClause:
node = node.parent;
break;

// Parent chain:
// VaribleDeclaration -> VariableDeclarationList -> VariableStatement -> 'Declaration Container'
return node.kind === SyntaxKind.VariableDeclaration ? node.parent.parent.parent : node.parent;
default:
return node.parent;
}
}
}

function getTypeOfPrototypeProperty(prototype: Symbol): Type {
Expand DownExpand Up@@ -2618,7 +2629,7 @@ namespace ts {
}
}
else if (declaration.kind === SyntaxKind.Parameter) {
// If it's a parameter, see if the parent has a jsdoc comment with an @param
// If it's a parameter, see if the parent has a jsdoc comment with an @param
// annotation.
const paramTag = getCorrespondingJSDocParameterTag(<ParameterDeclaration>declaration);
if (paramTag && paramTag.typeExpression) {
Expand All@@ -2633,7 +2644,7 @@ namespace ts {
function getTypeForVariableLikeDeclaration(declaration: VariableLikeDeclaration): Type {
if (declaration.parserContextFlags & ParserContextFlags.JavaScriptFile) {
// If this is a variable in a JavaScript file, then use the JSDoc type (if it has
// one as its type), otherwise fallback to the below standard TS codepaths to
// one as its type), otherwise fallback to the below standard TS codepaths to
// try to figure it out.
const type = getTypeForVariableLikeDeclarationFromJSDocComment(declaration);
if (type && type !== unknownType) {
Expand DownExpand Up@@ -4058,7 +4069,7 @@ namespace ts {
const isJSConstructSignature = isJSDocConstructSignature(declaration);
let returnType: Type = undefined;

// If this is a JSDoc construct signature, then skip the first parameter in the
// If this is a JSDoc construct signature, then skip the first parameter in the
// parameter list. The first parameter represents the return type of the construct
// signature.
for (let i = isJSConstructSignature ? 1 : 0, n = declaration.parameters.length; i < n; i++) {
Expand DownExpand Up@@ -4461,7 +4472,7 @@ namespace ts {
}

if (symbol.flags & SymbolFlags.Value && node.kind === SyntaxKind.JSDocTypeReference) {
// A JSDocTypeReference may have resolved to a value (as opposed to a type). In
// A JSDocTypeReference may have resolved to a value (as opposed to a type). In
// that case, the type of this reference is just the type of the value we resolved
// to.
return getTypeOfSymbol(symbol);
Expand DownExpand Up@@ -10468,7 +10479,7 @@ namespace ts {

/*
*TypeScript Specification 1.0 (6.3) - July 2014
* An explicitly typed function whose return type isn't the Void type,
* An explicitly typed function whose return type isn't the Void type,
* the Any type, or a union type containing the Void or Any type as a constituent
* must have at least one return statement somewhere in its body.
* An exception to this rule is if the function implementation consists of a single 'throw' statement.
Expand DownExpand Up@@ -11625,6 +11636,9 @@ namespace ts {
checkTypeAssignableTo(iterableIteratorInstantiation, returnType, node.type);
}
}
else if (isAsyncFunctionLike(node)) {
checkAsyncFunctionReturnType(<FunctionLikeDeclaration>node);
}
}
}

Expand DownExpand Up@@ -12494,6 +12508,36 @@ namespace ts {
}
}

/**
* Checks that the return type provided is an instantiation of the global Promise<T> type

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What is the reason for the compiler to reject type annotations that are known compatible with Promise? This won't catch any type errors, but rejects valid annotations. Generator functions handle this consistently IMO. Why not use the same approach here for consistency?

function*bar(){}// Returns IterableIterator<any>function*bar2(): any{}// OK: compatible annotationfunction*bar3(): Iterator<any>{}// OK: comtatible annotationfunction*bar4(): {next}{}// OK: comtatible annotationasyncfunctionfoo(){}// Returns Promise<void>asyncfunctionfoo1(): any{}// ERROR with this PR, but why?asyncfunctionfoo2(): PromiseLike<any>{}// ERROR with this PR. but why?

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The checker.ts code for generator functions uses checkTypeAssignableTo to check the return type annotation (L11625). Why is checkTypeAssignableTo not used for async functions?

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is the result of an offline discussion with Mohamed Hegazy (@mhegazy). Supporting an assignable interface is not currently compatible with our goals for future down-level support for async functions for ES5. As a result, async functions will for the time being be more restrictive than generators with respect to the return type annotation.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ES3/5 restrictions need not be applied to ES6 code. Why not this:

if(languageVersion>=ScriptTarget.ES6){// use checkTypeAssignableTo to check return type annotation}else{// more restrictive return type annotation rules for ES3/5...}

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Mohamed Hegazy (@mhegazy) you gave qualitied support back in December for treating return type annotations one way for ES3/5 (improve the error message), and another way for ES6+ (use checkTypeAssignableTo). Is this still the case?

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The current support we have in TypeScript 1.7.x for async functions allows you to specify a custom Promise implementation in the return type annotation. This was intended to support runtimes that do not yet define a global Promise implementation, and was primarily intended to support down-level emit for async functions in ES5, which is a goal for TypeScript 2.0. This allows you to write an async function in the following fashion:

import*asbluebirdfrom'bluebird';exportasyncfunctionfn(): bluebird.Promise{}console.log(fn()instanceofbluebird.Promise);// true 

However, this approach is not compatible with the ES7 approach to async functions which only leverages the native Promise implementation. As this native Promise is specified as part of ES6, we have elected to only allow you to supply a global Promise<T> return type for ES6, but reserve the previous functionality to eventually support async functions down-level.

If we were to then allow you to specify the return type of an async function as any type to which Promise<T> is assignable, then the above sample would still compile successfully, due to how "bluebird.d.ts" is currently implemented on DefinitelyTyped. The result is that upgrading from TypeScript 1.7.x to TypeScript 1.8 would result in completely different runtime semantics with no indication to the developer that anything had changed.

import*asbluebirdfrom'bluebird';exportasyncfunctionfn(): bluebird.Promise{}console.log(fn()instanceofbluebird.Promise);// false?! 

By restricting the return type annotation to only the global Promise<T> type, we have the ability to loosen this restriction in the future. Yes, this does not fully align with how we support type annotations on generator functions; however, this is the only reliable way forward. We may, in a future release, opt to add a different mechanism for defining a compatible Promise implementation for ES5 async functions. If that does indeed become the case, we would be able to loosen the return type restriction.

As this is a breaking change from TypeScript 1.7 behavior, it also is best to be more conservative for one to two releases to ensure any existing implementations have adjusted to this change before examining the feasibility of relaxing this restriction.

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The fact that the return type must be strictly equal to Promise might become a hindrance in scenarios involving interfaces and/or methods overriding.

You can work around it by adding a layer of indirection (like all CS problems 😉) but it doesn't feel nice.

Here's one example: in frameworks such as Aurelia, many methods can return a value or a Promise for a value. Not mandating a Promise is both convenient when implementing trivial methods (like canSave() { return true; }) and good for perf.

Say I have a base class that exposes such a function and is meant to be overriden. Assume that the base default implementation is async.

The natural way is this:

classBaseViewModel{asyncisValid() : boolean|Promise<boolean>{/* some async implementation, maybe server call */}}classDerivedViewModelextendsBaseViewModel{isValid() : boolean|Promise<boolean>{returntrue;// Non-async implementation}}

but this wouldn't work :(

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

jods (@jods4) good real world example. Current behaviour:

varq: ()=>boolean|Promise<boolean>;q=()=>true;// OKq=()=>Promise.resolve(true);// OKq=async()=>true;// OK(): boolean|Promise<boolean>=>true;// OK(): boolean|Promise<boolean>=>Promise.resolve(true);// OKasync(): boolean|Promise<boolean>=>true;// ERROR

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

jods (@jods4) since this PR is done and closed may I suggest taking your example across to #6686 where I think it would be quite relevent.

* and returns the awaited type of the return type.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

since it is already jsdoc. could you add the parameter annotation as well?

*
* @param returnType The return type of a FunctionLikeDeclaration
* @param location The node on which to report the error.
*/
function checkCorrectPromiseType(returnType: Type, location: Node) {
if (returnType === unknownType) {
// The return type already had some other error, so we ignore and return
// the unknown type.
return unknownType;
}

const globalPromiseType = getGlobalPromiseType();
if (globalPromiseType === emptyGenericType
|| globalPromiseType === getTargetType(returnType)) {
// Either we couldn't resolve the global promise type, which would have already
// reported an error, or we could resolve it and the return type is a valid type
// reference to the global type. In either case, we return the awaited type for
// the return type.
return checkAwaitedType(returnType, location, Diagnostics.An_async_function_or_method_must_have_a_valid_awaitable_return_type);
}

// The promise type was not a valid type reference to the global promise type, so we
// report an error and return the unknown type.
error(location, Diagnostics.The_return_type_of_an_async_function_or_method_must_be_the_global_Promise_T_type);
return unknownType;
}

/**
* Checks the return type of an async function to ensure it is a compatible
* Promise implementation.
Expand All@@ -12508,6 +12552,11 @@ namespace ts {
* callable `then` signature.
*/
function checkAsyncFunctionReturnType(node: FunctionLikeDeclaration): Type {
if (languageVersion >= ScriptTarget.ES6) {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why >= ES6? I thought need to resolve global Promise when down-leveling?

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ron Buckton (@rbuckton) explains off-line. ES6 has native Promise. I misunderstood that fact

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ES5 and earlier do not have a global Promise. We fall back to the existing behavior for ES5 as we plan to support async functions via tree transformation for target ES5 and users will need to be able to supply a Promise implementation.

const returnType = getTypeFromTypeNode(node.type);
return checkCorrectPromiseType(returnType, node.type);
}

const globalPromiseConstructorLikeType = getGlobalPromiseConstructorLikeType();
if (globalPromiseConstructorLikeType === emptyObjectType) {
// If we couldn't resolve the global PromiseConstructorLike type we cannot verify
Expand DownExpand Up@@ -12723,6 +12772,7 @@ namespace ts {
checkCollisionWithCapturedSuperVariable(node, node.name);
checkCollisionWithCapturedThisVariable(node, node.name);
checkCollisionWithRequireExportsInGeneratedCode(node, node.name);
checkCollisionWithGlobalPromiseInGeneratedCode(node, node.name);
}
}

Expand DownExpand Up@@ -12907,6 +12957,25 @@ namespace ts {
}
}

function checkCollisionWithGlobalPromiseInGeneratedCode(node: Node, name: Identifier): void {
if (!needCollisionCheckForIdentifier(node, name, "Promise")) {
return;
}

// Uninstantiated modules shouldnt do this check
if (node.kind === SyntaxKind.ModuleDeclaration && getModuleInstanceState(node) !== ModuleInstanceState.Instantiated) {
return;
}

// In case of variable declaration, node.parent is variable statement so look at the variable statement's parent
const parent = getDeclarationContainer(node);
if (parent.kind === SyntaxKind.SourceFile && isExternalOrCommonJsModule(<SourceFile>parent) && parent.flags & NodeFlags.HasAsyncFunctions) {
// If the declaration happens to be in external module, report error that Promise is a reserved identifier.
error(name, Diagnostics.Duplicate_identifier_0_Compiler_reserves_name_1_in_top_level_scope_of_a_module_containing_async_functions,
declarationNameToString(name), declarationNameToString(name));
}
}

function checkVarDeclaredNamesNotShadowed(node: VariableDeclaration | BindingElement) {
// - ScriptBody : StatementList
// It is a Syntax Error if any element of the LexicallyDeclaredNames of StatementList
Expand DownExpand Up@@ -13085,6 +13154,7 @@ namespace ts {
checkCollisionWithCapturedSuperVariable(node, <Identifier>node.name);
checkCollisionWithCapturedThisVariable(node, <Identifier>node.name);
checkCollisionWithRequireExportsInGeneratedCode(node, <Identifier>node.name);
checkCollisionWithGlobalPromiseInGeneratedCode(node, <Identifier>node.name);
}
}

Expand DownExpand Up@@ -13850,6 +13920,7 @@ namespace ts {
checkTypeNameIsReserved(node.name, Diagnostics.Class_name_cannot_be_0);
checkCollisionWithCapturedThisVariable(node, node.name);
checkCollisionWithRequireExportsInGeneratedCode(node, node.name);
checkCollisionWithGlobalPromiseInGeneratedCode(node, node.name);
}
checkTypeParameters(node.typeParameters);
checkExportsOnMergedDeclarations(node);
Expand DownExpand Up@@ -14356,6 +14427,7 @@ namespace ts {
checkTypeNameIsReserved(node.name, Diagnostics.Enum_name_cannot_be_0);
checkCollisionWithCapturedThisVariable(node, node.name);
checkCollisionWithRequireExportsInGeneratedCode(node, node.name);
checkCollisionWithGlobalPromiseInGeneratedCode(node, node.name);
checkExportsOnMergedDeclarations(node);

computeEnumMemberValues(node);
Expand DownExpand Up@@ -14460,6 +14532,7 @@ namespace ts {

checkCollisionWithCapturedThisVariable(node, node.name);
checkCollisionWithRequireExportsInGeneratedCode(node, node.name);
checkCollisionWithGlobalPromiseInGeneratedCode(node, node.name);
checkExportsOnMergedDeclarations(node);
const symbol = getSymbolOfNode(node);

Expand DownExpand Up@@ -14652,6 +14725,7 @@ namespace ts {
function checkImportBinding(node: ImportEqualsDeclaration | ImportClause | NamespaceImport | ImportSpecifier) {
checkCollisionWithCapturedThisVariable(node, node.name);
checkCollisionWithRequireExportsInGeneratedCode(node, node.name);
checkCollisionWithGlobalPromiseInGeneratedCode(node, node.name);
checkAliasSymbol(node);
}

Expand Down
10 changes: 9 additions & 1 deletion src/compiler/diagnosticMessages.json
Original file line numberDiff line numberDiff line change
Expand Up@@ -195,6 +195,10 @@
"category": "Error",
"code": 1063
},
"The return type of an async function or method must be the global Promise<T> type.": {
"category": "Error",
"code": 1064
},
"In ambient enum declarations member initializer must be constant expression.": {
"category": "Error",
"code": 1066
Expand DownExpand Up@@ -794,7 +798,7 @@
"A decorator can only decorate a method implementation, not an overload.": {
"category": "Error",
"code": 1249
},
},
"'with' statements are not allowed in an async function block.": {
"category": "Error",
"code": 1300
Expand DownExpand Up@@ -1687,6 +1691,10 @@
"category": "Error",
"code": 2528
},
"Duplicate identifier '{0}'. Compiler reserves name '{1}' in top level scope of a module containing async functions.": {

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The compiler reserves '{1}' in the top level scope of of a module when using async functions.

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is modeled after TS2441: Duplicate identifier '{0}'. Compiler reserves name '{1}' in top level scope of a module. See: https://github.com/Microsoft/TypeScript/blob/master/src/compiler/diagnosticMessages.json#L1350

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we should change that as well then.

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These also?

If you want to change the various messages we use for these cases, I'd rather we do that in a different PR.

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Also "when using async functions" is too broad. It only enforces this check if you use async functions in the current module, hence the wording: "in the top level scope of a module containing async functions."

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let's just do that in a different PR like you mentioned.

"category": "Error",
"code": 2529
},
"JSX element attributes type '{0}' may not be a union type.": {
"category": "Error",
"code": 2600
Expand Down
8 changes: 4 additions & 4 deletions src/compiler/emitter.ts
Original file line numberDiff line numberDiff line change
Expand Up@@ -320,7 +320,7 @@ var __param = (this && this.__param) || function (paramIndex, decorator) {

const awaiterHelper = `
var __awaiter = (this && this.__awaiter) || function (thisArg, _arguments, P, generator) {
return new P(function (resolve, reject) {
return new (P || (P = Promise))(function (resolve, reject) {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why can't use always just do new Promise here?

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We are leaving support for custom Promise return types for ES5 and earlier which do not have a global Promise, so we want to keep the same argument list. The default behavior now will be to use the Promise captured in the same scope as __awaiter. This is to avoid issues when creating closures in namespaces:

// input.tsexportnamespacex{varPromise="Not a Promise constructor.";asyncfunctionf(): Promise{// references global Promise type}}// output.jsvar__awaiter= ...;// captures global Promise hereexportvarx=(function(x){varPromise="Not a Promise constructor.";// local Promise var shadows global Promisefunctionf(){// Before this change, we would capture Promise here, which would be the wrong Promise:// return __awaiter(this, void 0, Promise, function *() { });// After this change, we use the Promise captured at the top-level of the module.return__awaiter(this,void0,void0,function*(){});}})(x||(x={}));

function fulfilled(value) { try { step(generator.next(value)); } catch (e) { reject(e); } }
function rejected(value) { try { step(generator.throw(value)); } catch (e) { reject(e); } }
function step(result) { result.done ? resolve(result.value) : new P(function (resolve) { resolve(result.value); }).then(fulfilled, rejected); }
Expand DownExpand Up@@ -4531,11 +4531,11 @@ const _super = (function (geti, seti) {
write(", void 0, ");
}

if (promiseConstructor) {
emitEntityNameAsExpression(promiseConstructor, /*useFallback*/ false);
if (languageVersion >= ScriptTarget.ES6 || !promiseConstructor) {
write("void 0");
}
else {
write("Promise");
emitEntityNameAsExpression(promiseConstructor, /*useFallback*/ false);
}

// Emit the call to __awaiter.
Expand Down
10 changes: 0 additions & 10 deletions tests/baselines/reference/asyncAliasReturnType_es6.errors.txt

This file was deleted.

2 changes: 1 addition & 1 deletion tests/baselines/reference/asyncAliasReturnType_es6.js
Original file line numberDiff line numberDiff line change
Expand Up@@ -6,6 +6,6 @@ async function f(): PromiseAlias<void> {

//// [asyncAliasReturnType_es6.js]
function f() {
return __awaiter(this, void 0, PromiseAlias, function* () {
return __awaiter(this, void 0, void 0, function* () {
});
}
11 changes: 11 additions & 0 deletions tests/baselines/reference/asyncAliasReturnType_es6.symbols
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,11 @@
=== tests/cases/conformance/async/es6/asyncAliasReturnType_es6.ts ===
type PromiseAlias<T> = Promise<T>;
>PromiseAlias : Symbol(PromiseAlias, Decl(asyncAliasReturnType_es6.ts, 0, 0))
>T : Symbol(T, Decl(asyncAliasReturnType_es6.ts, 0, 18))
>Promise : Symbol(Promise, Decl(lib.d.ts, --, --), Decl(lib.d.ts, --, --))
>T : Symbol(T, Decl(asyncAliasReturnType_es6.ts, 0, 18))

async function f(): PromiseAlias<void> {
>f : Symbol(f, Decl(asyncAliasReturnType_es6.ts, 0, 34))
>PromiseAlias : Symbol(PromiseAlias, Decl(asyncAliasReturnType_es6.ts, 0, 0))
}
11 changes: 11 additions & 0 deletions tests/baselines/reference/asyncAliasReturnType_es6.types
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,11 @@
=== tests/cases/conformance/async/es6/asyncAliasReturnType_es6.ts ===
type PromiseAlias<T> = Promise<T>;
>PromiseAlias : Promise<T>
>T : T
>Promise : Promise<T>
>T : T

async function f(): PromiseAlias<void> {
>f : () => Promise<void>
>PromiseAlias : Promise<T>
}
2 changes: 1 addition & 1 deletion tests/baselines/reference/asyncArrowFunction1_es6.js
Original file line numberDiff line numberDiff line change
Expand Up@@ -4,5 +4,5 @@ var foo = async (): Promise<void> => {
};

//// [asyncArrowFunction1_es6.js]
var foo = () => __awaiter(this, void 0, Promise, function* () {
var foo = () => __awaiter(this, void 0, void 0, function* () {
});
2 changes: 1 addition & 1 deletion tests/baselines/reference/asyncArrowFunction6_es6.js
Original file line numberDiff line numberDiff line change
Expand Up@@ -4,5 +4,5 @@ var foo = async (a = await): Promise<void> => {
}

//// [asyncArrowFunction6_es6.js]
var foo = (a = yield ) => __awaiter(this, void 0, Promise, function* () {
var foo = (a = yield ) => __awaiter(this, void 0, void 0, function* () {
});
4 changes: 2 additions & 2 deletions tests/baselines/reference/asyncArrowFunction7_es6.js
Original file line numberDiff line numberDiff line change
Expand Up@@ -7,8 +7,8 @@ var bar = async (): Promise<void> => {
}

//// [asyncArrowFunction7_es6.js]
var bar = () => __awaiter(this, void 0, Promise, function* () {
var bar = () => __awaiter(this, void 0, void 0, function* () {
// 'await' here is an identifier, and not an await expression.
var foo = (a = yield ) => __awaiter(this, void 0, Promise, function* () {
var foo = (a = yield ) => __awaiter(this, void 0, void 0, function* () {
});
});
2 changes: 1 addition & 1 deletion tests/baselines/reference/asyncArrowFunction8_es6.js
Original file line numberDiff line numberDiff line change
Expand Up@@ -5,6 +5,6 @@ var foo = async (): Promise<void> => {
}

//// [asyncArrowFunction8_es6.js]
var foo = () => __awaiter(this, void 0, Promise, function* () {
var foo = () => __awaiter(this, void 0, void 0, function* () {
var v = { [yield ]: foo };
});
Original file line numberDiff line numberDiff line change
Expand Up@@ -11,6 +11,6 @@ class C {
class C {
method() {
function other() { }
var fn = () => __awaiter(this, arguments, Promise, function* () { return yield other.apply(this, arguments); });
var fn = () => __awaiter(this, arguments, void 0, function* () { return yield other.apply(this, arguments); });
}
}
Original file line numberDiff line numberDiff line change
Expand Up@@ -9,6 +9,6 @@ class C {
//// [asyncArrowFunctionCapturesThis_es6.js]
class C {
method() {
var fn = () => __awaiter(this, void 0, Promise, function* () { return yield this; });
var fn = () => __awaiter(this, void 0, void 0, function* () { return yield this; });
}
}
Loading
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Add copy buttons to all
 blocks\n(function() {\n function addCopyButtons() {\n document.querySelectorAll('pre code').forEach(function(codeBlock) {\n if (codeBlock.parentElement.hasAttribute('data-copy-added')) return;\n codeBlock.parentElement.setAttribute('data-copy-added', 'true');\n \n var btn = document.createElement('button');\n btn.textContent = 'Copy';\n btn.style.cssText = 'position:absolute;top:4px;right:4px;padding:2px 8px;font-size:11px;background:#4ecdc4;border:none;border-radius:4px;color:#1a1a2e;cursor:pointer;opacity:0.7;transition:opacity 0.2s;';\n btn.onmouseover = function() { this.style.opacity = '1'; };\n btn.onmouseout = function() { this.style.opacity = '0.7'; };\n btn.onclick = function() {\n navigator.clipboard.writeText(codeBlock.textContent).then(function() {\n btn.textContent = 'Copied!';\n setTimeout(function() { btn.textContent = 'Copy'; }, 1500);\n });\n };\n codeBlock.parentElement.style.position = 'relative';\n codeBlock.parentElement.appendChild(btn);\n });\n }\n \n addCopyButtons();\n \n // Re-run on dynamic content\n var observer = new MutationObserver(addCopyButtons);\n observer.observe(document.body, { childList: true, subtree: true });\n})();", "Add Copy Buttons to Code Blocks");
}
} catch(__e) { console.warn('[Userscript:Add Copy Buttons to Code Blocks]', __e); }
})();
(function(){
try {
var __m = "github.com";
var __re = new RegExp('^' + "github\\.com" + '
Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
90 changes: 82 additions & 8 deletions src/compiler/checker.ts
Original file line numberDiff line numberDiff line change
Expand Up@@ -2473,10 +2473,21 @@ namespace ts {

function getDeclarationContainer(node: Node): Node {
node = getRootDeclaration(node);
while (node) {
switch (node.kind) {
case SyntaxKind.VariableDeclaration:
case SyntaxKind.VariableDeclarationList:
case SyntaxKind.ImportSpecifier:
case SyntaxKind.NamedImports:
case SyntaxKind.NamespaceImport:
case SyntaxKind.ImportClause:
node = node.parent;
break;

// Parent chain:
// VaribleDeclaration -> VariableDeclarationList -> VariableStatement -> 'Declaration Container'
return node.kind === SyntaxKind.VariableDeclaration ? node.parent.parent.parent : node.parent;
default:
return node.parent;
}
}
}

function getTypeOfPrototypeProperty(prototype: Symbol): Type {
Expand DownExpand Up@@ -2618,7 +2629,7 @@ namespace ts {
}
}
else if (declaration.kind === SyntaxKind.Parameter) {
// If it's a parameter, see if the parent has a jsdoc comment with an @param
// If it's a parameter, see if the parent has a jsdoc comment with an @param
// annotation.
const paramTag = getCorrespondingJSDocParameterTag(<ParameterDeclaration>declaration);
if (paramTag && paramTag.typeExpression) {
Expand All@@ -2633,7 +2644,7 @@ namespace ts {
function getTypeForVariableLikeDeclaration(declaration: VariableLikeDeclaration): Type {
if (declaration.parserContextFlags & ParserContextFlags.JavaScriptFile) {
// If this is a variable in a JavaScript file, then use the JSDoc type (if it has
// one as its type), otherwise fallback to the below standard TS codepaths to
// one as its type), otherwise fallback to the below standard TS codepaths to
// try to figure it out.
const type = getTypeForVariableLikeDeclarationFromJSDocComment(declaration);
if (type && type !== unknownType) {
Expand DownExpand Up@@ -4058,7 +4069,7 @@ namespace ts {
const isJSConstructSignature = isJSDocConstructSignature(declaration);
let returnType: Type = undefined;

// If this is a JSDoc construct signature, then skip the first parameter in the
// If this is a JSDoc construct signature, then skip the first parameter in the
// parameter list. The first parameter represents the return type of the construct
// signature.
for (let i = isJSConstructSignature ? 1 : 0, n = declaration.parameters.length; i < n; i++) {
Expand DownExpand Up@@ -4461,7 +4472,7 @@ namespace ts {
}

if (symbol.flags & SymbolFlags.Value && node.kind === SyntaxKind.JSDocTypeReference) {
// A JSDocTypeReference may have resolved to a value (as opposed to a type). In
// A JSDocTypeReference may have resolved to a value (as opposed to a type). In
// that case, the type of this reference is just the type of the value we resolved
// to.
return getTypeOfSymbol(symbol);
Expand DownExpand Up@@ -10468,7 +10479,7 @@ namespace ts {

/*
*TypeScript Specification 1.0 (6.3) - July 2014
* An explicitly typed function whose return type isn't the Void type,
* An explicitly typed function whose return type isn't the Void type,
* the Any type, or a union type containing the Void or Any type as a constituent
* must have at least one return statement somewhere in its body.
* An exception to this rule is if the function implementation consists of a single 'throw' statement.
Expand DownExpand Up@@ -11625,6 +11636,9 @@ namespace ts {
checkTypeAssignableTo(iterableIteratorInstantiation, returnType, node.type);
}
}
else if (isAsyncFunctionLike(node)) {
checkAsyncFunctionReturnType(<FunctionLikeDeclaration>node);
}
}
}

Expand DownExpand Up@@ -12494,6 +12508,36 @@ namespace ts {
}
}

/**
* Checks that the return type provided is an instantiation of the global Promise<T> type

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What is the reason for the compiler to reject type annotations that are known compatible with Promise? This won't catch any type errors, but rejects valid annotations. Generator functions handle this consistently IMO. Why not use the same approach here for consistency?

function*bar(){}// Returns IterableIterator<any>function*bar2(): any{}// OK: compatible annotationfunction*bar3(): Iterator<any>{}// OK: comtatible annotationfunction*bar4(): {next}{}// OK: comtatible annotationasyncfunctionfoo(){}// Returns Promise<void>asyncfunctionfoo1(): any{}// ERROR with this PR, but why?asyncfunctionfoo2(): PromiseLike<any>{}// ERROR with this PR. but why?

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The checker.ts code for generator functions uses checkTypeAssignableTo to check the return type annotation (L11625). Why is checkTypeAssignableTo not used for async functions?

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is the result of an offline discussion with Mohamed Hegazy (@mhegazy). Supporting an assignable interface is not currently compatible with our goals for future down-level support for async functions for ES5. As a result, async functions will for the time being be more restrictive than generators with respect to the return type annotation.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ES3/5 restrictions need not be applied to ES6 code. Why not this:

if(languageVersion>=ScriptTarget.ES6){// use checkTypeAssignableTo to check return type annotation}else{// more restrictive return type annotation rules for ES3/5...}

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Mohamed Hegazy (@mhegazy) you gave qualitied support back in December for treating return type annotations one way for ES3/5 (improve the error message), and another way for ES6+ (use checkTypeAssignableTo). Is this still the case?

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The current support we have in TypeScript 1.7.x for async functions allows you to specify a custom Promise implementation in the return type annotation. This was intended to support runtimes that do not yet define a global Promise implementation, and was primarily intended to support down-level emit for async functions in ES5, which is a goal for TypeScript 2.0. This allows you to write an async function in the following fashion:

import*asbluebirdfrom'bluebird';exportasyncfunctionfn(): bluebird.Promise{}console.log(fn()instanceofbluebird.Promise);// true 

However, this approach is not compatible with the ES7 approach to async functions which only leverages the native Promise implementation. As this native Promise is specified as part of ES6, we have elected to only allow you to supply a global Promise<T> return type for ES6, but reserve the previous functionality to eventually support async functions down-level.

If we were to then allow you to specify the return type of an async function as any type to which Promise<T> is assignable, then the above sample would still compile successfully, due to how "bluebird.d.ts" is currently implemented on DefinitelyTyped. The result is that upgrading from TypeScript 1.7.x to TypeScript 1.8 would result in completely different runtime semantics with no indication to the developer that anything had changed.

import*asbluebirdfrom'bluebird';exportasyncfunctionfn(): bluebird.Promise{}console.log(fn()instanceofbluebird.Promise);// false?! 

By restricting the return type annotation to only the global Promise<T> type, we have the ability to loosen this restriction in the future. Yes, this does not fully align with how we support type annotations on generator functions; however, this is the only reliable way forward. We may, in a future release, opt to add a different mechanism for defining a compatible Promise implementation for ES5 async functions. If that does indeed become the case, we would be able to loosen the return type restriction.

As this is a breaking change from TypeScript 1.7 behavior, it also is best to be more conservative for one to two releases to ensure any existing implementations have adjusted to this change before examining the feasibility of relaxing this restriction.

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The fact that the return type must be strictly equal to Promise might become a hindrance in scenarios involving interfaces and/or methods overriding.

You can work around it by adding a layer of indirection (like all CS problems 😉) but it doesn't feel nice.

Here's one example: in frameworks such as Aurelia, many methods can return a value or a Promise for a value. Not mandating a Promise is both convenient when implementing trivial methods (like canSave() { return true; }) and good for perf.

Say I have a base class that exposes such a function and is meant to be overriden. Assume that the base default implementation is async.

The natural way is this:

classBaseViewModel{asyncisValid() : boolean|Promise<boolean>{/* some async implementation, maybe server call */}}classDerivedViewModelextendsBaseViewModel{isValid() : boolean|Promise<boolean>{returntrue;// Non-async implementation}}

but this wouldn't work :(

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

jods (@jods4) good real world example. Current behaviour:

varq: ()=>boolean|Promise<boolean>;q=()=>true;// OKq=()=>Promise.resolve(true);// OKq=async()=>true;// OK(): boolean|Promise<boolean>=>true;// OK(): boolean|Promise<boolean>=>Promise.resolve(true);// OKasync(): boolean|Promise<boolean>=>true;// ERROR

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

jods (@jods4) since this PR is done and closed may I suggest taking your example across to #6686 where I think it would be quite relevent.

* and returns the awaited type of the return type.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

since it is already jsdoc. could you add the parameter annotation as well?

*
* @param returnType The return type of a FunctionLikeDeclaration
* @param location The node on which to report the error.
*/
function checkCorrectPromiseType(returnType: Type, location: Node) {
if (returnType === unknownType) {
// The return type already had some other error, so we ignore and return
// the unknown type.
return unknownType;
}

const globalPromiseType = getGlobalPromiseType();
if (globalPromiseType === emptyGenericType
|| globalPromiseType === getTargetType(returnType)) {
// Either we couldn't resolve the global promise type, which would have already
// reported an error, or we could resolve it and the return type is a valid type
// reference to the global type. In either case, we return the awaited type for
// the return type.
return checkAwaitedType(returnType, location, Diagnostics.An_async_function_or_method_must_have_a_valid_awaitable_return_type);
}

// The promise type was not a valid type reference to the global promise type, so we
// report an error and return the unknown type.
error(location, Diagnostics.The_return_type_of_an_async_function_or_method_must_be_the_global_Promise_T_type);
return unknownType;
}

/**
* Checks the return type of an async function to ensure it is a compatible
* Promise implementation.
Expand All@@ -12508,6 +12552,11 @@ namespace ts {
* callable `then` signature.
*/
function checkAsyncFunctionReturnType(node: FunctionLikeDeclaration): Type {
if (languageVersion >= ScriptTarget.ES6) {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why >= ES6? I thought need to resolve global Promise when down-leveling?

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ron Buckton (@rbuckton) explains off-line. ES6 has native Promise. I misunderstood that fact

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ES5 and earlier do not have a global Promise. We fall back to the existing behavior for ES5 as we plan to support async functions via tree transformation for target ES5 and users will need to be able to supply a Promise implementation.

const returnType = getTypeFromTypeNode(node.type);
return checkCorrectPromiseType(returnType, node.type);
}

const globalPromiseConstructorLikeType = getGlobalPromiseConstructorLikeType();
if (globalPromiseConstructorLikeType === emptyObjectType) {
// If we couldn't resolve the global PromiseConstructorLike type we cannot verify
Expand DownExpand Up@@ -12723,6 +12772,7 @@ namespace ts {
checkCollisionWithCapturedSuperVariable(node, node.name);
checkCollisionWithCapturedThisVariable(node, node.name);
checkCollisionWithRequireExportsInGeneratedCode(node, node.name);
checkCollisionWithGlobalPromiseInGeneratedCode(node, node.name);
}
}

Expand DownExpand Up@@ -12907,6 +12957,25 @@ namespace ts {
}
}

function checkCollisionWithGlobalPromiseInGeneratedCode(node: Node, name: Identifier): void {
if (!needCollisionCheckForIdentifier(node, name, "Promise")) {
return;
}

// Uninstantiated modules shouldnt do this check
if (node.kind === SyntaxKind.ModuleDeclaration && getModuleInstanceState(node) !== ModuleInstanceState.Instantiated) {
return;
}

// In case of variable declaration, node.parent is variable statement so look at the variable statement's parent
const parent = getDeclarationContainer(node);
if (parent.kind === SyntaxKind.SourceFile && isExternalOrCommonJsModule(<SourceFile>parent) && parent.flags & NodeFlags.HasAsyncFunctions) {
// If the declaration happens to be in external module, report error that Promise is a reserved identifier.
error(name, Diagnostics.Duplicate_identifier_0_Compiler_reserves_name_1_in_top_level_scope_of_a_module_containing_async_functions,
declarationNameToString(name), declarationNameToString(name));
}
}

function checkVarDeclaredNamesNotShadowed(node: VariableDeclaration | BindingElement) {
// - ScriptBody : StatementList
// It is a Syntax Error if any element of the LexicallyDeclaredNames of StatementList
Expand DownExpand Up@@ -13085,6 +13154,7 @@ namespace ts {
checkCollisionWithCapturedSuperVariable(node, <Identifier>node.name);
checkCollisionWithCapturedThisVariable(node, <Identifier>node.name);
checkCollisionWithRequireExportsInGeneratedCode(node, <Identifier>node.name);
checkCollisionWithGlobalPromiseInGeneratedCode(node, <Identifier>node.name);
}
}

Expand DownExpand Up@@ -13850,6 +13920,7 @@ namespace ts {
checkTypeNameIsReserved(node.name, Diagnostics.Class_name_cannot_be_0);
checkCollisionWithCapturedThisVariable(node, node.name);
checkCollisionWithRequireExportsInGeneratedCode(node, node.name);
checkCollisionWithGlobalPromiseInGeneratedCode(node, node.name);
}
checkTypeParameters(node.typeParameters);
checkExportsOnMergedDeclarations(node);
Expand DownExpand Up@@ -14356,6 +14427,7 @@ namespace ts {
checkTypeNameIsReserved(node.name, Diagnostics.Enum_name_cannot_be_0);
checkCollisionWithCapturedThisVariable(node, node.name);
checkCollisionWithRequireExportsInGeneratedCode(node, node.name);
checkCollisionWithGlobalPromiseInGeneratedCode(node, node.name);
checkExportsOnMergedDeclarations(node);

computeEnumMemberValues(node);
Expand DownExpand Up@@ -14460,6 +14532,7 @@ namespace ts {

checkCollisionWithCapturedThisVariable(node, node.name);
checkCollisionWithRequireExportsInGeneratedCode(node, node.name);
checkCollisionWithGlobalPromiseInGeneratedCode(node, node.name);
checkExportsOnMergedDeclarations(node);
const symbol = getSymbolOfNode(node);

Expand DownExpand Up@@ -14652,6 +14725,7 @@ namespace ts {
function checkImportBinding(node: ImportEqualsDeclaration | ImportClause | NamespaceImport | ImportSpecifier) {
checkCollisionWithCapturedThisVariable(node, node.name);
checkCollisionWithRequireExportsInGeneratedCode(node, node.name);
checkCollisionWithGlobalPromiseInGeneratedCode(node, node.name);
checkAliasSymbol(node);
}

Expand Down
10 changes: 9 additions & 1 deletion src/compiler/diagnosticMessages.json
Original file line numberDiff line numberDiff line change
Expand Up@@ -195,6 +195,10 @@
"category": "Error",
"code": 1063
},
"The return type of an async function or method must be the global Promise<T> type.": {
"category": "Error",
"code": 1064
},
"In ambient enum declarations member initializer must be constant expression.": {
"category": "Error",
"code": 1066
Expand DownExpand Up@@ -794,7 +798,7 @@
"A decorator can only decorate a method implementation, not an overload.": {
"category": "Error",
"code": 1249
},
},
"'with' statements are not allowed in an async function block.": {
"category": "Error",
"code": 1300
Expand DownExpand Up@@ -1687,6 +1691,10 @@
"category": "Error",
"code": 2528
},
"Duplicate identifier '{0}'. Compiler reserves name '{1}' in top level scope of a module containing async functions.": {

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The compiler reserves '{1}' in the top level scope of of a module when using async functions.

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is modeled after TS2441: Duplicate identifier '{0}'. Compiler reserves name '{1}' in top level scope of a module. See: https://github.com/Microsoft/TypeScript/blob/master/src/compiler/diagnosticMessages.json#L1350

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we should change that as well then.

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These also?

If you want to change the various messages we use for these cases, I'd rather we do that in a different PR.

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Also "when using async functions" is too broad. It only enforces this check if you use async functions in the current module, hence the wording: "in the top level scope of a module containing async functions."

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let's just do that in a different PR like you mentioned.

"category": "Error",
"code": 2529
},
"JSX element attributes type '{0}' may not be a union type.": {
"category": "Error",
"code": 2600
Expand Down
8 changes: 4 additions & 4 deletions src/compiler/emitter.ts
Original file line numberDiff line numberDiff line change
Expand Up@@ -320,7 +320,7 @@ var __param = (this && this.__param) || function (paramIndex, decorator) {

const awaiterHelper = `
var __awaiter = (this && this.__awaiter) || function (thisArg, _arguments, P, generator) {
return new P(function (resolve, reject) {
return new (P || (P = Promise))(function (resolve, reject) {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why can't use always just do new Promise here?

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We are leaving support for custom Promise return types for ES5 and earlier which do not have a global Promise, so we want to keep the same argument list. The default behavior now will be to use the Promise captured in the same scope as __awaiter. This is to avoid issues when creating closures in namespaces:

// input.tsexportnamespacex{varPromise="Not a Promise constructor.";asyncfunctionf(): Promise{// references global Promise type}}// output.jsvar__awaiter= ...;// captures global Promise hereexportvarx=(function(x){varPromise="Not a Promise constructor.";// local Promise var shadows global Promisefunctionf(){// Before this change, we would capture Promise here, which would be the wrong Promise:// return __awaiter(this, void 0, Promise, function *() { });// After this change, we use the Promise captured at the top-level of the module.return__awaiter(this,void0,void0,function*(){});}})(x||(x={}));

function fulfilled(value) { try { step(generator.next(value)); } catch (e) { reject(e); } }
function rejected(value) { try { step(generator.throw(value)); } catch (e) { reject(e); } }
function step(result) { result.done ? resolve(result.value) : new P(function (resolve) { resolve(result.value); }).then(fulfilled, rejected); }
Expand DownExpand Up@@ -4531,11 +4531,11 @@ const _super = (function (geti, seti) {
write(", void 0, ");
}

if (promiseConstructor) {
emitEntityNameAsExpression(promiseConstructor, /*useFallback*/ false);
if (languageVersion >= ScriptTarget.ES6 || !promiseConstructor) {
write("void 0");
}
else {
write("Promise");
emitEntityNameAsExpression(promiseConstructor, /*useFallback*/ false);
}

// Emit the call to __awaiter.
Expand Down
10 changes: 0 additions & 10 deletions tests/baselines/reference/asyncAliasReturnType_es6.errors.txt

This file was deleted.

2 changes: 1 addition & 1 deletion tests/baselines/reference/asyncAliasReturnType_es6.js
Original file line numberDiff line numberDiff line change
Expand Up@@ -6,6 +6,6 @@ async function f(): PromiseAlias<void> {

//// [asyncAliasReturnType_es6.js]
function f() {
return __awaiter(this, void 0, PromiseAlias, function* () {
return __awaiter(this, void 0, void 0, function* () {
});
}
11 changes: 11 additions & 0 deletions tests/baselines/reference/asyncAliasReturnType_es6.symbols
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,11 @@
=== tests/cases/conformance/async/es6/asyncAliasReturnType_es6.ts ===
type PromiseAlias<T> = Promise<T>;
>PromiseAlias : Symbol(PromiseAlias, Decl(asyncAliasReturnType_es6.ts, 0, 0))
>T : Symbol(T, Decl(asyncAliasReturnType_es6.ts, 0, 18))
>Promise : Symbol(Promise, Decl(lib.d.ts, --, --), Decl(lib.d.ts, --, --))
>T : Symbol(T, Decl(asyncAliasReturnType_es6.ts, 0, 18))

async function f(): PromiseAlias<void> {
>f : Symbol(f, Decl(asyncAliasReturnType_es6.ts, 0, 34))
>PromiseAlias : Symbol(PromiseAlias, Decl(asyncAliasReturnType_es6.ts, 0, 0))
}
11 changes: 11 additions & 0 deletions tests/baselines/reference/asyncAliasReturnType_es6.types
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,11 @@
=== tests/cases/conformance/async/es6/asyncAliasReturnType_es6.ts ===
type PromiseAlias<T> = Promise<T>;
>PromiseAlias : Promise<T>
>T : T
>Promise : Promise<T>
>T : T

async function f(): PromiseAlias<void> {
>f : () => Promise<void>
>PromiseAlias : Promise<T>
}
2 changes: 1 addition & 1 deletion tests/baselines/reference/asyncArrowFunction1_es6.js
Original file line numberDiff line numberDiff line change
Expand Up@@ -4,5 +4,5 @@ var foo = async (): Promise<void> => {
};

//// [asyncArrowFunction1_es6.js]
var foo = () => __awaiter(this, void 0, Promise, function* () {
var foo = () => __awaiter(this, void 0, void 0, function* () {
});
2 changes: 1 addition & 1 deletion tests/baselines/reference/asyncArrowFunction6_es6.js
Original file line numberDiff line numberDiff line change
Expand Up@@ -4,5 +4,5 @@ var foo = async (a = await): Promise<void> => {
}

//// [asyncArrowFunction6_es6.js]
var foo = (a = yield ) => __awaiter(this, void 0, Promise, function* () {
var foo = (a = yield ) => __awaiter(this, void 0, void 0, function* () {
});
4 changes: 2 additions & 2 deletions tests/baselines/reference/asyncArrowFunction7_es6.js
Original file line numberDiff line numberDiff line change
Expand Up@@ -7,8 +7,8 @@ var bar = async (): Promise<void> => {
}

//// [asyncArrowFunction7_es6.js]
var bar = () => __awaiter(this, void 0, Promise, function* () {
var bar = () => __awaiter(this, void 0, void 0, function* () {
// 'await' here is an identifier, and not an await expression.
var foo = (a = yield ) => __awaiter(this, void 0, Promise, function* () {
var foo = (a = yield ) => __awaiter(this, void 0, void 0, function* () {
});
});
2 changes: 1 addition & 1 deletion tests/baselines/reference/asyncArrowFunction8_es6.js
Original file line numberDiff line numberDiff line change
Expand Up@@ -5,6 +5,6 @@ var foo = async (): Promise<void> => {
}

//// [asyncArrowFunction8_es6.js]
var foo = () => __awaiter(this, void 0, Promise, function* () {
var foo = () => __awaiter(this, void 0, void 0, function* () {
var v = { [yield ]: foo };
});
Original file line numberDiff line numberDiff line change
Expand Up@@ -11,6 +11,6 @@ class C {
class C {
method() {
function other() { }
var fn = () => __awaiter(this, arguments, Promise, function* () { return yield other.apply(this, arguments); });
var fn = () => __awaiter(this, arguments, void 0, function* () { return yield other.apply(this, arguments); });
}
}
Original file line numberDiff line numberDiff line change
Expand Up@@ -9,6 +9,6 @@ class C {
//// [asyncArrowFunctionCapturesThis_es6.js]
class C {
method() {
var fn = () => __awaiter(this, void 0, Promise, function* () { return yield this; });
var fn = () => __awaiter(this, void 0, void 0, function* () { return yield this; });
}
}
Loading
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Force GitHub README to respect dark mode\n(function() {\n var style = document.createElement('style');\n style.textContent = '\n .markdown-body {\n color-scheme: dark light;\n }\n .markdown-body pre { background: #161b22 !important; }\n .markdown-body code { background: rgba(110, 118, 129, 0.4) !important; }\n .markdown-body table th, .markdown-body table td { border-color: #30363d !important; }\n .markdown-body img { background: #0d1117; }\n .markdown-body blockquote { border-left-color: #8b949e; }\n .markdown-body hr { border-color: #30363d; }\n ';\n document.head.appendChild(style);\n})();", "GitHub Dark Mode README Fix"); } } catch(__e) { console.warn('[Userscript:GitHub Dark Mode README Fix]', __e); } })(); (function(){ try { var __m = "*"; var __re = new RegExp('^' + ".*" + '
Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
90 changes: 82 additions & 8 deletions src/compiler/checker.ts
Original file line numberDiff line numberDiff line change
Expand Up@@ -2473,10 +2473,21 @@ namespace ts {

function getDeclarationContainer(node: Node): Node {
node = getRootDeclaration(node);
while (node) {
switch (node.kind) {
case SyntaxKind.VariableDeclaration:
case SyntaxKind.VariableDeclarationList:
case SyntaxKind.ImportSpecifier:
case SyntaxKind.NamedImports:
case SyntaxKind.NamespaceImport:
case SyntaxKind.ImportClause:
node = node.parent;
break;

// Parent chain:
// VaribleDeclaration -> VariableDeclarationList -> VariableStatement -> 'Declaration Container'
return node.kind === SyntaxKind.VariableDeclaration ? node.parent.parent.parent : node.parent;
default:
return node.parent;
}
}
}

function getTypeOfPrototypeProperty(prototype: Symbol): Type {
Expand DownExpand Up@@ -2618,7 +2629,7 @@ namespace ts {
}
}
else if (declaration.kind === SyntaxKind.Parameter) {
// If it's a parameter, see if the parent has a jsdoc comment with an @param
// If it's a parameter, see if the parent has a jsdoc comment with an @param
// annotation.
const paramTag = getCorrespondingJSDocParameterTag(<ParameterDeclaration>declaration);
if (paramTag && paramTag.typeExpression) {
Expand All@@ -2633,7 +2644,7 @@ namespace ts {
function getTypeForVariableLikeDeclaration(declaration: VariableLikeDeclaration): Type {
if (declaration.parserContextFlags & ParserContextFlags.JavaScriptFile) {
// If this is a variable in a JavaScript file, then use the JSDoc type (if it has
// one as its type), otherwise fallback to the below standard TS codepaths to
// one as its type), otherwise fallback to the below standard TS codepaths to
// try to figure it out.
const type = getTypeForVariableLikeDeclarationFromJSDocComment(declaration);
if (type && type !== unknownType) {
Expand DownExpand Up@@ -4058,7 +4069,7 @@ namespace ts {
const isJSConstructSignature = isJSDocConstructSignature(declaration);
let returnType: Type = undefined;

// If this is a JSDoc construct signature, then skip the first parameter in the
// If this is a JSDoc construct signature, then skip the first parameter in the
// parameter list. The first parameter represents the return type of the construct
// signature.
for (let i = isJSConstructSignature ? 1 : 0, n = declaration.parameters.length; i < n; i++) {
Expand DownExpand Up@@ -4461,7 +4472,7 @@ namespace ts {
}

if (symbol.flags & SymbolFlags.Value && node.kind === SyntaxKind.JSDocTypeReference) {
// A JSDocTypeReference may have resolved to a value (as opposed to a type). In
// A JSDocTypeReference may have resolved to a value (as opposed to a type). In
// that case, the type of this reference is just the type of the value we resolved
// to.
return getTypeOfSymbol(symbol);
Expand DownExpand Up@@ -10468,7 +10479,7 @@ namespace ts {

/*
*TypeScript Specification 1.0 (6.3) - July 2014
* An explicitly typed function whose return type isn't the Void type,
* An explicitly typed function whose return type isn't the Void type,
* the Any type, or a union type containing the Void or Any type as a constituent
* must have at least one return statement somewhere in its body.
* An exception to this rule is if the function implementation consists of a single 'throw' statement.
Expand DownExpand Up@@ -11625,6 +11636,9 @@ namespace ts {
checkTypeAssignableTo(iterableIteratorInstantiation, returnType, node.type);
}
}
else if (isAsyncFunctionLike(node)) {
checkAsyncFunctionReturnType(<FunctionLikeDeclaration>node);
}
}
}

Expand DownExpand Up@@ -12494,6 +12508,36 @@ namespace ts {
}
}

/**
* Checks that the return type provided is an instantiation of the global Promise<T> type

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What is the reason for the compiler to reject type annotations that are known compatible with Promise? This won't catch any type errors, but rejects valid annotations. Generator functions handle this consistently IMO. Why not use the same approach here for consistency?

function*bar(){}// Returns IterableIterator<any>function*bar2(): any{}// OK: compatible annotationfunction*bar3(): Iterator<any>{}// OK: comtatible annotationfunction*bar4(): {next}{}// OK: comtatible annotationasyncfunctionfoo(){}// Returns Promise<void>asyncfunctionfoo1(): any{}// ERROR with this PR, but why?asyncfunctionfoo2(): PromiseLike<any>{}// ERROR with this PR. but why?

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The checker.ts code for generator functions uses checkTypeAssignableTo to check the return type annotation (L11625). Why is checkTypeAssignableTo not used for async functions?

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is the result of an offline discussion with Mohamed Hegazy (@mhegazy). Supporting an assignable interface is not currently compatible with our goals for future down-level support for async functions for ES5. As a result, async functions will for the time being be more restrictive than generators with respect to the return type annotation.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ES3/5 restrictions need not be applied to ES6 code. Why not this:

if(languageVersion>=ScriptTarget.ES6){// use checkTypeAssignableTo to check return type annotation}else{// more restrictive return type annotation rules for ES3/5...}

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Mohamed Hegazy (@mhegazy) you gave qualitied support back in December for treating return type annotations one way for ES3/5 (improve the error message), and another way for ES6+ (use checkTypeAssignableTo). Is this still the case?

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The current support we have in TypeScript 1.7.x for async functions allows you to specify a custom Promise implementation in the return type annotation. This was intended to support runtimes that do not yet define a global Promise implementation, and was primarily intended to support down-level emit for async functions in ES5, which is a goal for TypeScript 2.0. This allows you to write an async function in the following fashion:

import*asbluebirdfrom'bluebird';exportasyncfunctionfn(): bluebird.Promise{}console.log(fn()instanceofbluebird.Promise);// true 

However, this approach is not compatible with the ES7 approach to async functions which only leverages the native Promise implementation. As this native Promise is specified as part of ES6, we have elected to only allow you to supply a global Promise<T> return type for ES6, but reserve the previous functionality to eventually support async functions down-level.

If we were to then allow you to specify the return type of an async function as any type to which Promise<T> is assignable, then the above sample would still compile successfully, due to how "bluebird.d.ts" is currently implemented on DefinitelyTyped. The result is that upgrading from TypeScript 1.7.x to TypeScript 1.8 would result in completely different runtime semantics with no indication to the developer that anything had changed.

import*asbluebirdfrom'bluebird';exportasyncfunctionfn(): bluebird.Promise{}console.log(fn()instanceofbluebird.Promise);// false?! 

By restricting the return type annotation to only the global Promise<T> type, we have the ability to loosen this restriction in the future. Yes, this does not fully align with how we support type annotations on generator functions; however, this is the only reliable way forward. We may, in a future release, opt to add a different mechanism for defining a compatible Promise implementation for ES5 async functions. If that does indeed become the case, we would be able to loosen the return type restriction.

As this is a breaking change from TypeScript 1.7 behavior, it also is best to be more conservative for one to two releases to ensure any existing implementations have adjusted to this change before examining the feasibility of relaxing this restriction.

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The fact that the return type must be strictly equal to Promise might become a hindrance in scenarios involving interfaces and/or methods overriding.

You can work around it by adding a layer of indirection (like all CS problems 😉) but it doesn't feel nice.

Here's one example: in frameworks such as Aurelia, many methods can return a value or a Promise for a value. Not mandating a Promise is both convenient when implementing trivial methods (like canSave() { return true; }) and good for perf.

Say I have a base class that exposes such a function and is meant to be overriden. Assume that the base default implementation is async.

The natural way is this:

classBaseViewModel{asyncisValid() : boolean|Promise<boolean>{/* some async implementation, maybe server call */}}classDerivedViewModelextendsBaseViewModel{isValid() : boolean|Promise<boolean>{returntrue;// Non-async implementation}}

but this wouldn't work :(

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

jods (@jods4) good real world example. Current behaviour:

varq: ()=>boolean|Promise<boolean>;q=()=>true;// OKq=()=>Promise.resolve(true);// OKq=async()=>true;// OK(): boolean|Promise<boolean>=>true;// OK(): boolean|Promise<boolean>=>Promise.resolve(true);// OKasync(): boolean|Promise<boolean>=>true;// ERROR

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

jods (@jods4) since this PR is done and closed may I suggest taking your example across to #6686 where I think it would be quite relevent.

* and returns the awaited type of the return type.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

since it is already jsdoc. could you add the parameter annotation as well?

*
* @param returnType The return type of a FunctionLikeDeclaration
* @param location The node on which to report the error.
*/
function checkCorrectPromiseType(returnType: Type, location: Node) {
if (returnType === unknownType) {
// The return type already had some other error, so we ignore and return
// the unknown type.
return unknownType;
}

const globalPromiseType = getGlobalPromiseType();
if (globalPromiseType === emptyGenericType
|| globalPromiseType === getTargetType(returnType)) {
// Either we couldn't resolve the global promise type, which would have already
// reported an error, or we could resolve it and the return type is a valid type
// reference to the global type. In either case, we return the awaited type for
// the return type.
return checkAwaitedType(returnType, location, Diagnostics.An_async_function_or_method_must_have_a_valid_awaitable_return_type);
}

// The promise type was not a valid type reference to the global promise type, so we
// report an error and return the unknown type.
error(location, Diagnostics.The_return_type_of_an_async_function_or_method_must_be_the_global_Promise_T_type);
return unknownType;
}

/**
* Checks the return type of an async function to ensure it is a compatible
* Promise implementation.
Expand All@@ -12508,6 +12552,11 @@ namespace ts {
* callable `then` signature.
*/
function checkAsyncFunctionReturnType(node: FunctionLikeDeclaration): Type {
if (languageVersion >= ScriptTarget.ES6) {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why >= ES6? I thought need to resolve global Promise when down-leveling?

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ron Buckton (@rbuckton) explains off-line. ES6 has native Promise. I misunderstood that fact

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ES5 and earlier do not have a global Promise. We fall back to the existing behavior for ES5 as we plan to support async functions via tree transformation for target ES5 and users will need to be able to supply a Promise implementation.

const returnType = getTypeFromTypeNode(node.type);
return checkCorrectPromiseType(returnType, node.type);
}

const globalPromiseConstructorLikeType = getGlobalPromiseConstructorLikeType();
if (globalPromiseConstructorLikeType === emptyObjectType) {
// If we couldn't resolve the global PromiseConstructorLike type we cannot verify
Expand DownExpand Up@@ -12723,6 +12772,7 @@ namespace ts {
checkCollisionWithCapturedSuperVariable(node, node.name);
checkCollisionWithCapturedThisVariable(node, node.name);
checkCollisionWithRequireExportsInGeneratedCode(node, node.name);
checkCollisionWithGlobalPromiseInGeneratedCode(node, node.name);
}
}

Expand DownExpand Up@@ -12907,6 +12957,25 @@ namespace ts {
}
}

function checkCollisionWithGlobalPromiseInGeneratedCode(node: Node, name: Identifier): void {
if (!needCollisionCheckForIdentifier(node, name, "Promise")) {
return;
}

// Uninstantiated modules shouldnt do this check
if (node.kind === SyntaxKind.ModuleDeclaration && getModuleInstanceState(node) !== ModuleInstanceState.Instantiated) {
return;
}

// In case of variable declaration, node.parent is variable statement so look at the variable statement's parent
const parent = getDeclarationContainer(node);
if (parent.kind === SyntaxKind.SourceFile && isExternalOrCommonJsModule(<SourceFile>parent) && parent.flags & NodeFlags.HasAsyncFunctions) {
// If the declaration happens to be in external module, report error that Promise is a reserved identifier.
error(name, Diagnostics.Duplicate_identifier_0_Compiler_reserves_name_1_in_top_level_scope_of_a_module_containing_async_functions,
declarationNameToString(name), declarationNameToString(name));
}
}

function checkVarDeclaredNamesNotShadowed(node: VariableDeclaration | BindingElement) {
// - ScriptBody : StatementList
// It is a Syntax Error if any element of the LexicallyDeclaredNames of StatementList
Expand DownExpand Up@@ -13085,6 +13154,7 @@ namespace ts {
checkCollisionWithCapturedSuperVariable(node, <Identifier>node.name);
checkCollisionWithCapturedThisVariable(node, <Identifier>node.name);
checkCollisionWithRequireExportsInGeneratedCode(node, <Identifier>node.name);
checkCollisionWithGlobalPromiseInGeneratedCode(node, <Identifier>node.name);
}
}

Expand DownExpand Up@@ -13850,6 +13920,7 @@ namespace ts {
checkTypeNameIsReserved(node.name, Diagnostics.Class_name_cannot_be_0);
checkCollisionWithCapturedThisVariable(node, node.name);
checkCollisionWithRequireExportsInGeneratedCode(node, node.name);
checkCollisionWithGlobalPromiseInGeneratedCode(node, node.name);
}
checkTypeParameters(node.typeParameters);
checkExportsOnMergedDeclarations(node);
Expand DownExpand Up@@ -14356,6 +14427,7 @@ namespace ts {
checkTypeNameIsReserved(node.name, Diagnostics.Enum_name_cannot_be_0);
checkCollisionWithCapturedThisVariable(node, node.name);
checkCollisionWithRequireExportsInGeneratedCode(node, node.name);
checkCollisionWithGlobalPromiseInGeneratedCode(node, node.name);
checkExportsOnMergedDeclarations(node);

computeEnumMemberValues(node);
Expand DownExpand Up@@ -14460,6 +14532,7 @@ namespace ts {

checkCollisionWithCapturedThisVariable(node, node.name);
checkCollisionWithRequireExportsInGeneratedCode(node, node.name);
checkCollisionWithGlobalPromiseInGeneratedCode(node, node.name);
checkExportsOnMergedDeclarations(node);
const symbol = getSymbolOfNode(node);

Expand DownExpand Up@@ -14652,6 +14725,7 @@ namespace ts {
function checkImportBinding(node: ImportEqualsDeclaration | ImportClause | NamespaceImport | ImportSpecifier) {
checkCollisionWithCapturedThisVariable(node, node.name);
checkCollisionWithRequireExportsInGeneratedCode(node, node.name);
checkCollisionWithGlobalPromiseInGeneratedCode(node, node.name);
checkAliasSymbol(node);
}

Expand Down
10 changes: 9 additions & 1 deletion src/compiler/diagnosticMessages.json
Original file line numberDiff line numberDiff line change
Expand Up@@ -195,6 +195,10 @@
"category": "Error",
"code": 1063
},
"The return type of an async function or method must be the global Promise<T> type.": {
"category": "Error",
"code": 1064
},
"In ambient enum declarations member initializer must be constant expression.": {
"category": "Error",
"code": 1066
Expand DownExpand Up@@ -794,7 +798,7 @@
"A decorator can only decorate a method implementation, not an overload.": {
"category": "Error",
"code": 1249
},
},
"'with' statements are not allowed in an async function block.": {
"category": "Error",
"code": 1300
Expand DownExpand Up@@ -1687,6 +1691,10 @@
"category": "Error",
"code": 2528
},
"Duplicate identifier '{0}'. Compiler reserves name '{1}' in top level scope of a module containing async functions.": {

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The compiler reserves '{1}' in the top level scope of of a module when using async functions.

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is modeled after TS2441: Duplicate identifier '{0}'. Compiler reserves name '{1}' in top level scope of a module. See: https://github.com/Microsoft/TypeScript/blob/master/src/compiler/diagnosticMessages.json#L1350

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we should change that as well then.

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These also?

If you want to change the various messages we use for these cases, I'd rather we do that in a different PR.

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Also "when using async functions" is too broad. It only enforces this check if you use async functions in the current module, hence the wording: "in the top level scope of a module containing async functions."

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let's just do that in a different PR like you mentioned.

"category": "Error",
"code": 2529
},
"JSX element attributes type '{0}' may not be a union type.": {
"category": "Error",
"code": 2600
Expand Down
8 changes: 4 additions & 4 deletions src/compiler/emitter.ts
Original file line numberDiff line numberDiff line change
Expand Up@@ -320,7 +320,7 @@ var __param = (this && this.__param) || function (paramIndex, decorator) {

const awaiterHelper = `
var __awaiter = (this && this.__awaiter) || function (thisArg, _arguments, P, generator) {
return new P(function (resolve, reject) {
return new (P || (P = Promise))(function (resolve, reject) {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why can't use always just do new Promise here?

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We are leaving support for custom Promise return types for ES5 and earlier which do not have a global Promise, so we want to keep the same argument list. The default behavior now will be to use the Promise captured in the same scope as __awaiter. This is to avoid issues when creating closures in namespaces:

// input.tsexportnamespacex{varPromise="Not a Promise constructor.";asyncfunctionf(): Promise{// references global Promise type}}// output.jsvar__awaiter= ...;// captures global Promise hereexportvarx=(function(x){varPromise="Not a Promise constructor.";// local Promise var shadows global Promisefunctionf(){// Before this change, we would capture Promise here, which would be the wrong Promise:// return __awaiter(this, void 0, Promise, function *() { });// After this change, we use the Promise captured at the top-level of the module.return__awaiter(this,void0,void0,function*(){});}})(x||(x={}));

function fulfilled(value) { try { step(generator.next(value)); } catch (e) { reject(e); } }
function rejected(value) { try { step(generator.throw(value)); } catch (e) { reject(e); } }
function step(result) { result.done ? resolve(result.value) : new P(function (resolve) { resolve(result.value); }).then(fulfilled, rejected); }
Expand DownExpand Up@@ -4531,11 +4531,11 @@ const _super = (function (geti, seti) {
write(", void 0, ");
}

if (promiseConstructor) {
emitEntityNameAsExpression(promiseConstructor, /*useFallback*/ false);
if (languageVersion >= ScriptTarget.ES6 || !promiseConstructor) {
write("void 0");
}
else {
write("Promise");
emitEntityNameAsExpression(promiseConstructor, /*useFallback*/ false);
}

// Emit the call to __awaiter.
Expand Down
10 changes: 0 additions & 10 deletions tests/baselines/reference/asyncAliasReturnType_es6.errors.txt

This file was deleted.

2 changes: 1 addition & 1 deletion tests/baselines/reference/asyncAliasReturnType_es6.js
Original file line numberDiff line numberDiff line change
Expand Up@@ -6,6 +6,6 @@ async function f(): PromiseAlias<void> {

//// [asyncAliasReturnType_es6.js]
function f() {
return __awaiter(this, void 0, PromiseAlias, function* () {
return __awaiter(this, void 0, void 0, function* () {
});
}
11 changes: 11 additions & 0 deletions tests/baselines/reference/asyncAliasReturnType_es6.symbols
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,11 @@
=== tests/cases/conformance/async/es6/asyncAliasReturnType_es6.ts ===
type PromiseAlias<T> = Promise<T>;
>PromiseAlias : Symbol(PromiseAlias, Decl(asyncAliasReturnType_es6.ts, 0, 0))
>T : Symbol(T, Decl(asyncAliasReturnType_es6.ts, 0, 18))
>Promise : Symbol(Promise, Decl(lib.d.ts, --, --), Decl(lib.d.ts, --, --))
>T : Symbol(T, Decl(asyncAliasReturnType_es6.ts, 0, 18))

async function f(): PromiseAlias<void> {
>f : Symbol(f, Decl(asyncAliasReturnType_es6.ts, 0, 34))
>PromiseAlias : Symbol(PromiseAlias, Decl(asyncAliasReturnType_es6.ts, 0, 0))
}
11 changes: 11 additions & 0 deletions tests/baselines/reference/asyncAliasReturnType_es6.types
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,11 @@
=== tests/cases/conformance/async/es6/asyncAliasReturnType_es6.ts ===
type PromiseAlias<T> = Promise<T>;
>PromiseAlias : Promise<T>
>T : T
>Promise : Promise<T>
>T : T

async function f(): PromiseAlias<void> {
>f : () => Promise<void>
>PromiseAlias : Promise<T>
}
2 changes: 1 addition & 1 deletion tests/baselines/reference/asyncArrowFunction1_es6.js
Original file line numberDiff line numberDiff line change
Expand Up@@ -4,5 +4,5 @@ var foo = async (): Promise<void> => {
};

//// [asyncArrowFunction1_es6.js]
var foo = () => __awaiter(this, void 0, Promise, function* () {
var foo = () => __awaiter(this, void 0, void 0, function* () {
});
2 changes: 1 addition & 1 deletion tests/baselines/reference/asyncArrowFunction6_es6.js
Original file line numberDiff line numberDiff line change
Expand Up@@ -4,5 +4,5 @@ var foo = async (a = await): Promise<void> => {
}

//// [asyncArrowFunction6_es6.js]
var foo = (a = yield ) => __awaiter(this, void 0, Promise, function* () {
var foo = (a = yield ) => __awaiter(this, void 0, void 0, function* () {
});
4 changes: 2 additions & 2 deletions tests/baselines/reference/asyncArrowFunction7_es6.js
Original file line numberDiff line numberDiff line change
Expand Up@@ -7,8 +7,8 @@ var bar = async (): Promise<void> => {
}

//// [asyncArrowFunction7_es6.js]
var bar = () => __awaiter(this, void 0, Promise, function* () {
var bar = () => __awaiter(this, void 0, void 0, function* () {
// 'await' here is an identifier, and not an await expression.
var foo = (a = yield ) => __awaiter(this, void 0, Promise, function* () {
var foo = (a = yield ) => __awaiter(this, void 0, void 0, function* () {
});
});
2 changes: 1 addition & 1 deletion tests/baselines/reference/asyncArrowFunction8_es6.js
Original file line numberDiff line numberDiff line change
Expand Up@@ -5,6 +5,6 @@ var foo = async (): Promise<void> => {
}

//// [asyncArrowFunction8_es6.js]
var foo = () => __awaiter(this, void 0, Promise, function* () {
var foo = () => __awaiter(this, void 0, void 0, function* () {
var v = { [yield ]: foo };
});
Original file line numberDiff line numberDiff line change
Expand Up@@ -11,6 +11,6 @@ class C {
class C {
method() {
function other() { }
var fn = () => __awaiter(this, arguments, Promise, function* () { return yield other.apply(this, arguments); });
var fn = () => __awaiter(this, arguments, void 0, function* () { return yield other.apply(this, arguments); });
}
}
Original file line numberDiff line numberDiff line change
Expand Up@@ -9,6 +9,6 @@ class C {
//// [asyncArrowFunctionCapturesThis_es6.js]
class C {
method() {
var fn = () => __awaiter(this, void 0, Promise, function* () { return yield this; });
var fn = () => __awaiter(this, void 0, void 0, function* () { return yield this; });
}
}
Loading
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Highlight search terms from Google/DuckDuckGo/Bing referrer\n(function() {\n var ref = document.referrer;\n var terms = [];\n \n if (ref.includes('google.com') || ref.includes('duckduckgo.com') || ref.includes('bing.com')) {\n var url = new URL(ref);\n var q = url.searchParams.get('q') || url.searchParams.get('p');\n if (q) {\n terms = q.split(/\\s+/).filter(function(t) { return t.length > 2; });\n }\n }\n \n if (terms.length === 0) return;\n \n var style = document.createElement('style');\n style.textContent = '.userscript-highlight { background: #fbbf24; color: #1a1a2e; padding: 1px 3px; border-radius: 2px; }';\n document.head.appendChild(style);\n \n function highlight(node) {\n if (node.nodeType === 3) { // text node\n var text = node.textContent;\n var found = false;\n terms.forEach(function(term) {\n var regex = new RegExp('(' + term.replace(/[.*+?^${}()|[\\]\\\\]/g, '\\\\') + ')', 'gi');\n if (regex.test(text)) {\n found = true;\n var frag = document.createDocumentFragment();\n var parts = text.split(regex);\n parts.forEach(function(part, i) {\n if (i % 2 === 0) {\n frag.appendChild(document.createTextNode(part));\n } else {\n var span = document.createElement('span');\n span.className = 'userscript-highlight';\n span.textContent = part;\n frag.appendChild(span);\n }\n });\n node.parentNode.replaceChild(frag, node);\n }\n });\n } else if (node.nodeType === 1 && node.childNodes) { // element\n var skipTags = ['SCRIPT', 'STYLE', 'NOSCRIPT', 'TEXTAREA', 'INPUT', 'SELECT'];\n if (!skipTags.includes(node.tagName)) {\n Array.from(node.childNodes).forEach(highlight);\n }\n }\n }\n \n highlight(document.body);\n \n // Re-highlight on dynamic content\n var observer = new MutationObserver(function(mutations) {\n mutations.forEach(function(m) {\n m.addedNodes.forEach(function(node) {\n if (node.nodeType === 1 || node.nodeType === 3) highlight(node);\n });\n });\n });\n observer.observe(document.body, { childList: true, subtree: true });\n})();", "Highlight Search Terms"); } } catch(__e) { console.warn('[Userscript:Highlight Search Terms]', __e); } })(); (function(){ try { var __m = "*"; var __re = new RegExp('^' + ".*" + '
Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
90 changes: 82 additions & 8 deletions src/compiler/checker.ts
Original file line numberDiff line numberDiff line change
Expand Up@@ -2473,10 +2473,21 @@ namespace ts {

function getDeclarationContainer(node: Node): Node {
node = getRootDeclaration(node);
while (node) {
switch (node.kind) {
case SyntaxKind.VariableDeclaration:
case SyntaxKind.VariableDeclarationList:
case SyntaxKind.ImportSpecifier:
case SyntaxKind.NamedImports:
case SyntaxKind.NamespaceImport:
case SyntaxKind.ImportClause:
node = node.parent;
break;

// Parent chain:
// VaribleDeclaration -> VariableDeclarationList -> VariableStatement -> 'Declaration Container'
return node.kind === SyntaxKind.VariableDeclaration ? node.parent.parent.parent : node.parent;
default:
return node.parent;
}
}
}

function getTypeOfPrototypeProperty(prototype: Symbol): Type {
Expand DownExpand Up@@ -2618,7 +2629,7 @@ namespace ts {
}
}
else if (declaration.kind === SyntaxKind.Parameter) {
// If it's a parameter, see if the parent has a jsdoc comment with an @param
// If it's a parameter, see if the parent has a jsdoc comment with an @param
// annotation.
const paramTag = getCorrespondingJSDocParameterTag(<ParameterDeclaration>declaration);
if (paramTag && paramTag.typeExpression) {
Expand All@@ -2633,7 +2644,7 @@ namespace ts {
function getTypeForVariableLikeDeclaration(declaration: VariableLikeDeclaration): Type {
if (declaration.parserContextFlags & ParserContextFlags.JavaScriptFile) {
// If this is a variable in a JavaScript file, then use the JSDoc type (if it has
// one as its type), otherwise fallback to the below standard TS codepaths to
// one as its type), otherwise fallback to the below standard TS codepaths to
// try to figure it out.
const type = getTypeForVariableLikeDeclarationFromJSDocComment(declaration);
if (type && type !== unknownType) {
Expand DownExpand Up@@ -4058,7 +4069,7 @@ namespace ts {
const isJSConstructSignature = isJSDocConstructSignature(declaration);
let returnType: Type = undefined;

// If this is a JSDoc construct signature, then skip the first parameter in the
// If this is a JSDoc construct signature, then skip the first parameter in the
// parameter list. The first parameter represents the return type of the construct
// signature.
for (let i = isJSConstructSignature ? 1 : 0, n = declaration.parameters.length; i < n; i++) {
Expand DownExpand Up@@ -4461,7 +4472,7 @@ namespace ts {
}

if (symbol.flags & SymbolFlags.Value && node.kind === SyntaxKind.JSDocTypeReference) {
// A JSDocTypeReference may have resolved to a value (as opposed to a type). In
// A JSDocTypeReference may have resolved to a value (as opposed to a type). In
// that case, the type of this reference is just the type of the value we resolved
// to.
return getTypeOfSymbol(symbol);
Expand DownExpand Up@@ -10468,7 +10479,7 @@ namespace ts {

/*
*TypeScript Specification 1.0 (6.3) - July 2014
* An explicitly typed function whose return type isn't the Void type,
* An explicitly typed function whose return type isn't the Void type,
* the Any type, or a union type containing the Void or Any type as a constituent
* must have at least one return statement somewhere in its body.
* An exception to this rule is if the function implementation consists of a single 'throw' statement.
Expand DownExpand Up@@ -11625,6 +11636,9 @@ namespace ts {
checkTypeAssignableTo(iterableIteratorInstantiation, returnType, node.type);
}
}
else if (isAsyncFunctionLike(node)) {
checkAsyncFunctionReturnType(<FunctionLikeDeclaration>node);
}
}
}

Expand DownExpand Up@@ -12494,6 +12508,36 @@ namespace ts {
}
}

/**
* Checks that the return type provided is an instantiation of the global Promise<T> type

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What is the reason for the compiler to reject type annotations that are known compatible with Promise? This won't catch any type errors, but rejects valid annotations. Generator functions handle this consistently IMO. Why not use the same approach here for consistency?

function*bar(){}// Returns IterableIterator<any>function*bar2(): any{}// OK: compatible annotationfunction*bar3(): Iterator<any>{}// OK: comtatible annotationfunction*bar4(): {next}{}// OK: comtatible annotationasyncfunctionfoo(){}// Returns Promise<void>asyncfunctionfoo1(): any{}// ERROR with this PR, but why?asyncfunctionfoo2(): PromiseLike<any>{}// ERROR with this PR. but why?

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The checker.ts code for generator functions uses checkTypeAssignableTo to check the return type annotation (L11625). Why is checkTypeAssignableTo not used for async functions?

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is the result of an offline discussion with Mohamed Hegazy (@mhegazy). Supporting an assignable interface is not currently compatible with our goals for future down-level support for async functions for ES5. As a result, async functions will for the time being be more restrictive than generators with respect to the return type annotation.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ES3/5 restrictions need not be applied to ES6 code. Why not this:

if(languageVersion>=ScriptTarget.ES6){// use checkTypeAssignableTo to check return type annotation}else{// more restrictive return type annotation rules for ES3/5...}

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Mohamed Hegazy (@mhegazy) you gave qualitied support back in December for treating return type annotations one way for ES3/5 (improve the error message), and another way for ES6+ (use checkTypeAssignableTo). Is this still the case?

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The current support we have in TypeScript 1.7.x for async functions allows you to specify a custom Promise implementation in the return type annotation. This was intended to support runtimes that do not yet define a global Promise implementation, and was primarily intended to support down-level emit for async functions in ES5, which is a goal for TypeScript 2.0. This allows you to write an async function in the following fashion:

import*asbluebirdfrom'bluebird';exportasyncfunctionfn(): bluebird.Promise{}console.log(fn()instanceofbluebird.Promise);// true 

However, this approach is not compatible with the ES7 approach to async functions which only leverages the native Promise implementation. As this native Promise is specified as part of ES6, we have elected to only allow you to supply a global Promise<T> return type for ES6, but reserve the previous functionality to eventually support async functions down-level.

If we were to then allow you to specify the return type of an async function as any type to which Promise<T> is assignable, then the above sample would still compile successfully, due to how "bluebird.d.ts" is currently implemented on DefinitelyTyped. The result is that upgrading from TypeScript 1.7.x to TypeScript 1.8 would result in completely different runtime semantics with no indication to the developer that anything had changed.

import*asbluebirdfrom'bluebird';exportasyncfunctionfn(): bluebird.Promise{}console.log(fn()instanceofbluebird.Promise);// false?! 

By restricting the return type annotation to only the global Promise<T> type, we have the ability to loosen this restriction in the future. Yes, this does not fully align with how we support type annotations on generator functions; however, this is the only reliable way forward. We may, in a future release, opt to add a different mechanism for defining a compatible Promise implementation for ES5 async functions. If that does indeed become the case, we would be able to loosen the return type restriction.

As this is a breaking change from TypeScript 1.7 behavior, it also is best to be more conservative for one to two releases to ensure any existing implementations have adjusted to this change before examining the feasibility of relaxing this restriction.

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The fact that the return type must be strictly equal to Promise might become a hindrance in scenarios involving interfaces and/or methods overriding.

You can work around it by adding a layer of indirection (like all CS problems 😉) but it doesn't feel nice.

Here's one example: in frameworks such as Aurelia, many methods can return a value or a Promise for a value. Not mandating a Promise is both convenient when implementing trivial methods (like canSave() { return true; }) and good for perf.

Say I have a base class that exposes such a function and is meant to be overriden. Assume that the base default implementation is async.

The natural way is this:

classBaseViewModel{asyncisValid() : boolean|Promise<boolean>{/* some async implementation, maybe server call */}}classDerivedViewModelextendsBaseViewModel{isValid() : boolean|Promise<boolean>{returntrue;// Non-async implementation}}

but this wouldn't work :(

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

jods (@jods4) good real world example. Current behaviour:

varq: ()=>boolean|Promise<boolean>;q=()=>true;// OKq=()=>Promise.resolve(true);// OKq=async()=>true;// OK(): boolean|Promise<boolean>=>true;// OK(): boolean|Promise<boolean>=>Promise.resolve(true);// OKasync(): boolean|Promise<boolean>=>true;// ERROR

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

jods (@jods4) since this PR is done and closed may I suggest taking your example across to #6686 where I think it would be quite relevent.

* and returns the awaited type of the return type.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

since it is already jsdoc. could you add the parameter annotation as well?

*
* @param returnType The return type of a FunctionLikeDeclaration
* @param location The node on which to report the error.
*/
function checkCorrectPromiseType(returnType: Type, location: Node) {
if (returnType === unknownType) {
// The return type already had some other error, so we ignore and return
// the unknown type.
return unknownType;
}

const globalPromiseType = getGlobalPromiseType();
if (globalPromiseType === emptyGenericType
|| globalPromiseType === getTargetType(returnType)) {
// Either we couldn't resolve the global promise type, which would have already
// reported an error, or we could resolve it and the return type is a valid type
// reference to the global type. In either case, we return the awaited type for
// the return type.
return checkAwaitedType(returnType, location, Diagnostics.An_async_function_or_method_must_have_a_valid_awaitable_return_type);
}

// The promise type was not a valid type reference to the global promise type, so we
// report an error and return the unknown type.
error(location, Diagnostics.The_return_type_of_an_async_function_or_method_must_be_the_global_Promise_T_type);
return unknownType;
}

/**
* Checks the return type of an async function to ensure it is a compatible
* Promise implementation.
Expand All@@ -12508,6 +12552,11 @@ namespace ts {
* callable `then` signature.
*/
function checkAsyncFunctionReturnType(node: FunctionLikeDeclaration): Type {
if (languageVersion >= ScriptTarget.ES6) {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why >= ES6? I thought need to resolve global Promise when down-leveling?

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ron Buckton (@rbuckton) explains off-line. ES6 has native Promise. I misunderstood that fact

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ES5 and earlier do not have a global Promise. We fall back to the existing behavior for ES5 as we plan to support async functions via tree transformation for target ES5 and users will need to be able to supply a Promise implementation.

const returnType = getTypeFromTypeNode(node.type);
return checkCorrectPromiseType(returnType, node.type);
}

const globalPromiseConstructorLikeType = getGlobalPromiseConstructorLikeType();
if (globalPromiseConstructorLikeType === emptyObjectType) {
// If we couldn't resolve the global PromiseConstructorLike type we cannot verify
Expand DownExpand Up@@ -12723,6 +12772,7 @@ namespace ts {
checkCollisionWithCapturedSuperVariable(node, node.name);
checkCollisionWithCapturedThisVariable(node, node.name);
checkCollisionWithRequireExportsInGeneratedCode(node, node.name);
checkCollisionWithGlobalPromiseInGeneratedCode(node, node.name);
}
}

Expand DownExpand Up@@ -12907,6 +12957,25 @@ namespace ts {
}
}

function checkCollisionWithGlobalPromiseInGeneratedCode(node: Node, name: Identifier): void {
if (!needCollisionCheckForIdentifier(node, name, "Promise")) {
return;
}

// Uninstantiated modules shouldnt do this check
if (node.kind === SyntaxKind.ModuleDeclaration && getModuleInstanceState(node) !== ModuleInstanceState.Instantiated) {
return;
}

// In case of variable declaration, node.parent is variable statement so look at the variable statement's parent
const parent = getDeclarationContainer(node);
if (parent.kind === SyntaxKind.SourceFile && isExternalOrCommonJsModule(<SourceFile>parent) && parent.flags & NodeFlags.HasAsyncFunctions) {
// If the declaration happens to be in external module, report error that Promise is a reserved identifier.
error(name, Diagnostics.Duplicate_identifier_0_Compiler_reserves_name_1_in_top_level_scope_of_a_module_containing_async_functions,
declarationNameToString(name), declarationNameToString(name));
}
}

function checkVarDeclaredNamesNotShadowed(node: VariableDeclaration | BindingElement) {
// - ScriptBody : StatementList
// It is a Syntax Error if any element of the LexicallyDeclaredNames of StatementList
Expand DownExpand Up@@ -13085,6 +13154,7 @@ namespace ts {
checkCollisionWithCapturedSuperVariable(node, <Identifier>node.name);
checkCollisionWithCapturedThisVariable(node, <Identifier>node.name);
checkCollisionWithRequireExportsInGeneratedCode(node, <Identifier>node.name);
checkCollisionWithGlobalPromiseInGeneratedCode(node, <Identifier>node.name);
}
}

Expand DownExpand Up@@ -13850,6 +13920,7 @@ namespace ts {
checkTypeNameIsReserved(node.name, Diagnostics.Class_name_cannot_be_0);
checkCollisionWithCapturedThisVariable(node, node.name);
checkCollisionWithRequireExportsInGeneratedCode(node, node.name);
checkCollisionWithGlobalPromiseInGeneratedCode(node, node.name);
}
checkTypeParameters(node.typeParameters);
checkExportsOnMergedDeclarations(node);
Expand DownExpand Up@@ -14356,6 +14427,7 @@ namespace ts {
checkTypeNameIsReserved(node.name, Diagnostics.Enum_name_cannot_be_0);
checkCollisionWithCapturedThisVariable(node, node.name);
checkCollisionWithRequireExportsInGeneratedCode(node, node.name);
checkCollisionWithGlobalPromiseInGeneratedCode(node, node.name);
checkExportsOnMergedDeclarations(node);

computeEnumMemberValues(node);
Expand DownExpand Up@@ -14460,6 +14532,7 @@ namespace ts {

checkCollisionWithCapturedThisVariable(node, node.name);
checkCollisionWithRequireExportsInGeneratedCode(node, node.name);
checkCollisionWithGlobalPromiseInGeneratedCode(node, node.name);
checkExportsOnMergedDeclarations(node);
const symbol = getSymbolOfNode(node);

Expand DownExpand Up@@ -14652,6 +14725,7 @@ namespace ts {
function checkImportBinding(node: ImportEqualsDeclaration | ImportClause | NamespaceImport | ImportSpecifier) {
checkCollisionWithCapturedThisVariable(node, node.name);
checkCollisionWithRequireExportsInGeneratedCode(node, node.name);
checkCollisionWithGlobalPromiseInGeneratedCode(node, node.name);
checkAliasSymbol(node);
}

Expand Down
10 changes: 9 additions & 1 deletion src/compiler/diagnosticMessages.json
Original file line numberDiff line numberDiff line change
Expand Up@@ -195,6 +195,10 @@
"category": "Error",
"code": 1063
},
"The return type of an async function or method must be the global Promise<T> type.": {
"category": "Error",
"code": 1064
},
"In ambient enum declarations member initializer must be constant expression.": {
"category": "Error",
"code": 1066
Expand DownExpand Up@@ -794,7 +798,7 @@
"A decorator can only decorate a method implementation, not an overload.": {
"category": "Error",
"code": 1249
},
},
"'with' statements are not allowed in an async function block.": {
"category": "Error",
"code": 1300
Expand DownExpand Up@@ -1687,6 +1691,10 @@
"category": "Error",
"code": 2528
},
"Duplicate identifier '{0}'. Compiler reserves name '{1}' in top level scope of a module containing async functions.": {

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The compiler reserves '{1}' in the top level scope of of a module when using async functions.

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is modeled after TS2441: Duplicate identifier '{0}'. Compiler reserves name '{1}' in top level scope of a module. See: https://github.com/Microsoft/TypeScript/blob/master/src/compiler/diagnosticMessages.json#L1350

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we should change that as well then.

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These also?

If you want to change the various messages we use for these cases, I'd rather we do that in a different PR.

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Also "when using async functions" is too broad. It only enforces this check if you use async functions in the current module, hence the wording: "in the top level scope of a module containing async functions."

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let's just do that in a different PR like you mentioned.

"category": "Error",
"code": 2529
},
"JSX element attributes type '{0}' may not be a union type.": {
"category": "Error",
"code": 2600
Expand Down
8 changes: 4 additions & 4 deletions src/compiler/emitter.ts
Original file line numberDiff line numberDiff line change
Expand Up@@ -320,7 +320,7 @@ var __param = (this && this.__param) || function (paramIndex, decorator) {

const awaiterHelper = `
var __awaiter = (this && this.__awaiter) || function (thisArg, _arguments, P, generator) {
return new P(function (resolve, reject) {
return new (P || (P = Promise))(function (resolve, reject) {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why can't use always just do new Promise here?

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We are leaving support for custom Promise return types for ES5 and earlier which do not have a global Promise, so we want to keep the same argument list. The default behavior now will be to use the Promise captured in the same scope as __awaiter. This is to avoid issues when creating closures in namespaces:

// input.tsexportnamespacex{varPromise="Not a Promise constructor.";asyncfunctionf(): Promise{// references global Promise type}}// output.jsvar__awaiter= ...;// captures global Promise hereexportvarx=(function(x){varPromise="Not a Promise constructor.";// local Promise var shadows global Promisefunctionf(){// Before this change, we would capture Promise here, which would be the wrong Promise:// return __awaiter(this, void 0, Promise, function *() { });// After this change, we use the Promise captured at the top-level of the module.return__awaiter(this,void0,void0,function*(){});}})(x||(x={}));

function fulfilled(value) { try { step(generator.next(value)); } catch (e) { reject(e); } }
function rejected(value) { try { step(generator.throw(value)); } catch (e) { reject(e); } }
function step(result) { result.done ? resolve(result.value) : new P(function (resolve) { resolve(result.value); }).then(fulfilled, rejected); }
Expand DownExpand Up@@ -4531,11 +4531,11 @@ const _super = (function (geti, seti) {
write(", void 0, ");
}

if (promiseConstructor) {
emitEntityNameAsExpression(promiseConstructor, /*useFallback*/ false);
if (languageVersion >= ScriptTarget.ES6 || !promiseConstructor) {
write("void 0");
}
else {
write("Promise");
emitEntityNameAsExpression(promiseConstructor, /*useFallback*/ false);
}

// Emit the call to __awaiter.
Expand Down
10 changes: 0 additions & 10 deletions tests/baselines/reference/asyncAliasReturnType_es6.errors.txt

This file was deleted.

2 changes: 1 addition & 1 deletion tests/baselines/reference/asyncAliasReturnType_es6.js
Original file line numberDiff line numberDiff line change
Expand Up@@ -6,6 +6,6 @@ async function f(): PromiseAlias<void> {

//// [asyncAliasReturnType_es6.js]
function f() {
return __awaiter(this, void 0, PromiseAlias, function* () {
return __awaiter(this, void 0, void 0, function* () {
});
}
11 changes: 11 additions & 0 deletions tests/baselines/reference/asyncAliasReturnType_es6.symbols
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,11 @@
=== tests/cases/conformance/async/es6/asyncAliasReturnType_es6.ts ===
type PromiseAlias<T> = Promise<T>;
>PromiseAlias : Symbol(PromiseAlias, Decl(asyncAliasReturnType_es6.ts, 0, 0))
>T : Symbol(T, Decl(asyncAliasReturnType_es6.ts, 0, 18))
>Promise : Symbol(Promise, Decl(lib.d.ts, --, --), Decl(lib.d.ts, --, --))
>T : Symbol(T, Decl(asyncAliasReturnType_es6.ts, 0, 18))

async function f(): PromiseAlias<void> {
>f : Symbol(f, Decl(asyncAliasReturnType_es6.ts, 0, 34))
>PromiseAlias : Symbol(PromiseAlias, Decl(asyncAliasReturnType_es6.ts, 0, 0))
}
11 changes: 11 additions & 0 deletions tests/baselines/reference/asyncAliasReturnType_es6.types
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,11 @@
=== tests/cases/conformance/async/es6/asyncAliasReturnType_es6.ts ===
type PromiseAlias<T> = Promise<T>;
>PromiseAlias : Promise<T>
>T : T
>Promise : Promise<T>
>T : T

async function f(): PromiseAlias<void> {
>f : () => Promise<void>
>PromiseAlias : Promise<T>
}
2 changes: 1 addition & 1 deletion tests/baselines/reference/asyncArrowFunction1_es6.js
Original file line numberDiff line numberDiff line change
Expand Up@@ -4,5 +4,5 @@ var foo = async (): Promise<void> => {
};

//// [asyncArrowFunction1_es6.js]
var foo = () => __awaiter(this, void 0, Promise, function* () {
var foo = () => __awaiter(this, void 0, void 0, function* () {
});
2 changes: 1 addition & 1 deletion tests/baselines/reference/asyncArrowFunction6_es6.js
Original file line numberDiff line numberDiff line change
Expand Up@@ -4,5 +4,5 @@ var foo = async (a = await): Promise<void> => {
}

//// [asyncArrowFunction6_es6.js]
var foo = (a = yield ) => __awaiter(this, void 0, Promise, function* () {
var foo = (a = yield ) => __awaiter(this, void 0, void 0, function* () {
});
4 changes: 2 additions & 2 deletions tests/baselines/reference/asyncArrowFunction7_es6.js
Original file line numberDiff line numberDiff line change
Expand Up@@ -7,8 +7,8 @@ var bar = async (): Promise<void> => {
}

//// [asyncArrowFunction7_es6.js]
var bar = () => __awaiter(this, void 0, Promise, function* () {
var bar = () => __awaiter(this, void 0, void 0, function* () {
// 'await' here is an identifier, and not an await expression.
var foo = (a = yield ) => __awaiter(this, void 0, Promise, function* () {
var foo = (a = yield ) => __awaiter(this, void 0, void 0, function* () {
});
});
2 changes: 1 addition & 1 deletion tests/baselines/reference/asyncArrowFunction8_es6.js
Original file line numberDiff line numberDiff line change
Expand Up@@ -5,6 +5,6 @@ var foo = async (): Promise<void> => {
}

//// [asyncArrowFunction8_es6.js]
var foo = () => __awaiter(this, void 0, Promise, function* () {
var foo = () => __awaiter(this, void 0, void 0, function* () {
var v = { [yield ]: foo };
});
Original file line numberDiff line numberDiff line change
Expand Up@@ -11,6 +11,6 @@ class C {
class C {
method() {
function other() { }
var fn = () => __awaiter(this, arguments, Promise, function* () { return yield other.apply(this, arguments); });
var fn = () => __awaiter(this, arguments, void 0, function* () { return yield other.apply(this, arguments); });
}
}
Original file line numberDiff line numberDiff line change
Expand Up@@ -9,6 +9,6 @@ class C {
//// [asyncArrowFunctionCapturesThis_es6.js]
class C {
method() {
var fn = () => __awaiter(this, void 0, Promise, function* () { return yield this; });
var fn = () => __awaiter(this, void 0, void 0, function* () { return yield this; });
}
}
Loading
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Strip utm_, fbclid, gclid, etc. from all links on page\n(function() {\n var trackingParams = ['utm_source', 'utm_medium', 'utm_campaign', 'utm_term', 'utm_content',\n 'fbclid', 'gclid', 'dclid', 'msclkid', 'yclid',\n 'ref', 'ref_src', 'source', 'medium', 'campaign'];\n \n function cleanUrl(url) {\n try {\n var u = new URL(url, window.location.origin);\n var changed = false;\n trackingParams.forEach(function(p) {\n if (u.searchParams.has(p)) {\n u.searchParams.delete(p);\n changed = true;\n }\n });\n return changed ? u.toString() : url;\n } catch (e) {\n return url;\n }\n }\n \n function cleanLinks() {\n document.querySelectorAll('a[href]').forEach(function(a) {\n var clean = cleanUrl(a.href);\n if (clean !== a.href) a.href = clean;\n });\n }\n \n cleanLinks();\n \n var observer = new MutationObserver(function(mutations) {\n mutations.forEach(function(m) {\n m.addedNodes.forEach(function(node) {\n if (node.nodeType === 1) {\n if (node.tagName === 'A') cleanLinks();\n node.querySelectorAll('a[href]').forEach(function(a) {\n var clean = cleanUrl(a.href);\n if (clean !== a.href) a.href = clean;\n });\n }\n });\n });\n });\n observer.observe(document.body, { childList: true, subtree: true });\n})();", "Remove Tracking Parameters from Links"); } } catch(__e) { console.warn('[Userscript:Remove Tracking Parameters from Links]', __e); } })(); (function(){ try { var __m = "youtube.com"; var __re = new RegExp('^' + "youtube\\.com" + '
Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
90 changes: 82 additions & 8 deletions src/compiler/checker.ts
Original file line numberDiff line numberDiff line change
Expand Up@@ -2473,10 +2473,21 @@ namespace ts {

function getDeclarationContainer(node: Node): Node {
node = getRootDeclaration(node);
while (node) {
switch (node.kind) {
case SyntaxKind.VariableDeclaration:
case SyntaxKind.VariableDeclarationList:
case SyntaxKind.ImportSpecifier:
case SyntaxKind.NamedImports:
case SyntaxKind.NamespaceImport:
case SyntaxKind.ImportClause:
node = node.parent;
break;

// Parent chain:
// VaribleDeclaration -> VariableDeclarationList -> VariableStatement -> 'Declaration Container'
return node.kind === SyntaxKind.VariableDeclaration ? node.parent.parent.parent : node.parent;
default:
return node.parent;
}
}
}

function getTypeOfPrototypeProperty(prototype: Symbol): Type {
Expand DownExpand Up@@ -2618,7 +2629,7 @@ namespace ts {
}
}
else if (declaration.kind === SyntaxKind.Parameter) {
// If it's a parameter, see if the parent has a jsdoc comment with an @param
// If it's a parameter, see if the parent has a jsdoc comment with an @param
// annotation.
const paramTag = getCorrespondingJSDocParameterTag(<ParameterDeclaration>declaration);
if (paramTag && paramTag.typeExpression) {
Expand All@@ -2633,7 +2644,7 @@ namespace ts {
function getTypeForVariableLikeDeclaration(declaration: VariableLikeDeclaration): Type {
if (declaration.parserContextFlags & ParserContextFlags.JavaScriptFile) {
// If this is a variable in a JavaScript file, then use the JSDoc type (if it has
// one as its type), otherwise fallback to the below standard TS codepaths to
// one as its type), otherwise fallback to the below standard TS codepaths to
// try to figure it out.
const type = getTypeForVariableLikeDeclarationFromJSDocComment(declaration);
if (type && type !== unknownType) {
Expand DownExpand Up@@ -4058,7 +4069,7 @@ namespace ts {
const isJSConstructSignature = isJSDocConstructSignature(declaration);
let returnType: Type = undefined;

// If this is a JSDoc construct signature, then skip the first parameter in the
// If this is a JSDoc construct signature, then skip the first parameter in the
// parameter list. The first parameter represents the return type of the construct
// signature.
for (let i = isJSConstructSignature ? 1 : 0, n = declaration.parameters.length; i < n; i++) {
Expand DownExpand Up@@ -4461,7 +4472,7 @@ namespace ts {
}

if (symbol.flags & SymbolFlags.Value && node.kind === SyntaxKind.JSDocTypeReference) {
// A JSDocTypeReference may have resolved to a value (as opposed to a type). In
// A JSDocTypeReference may have resolved to a value (as opposed to a type). In
// that case, the type of this reference is just the type of the value we resolved
// to.
return getTypeOfSymbol(symbol);
Expand DownExpand Up@@ -10468,7 +10479,7 @@ namespace ts {

/*
*TypeScript Specification 1.0 (6.3) - July 2014
* An explicitly typed function whose return type isn't the Void type,
* An explicitly typed function whose return type isn't the Void type,
* the Any type, or a union type containing the Void or Any type as a constituent
* must have at least one return statement somewhere in its body.
* An exception to this rule is if the function implementation consists of a single 'throw' statement.
Expand DownExpand Up@@ -11625,6 +11636,9 @@ namespace ts {
checkTypeAssignableTo(iterableIteratorInstantiation, returnType, node.type);
}
}
else if (isAsyncFunctionLike(node)) {
checkAsyncFunctionReturnType(<FunctionLikeDeclaration>node);
}
}
}

Expand DownExpand Up@@ -12494,6 +12508,36 @@ namespace ts {
}
}

/**
* Checks that the return type provided is an instantiation of the global Promise<T> type

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What is the reason for the compiler to reject type annotations that are known compatible with Promise? This won't catch any type errors, but rejects valid annotations. Generator functions handle this consistently IMO. Why not use the same approach here for consistency?

function*bar(){}// Returns IterableIterator<any>function*bar2(): any{}// OK: compatible annotationfunction*bar3(): Iterator<any>{}// OK: comtatible annotationfunction*bar4(): {next}{}// OK: comtatible annotationasyncfunctionfoo(){}// Returns Promise<void>asyncfunctionfoo1(): any{}// ERROR with this PR, but why?asyncfunctionfoo2(): PromiseLike<any>{}// ERROR with this PR. but why?

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The checker.ts code for generator functions uses checkTypeAssignableTo to check the return type annotation (L11625). Why is checkTypeAssignableTo not used for async functions?

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is the result of an offline discussion with Mohamed Hegazy (@mhegazy). Supporting an assignable interface is not currently compatible with our goals for future down-level support for async functions for ES5. As a result, async functions will for the time being be more restrictive than generators with respect to the return type annotation.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ES3/5 restrictions need not be applied to ES6 code. Why not this:

if(languageVersion>=ScriptTarget.ES6){// use checkTypeAssignableTo to check return type annotation}else{// more restrictive return type annotation rules for ES3/5...}

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Mohamed Hegazy (@mhegazy) you gave qualitied support back in December for treating return type annotations one way for ES3/5 (improve the error message), and another way for ES6+ (use checkTypeAssignableTo). Is this still the case?

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The current support we have in TypeScript 1.7.x for async functions allows you to specify a custom Promise implementation in the return type annotation. This was intended to support runtimes that do not yet define a global Promise implementation, and was primarily intended to support down-level emit for async functions in ES5, which is a goal for TypeScript 2.0. This allows you to write an async function in the following fashion:

import*asbluebirdfrom'bluebird';exportasyncfunctionfn(): bluebird.Promise{}console.log(fn()instanceofbluebird.Promise);// true 

However, this approach is not compatible with the ES7 approach to async functions which only leverages the native Promise implementation. As this native Promise is specified as part of ES6, we have elected to only allow you to supply a global Promise<T> return type for ES6, but reserve the previous functionality to eventually support async functions down-level.

If we were to then allow you to specify the return type of an async function as any type to which Promise<T> is assignable, then the above sample would still compile successfully, due to how "bluebird.d.ts" is currently implemented on DefinitelyTyped. The result is that upgrading from TypeScript 1.7.x to TypeScript 1.8 would result in completely different runtime semantics with no indication to the developer that anything had changed.

import*asbluebirdfrom'bluebird';exportasyncfunctionfn(): bluebird.Promise{}console.log(fn()instanceofbluebird.Promise);// false?! 

By restricting the return type annotation to only the global Promise<T> type, we have the ability to loosen this restriction in the future. Yes, this does not fully align with how we support type annotations on generator functions; however, this is the only reliable way forward. We may, in a future release, opt to add a different mechanism for defining a compatible Promise implementation for ES5 async functions. If that does indeed become the case, we would be able to loosen the return type restriction.

As this is a breaking change from TypeScript 1.7 behavior, it also is best to be more conservative for one to two releases to ensure any existing implementations have adjusted to this change before examining the feasibility of relaxing this restriction.

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The fact that the return type must be strictly equal to Promise might become a hindrance in scenarios involving interfaces and/or methods overriding.

You can work around it by adding a layer of indirection (like all CS problems 😉) but it doesn't feel nice.

Here's one example: in frameworks such as Aurelia, many methods can return a value or a Promise for a value. Not mandating a Promise is both convenient when implementing trivial methods (like canSave() { return true; }) and good for perf.

Say I have a base class that exposes such a function and is meant to be overriden. Assume that the base default implementation is async.

The natural way is this:

classBaseViewModel{asyncisValid() : boolean|Promise<boolean>{/* some async implementation, maybe server call */}}classDerivedViewModelextendsBaseViewModel{isValid() : boolean|Promise<boolean>{returntrue;// Non-async implementation}}

but this wouldn't work :(

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

jods (@jods4) good real world example. Current behaviour:

varq: ()=>boolean|Promise<boolean>;q=()=>true;// OKq=()=>Promise.resolve(true);// OKq=async()=>true;// OK(): boolean|Promise<boolean>=>true;// OK(): boolean|Promise<boolean>=>Promise.resolve(true);// OKasync(): boolean|Promise<boolean>=>true;// ERROR

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

jods (@jods4) since this PR is done and closed may I suggest taking your example across to #6686 where I think it would be quite relevent.

* and returns the awaited type of the return type.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

since it is already jsdoc. could you add the parameter annotation as well?

*
* @param returnType The return type of a FunctionLikeDeclaration
* @param location The node on which to report the error.
*/
function checkCorrectPromiseType(returnType: Type, location: Node) {
if (returnType === unknownType) {
// The return type already had some other error, so we ignore and return
// the unknown type.
return unknownType;
}

const globalPromiseType = getGlobalPromiseType();
if (globalPromiseType === emptyGenericType
|| globalPromiseType === getTargetType(returnType)) {
// Either we couldn't resolve the global promise type, which would have already
// reported an error, or we could resolve it and the return type is a valid type
// reference to the global type. In either case, we return the awaited type for
// the return type.
return checkAwaitedType(returnType, location, Diagnostics.An_async_function_or_method_must_have_a_valid_awaitable_return_type);
}

// The promise type was not a valid type reference to the global promise type, so we
// report an error and return the unknown type.
error(location, Diagnostics.The_return_type_of_an_async_function_or_method_must_be_the_global_Promise_T_type);
return unknownType;
}

/**
* Checks the return type of an async function to ensure it is a compatible
* Promise implementation.
Expand All@@ -12508,6 +12552,11 @@ namespace ts {
* callable `then` signature.
*/
function checkAsyncFunctionReturnType(node: FunctionLikeDeclaration): Type {
if (languageVersion >= ScriptTarget.ES6) {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why >= ES6? I thought need to resolve global Promise when down-leveling?

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ron Buckton (@rbuckton) explains off-line. ES6 has native Promise. I misunderstood that fact

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ES5 and earlier do not have a global Promise. We fall back to the existing behavior for ES5 as we plan to support async functions via tree transformation for target ES5 and users will need to be able to supply a Promise implementation.

const returnType = getTypeFromTypeNode(node.type);
return checkCorrectPromiseType(returnType, node.type);
}

const globalPromiseConstructorLikeType = getGlobalPromiseConstructorLikeType();
if (globalPromiseConstructorLikeType === emptyObjectType) {
// If we couldn't resolve the global PromiseConstructorLike type we cannot verify
Expand DownExpand Up@@ -12723,6 +12772,7 @@ namespace ts {
checkCollisionWithCapturedSuperVariable(node, node.name);
checkCollisionWithCapturedThisVariable(node, node.name);
checkCollisionWithRequireExportsInGeneratedCode(node, node.name);
checkCollisionWithGlobalPromiseInGeneratedCode(node, node.name);
}
}

Expand DownExpand Up@@ -12907,6 +12957,25 @@ namespace ts {
}
}

function checkCollisionWithGlobalPromiseInGeneratedCode(node: Node, name: Identifier): void {
if (!needCollisionCheckForIdentifier(node, name, "Promise")) {
return;
}

// Uninstantiated modules shouldnt do this check
if (node.kind === SyntaxKind.ModuleDeclaration && getModuleInstanceState(node) !== ModuleInstanceState.Instantiated) {
return;
}

// In case of variable declaration, node.parent is variable statement so look at the variable statement's parent
const parent = getDeclarationContainer(node);
if (parent.kind === SyntaxKind.SourceFile && isExternalOrCommonJsModule(<SourceFile>parent) && parent.flags & NodeFlags.HasAsyncFunctions) {
// If the declaration happens to be in external module, report error that Promise is a reserved identifier.
error(name, Diagnostics.Duplicate_identifier_0_Compiler_reserves_name_1_in_top_level_scope_of_a_module_containing_async_functions,
declarationNameToString(name), declarationNameToString(name));
}
}

function checkVarDeclaredNamesNotShadowed(node: VariableDeclaration | BindingElement) {
// - ScriptBody : StatementList
// It is a Syntax Error if any element of the LexicallyDeclaredNames of StatementList
Expand DownExpand Up@@ -13085,6 +13154,7 @@ namespace ts {
checkCollisionWithCapturedSuperVariable(node, <Identifier>node.name);
checkCollisionWithCapturedThisVariable(node, <Identifier>node.name);
checkCollisionWithRequireExportsInGeneratedCode(node, <Identifier>node.name);
checkCollisionWithGlobalPromiseInGeneratedCode(node, <Identifier>node.name);
}
}

Expand DownExpand Up@@ -13850,6 +13920,7 @@ namespace ts {
checkTypeNameIsReserved(node.name, Diagnostics.Class_name_cannot_be_0);
checkCollisionWithCapturedThisVariable(node, node.name);
checkCollisionWithRequireExportsInGeneratedCode(node, node.name);
checkCollisionWithGlobalPromiseInGeneratedCode(node, node.name);
}
checkTypeParameters(node.typeParameters);
checkExportsOnMergedDeclarations(node);
Expand DownExpand Up@@ -14356,6 +14427,7 @@ namespace ts {
checkTypeNameIsReserved(node.name, Diagnostics.Enum_name_cannot_be_0);
checkCollisionWithCapturedThisVariable(node, node.name);
checkCollisionWithRequireExportsInGeneratedCode(node, node.name);
checkCollisionWithGlobalPromiseInGeneratedCode(node, node.name);
checkExportsOnMergedDeclarations(node);

computeEnumMemberValues(node);
Expand DownExpand Up@@ -14460,6 +14532,7 @@ namespace ts {

checkCollisionWithCapturedThisVariable(node, node.name);
checkCollisionWithRequireExportsInGeneratedCode(node, node.name);
checkCollisionWithGlobalPromiseInGeneratedCode(node, node.name);
checkExportsOnMergedDeclarations(node);
const symbol = getSymbolOfNode(node);

Expand DownExpand Up@@ -14652,6 +14725,7 @@ namespace ts {
function checkImportBinding(node: ImportEqualsDeclaration | ImportClause | NamespaceImport | ImportSpecifier) {
checkCollisionWithCapturedThisVariable(node, node.name);
checkCollisionWithRequireExportsInGeneratedCode(node, node.name);
checkCollisionWithGlobalPromiseInGeneratedCode(node, node.name);
checkAliasSymbol(node);
}

Expand Down
10 changes: 9 additions & 1 deletion src/compiler/diagnosticMessages.json
Original file line numberDiff line numberDiff line change
Expand Up@@ -195,6 +195,10 @@
"category": "Error",
"code": 1063
},
"The return type of an async function or method must be the global Promise<T> type.": {
"category": "Error",
"code": 1064
},
"In ambient enum declarations member initializer must be constant expression.": {
"category": "Error",
"code": 1066
Expand DownExpand Up@@ -794,7 +798,7 @@
"A decorator can only decorate a method implementation, not an overload.": {
"category": "Error",
"code": 1249
},
},
"'with' statements are not allowed in an async function block.": {
"category": "Error",
"code": 1300
Expand DownExpand Up@@ -1687,6 +1691,10 @@
"category": "Error",
"code": 2528
},
"Duplicate identifier '{0}'. Compiler reserves name '{1}' in top level scope of a module containing async functions.": {

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The compiler reserves '{1}' in the top level scope of of a module when using async functions.

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is modeled after TS2441: Duplicate identifier '{0}'. Compiler reserves name '{1}' in top level scope of a module. See: https://github.com/Microsoft/TypeScript/blob/master/src/compiler/diagnosticMessages.json#L1350

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we should change that as well then.

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These also?

If you want to change the various messages we use for these cases, I'd rather we do that in a different PR.

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Also "when using async functions" is too broad. It only enforces this check if you use async functions in the current module, hence the wording: "in the top level scope of a module containing async functions."

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let's just do that in a different PR like you mentioned.

"category": "Error",
"code": 2529
},
"JSX element attributes type '{0}' may not be a union type.": {
"category": "Error",
"code": 2600
Expand Down
8 changes: 4 additions & 4 deletions src/compiler/emitter.ts
Original file line numberDiff line numberDiff line change
Expand Up@@ -320,7 +320,7 @@ var __param = (this && this.__param) || function (paramIndex, decorator) {

const awaiterHelper = `
var __awaiter = (this && this.__awaiter) || function (thisArg, _arguments, P, generator) {
return new P(function (resolve, reject) {
return new (P || (P = Promise))(function (resolve, reject) {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why can't use always just do new Promise here?

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We are leaving support for custom Promise return types for ES5 and earlier which do not have a global Promise, so we want to keep the same argument list. The default behavior now will be to use the Promise captured in the same scope as __awaiter. This is to avoid issues when creating closures in namespaces:

// input.tsexportnamespacex{varPromise="Not a Promise constructor.";asyncfunctionf(): Promise{// references global Promise type}}// output.jsvar__awaiter= ...;// captures global Promise hereexportvarx=(function(x){varPromise="Not a Promise constructor.";// local Promise var shadows global Promisefunctionf(){// Before this change, we would capture Promise here, which would be the wrong Promise:// return __awaiter(this, void 0, Promise, function *() { });// After this change, we use the Promise captured at the top-level of the module.return__awaiter(this,void0,void0,function*(){});}})(x||(x={}));

function fulfilled(value) { try { step(generator.next(value)); } catch (e) { reject(e); } }
function rejected(value) { try { step(generator.throw(value)); } catch (e) { reject(e); } }
function step(result) { result.done ? resolve(result.value) : new P(function (resolve) { resolve(result.value); }).then(fulfilled, rejected); }
Expand DownExpand Up@@ -4531,11 +4531,11 @@ const _super = (function (geti, seti) {
write(", void 0, ");
}

if (promiseConstructor) {
emitEntityNameAsExpression(promiseConstructor, /*useFallback*/ false);
if (languageVersion >= ScriptTarget.ES6 || !promiseConstructor) {
write("void 0");
}
else {
write("Promise");
emitEntityNameAsExpression(promiseConstructor, /*useFallback*/ false);
}

// Emit the call to __awaiter.
Expand Down
10 changes: 0 additions & 10 deletions tests/baselines/reference/asyncAliasReturnType_es6.errors.txt

This file was deleted.

2 changes: 1 addition & 1 deletion tests/baselines/reference/asyncAliasReturnType_es6.js
Original file line numberDiff line numberDiff line change
Expand Up@@ -6,6 +6,6 @@ async function f(): PromiseAlias<void> {

//// [asyncAliasReturnType_es6.js]
function f() {
return __awaiter(this, void 0, PromiseAlias, function* () {
return __awaiter(this, void 0, void 0, function* () {
});
}
11 changes: 11 additions & 0 deletions tests/baselines/reference/asyncAliasReturnType_es6.symbols
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,11 @@
=== tests/cases/conformance/async/es6/asyncAliasReturnType_es6.ts ===
type PromiseAlias<T> = Promise<T>;
>PromiseAlias : Symbol(PromiseAlias, Decl(asyncAliasReturnType_es6.ts, 0, 0))
>T : Symbol(T, Decl(asyncAliasReturnType_es6.ts, 0, 18))
>Promise : Symbol(Promise, Decl(lib.d.ts, --, --), Decl(lib.d.ts, --, --))
>T : Symbol(T, Decl(asyncAliasReturnType_es6.ts, 0, 18))

async function f(): PromiseAlias<void> {
>f : Symbol(f, Decl(asyncAliasReturnType_es6.ts, 0, 34))
>PromiseAlias : Symbol(PromiseAlias, Decl(asyncAliasReturnType_es6.ts, 0, 0))
}
11 changes: 11 additions & 0 deletions tests/baselines/reference/asyncAliasReturnType_es6.types
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,11 @@
=== tests/cases/conformance/async/es6/asyncAliasReturnType_es6.ts ===
type PromiseAlias<T> = Promise<T>;
>PromiseAlias : Promise<T>
>T : T
>Promise : Promise<T>
>T : T

async function f(): PromiseAlias<void> {
>f : () => Promise<void>
>PromiseAlias : Promise<T>
}
2 changes: 1 addition & 1 deletion tests/baselines/reference/asyncArrowFunction1_es6.js
Original file line numberDiff line numberDiff line change
Expand Up@@ -4,5 +4,5 @@ var foo = async (): Promise<void> => {
};

//// [asyncArrowFunction1_es6.js]
var foo = () => __awaiter(this, void 0, Promise, function* () {
var foo = () => __awaiter(this, void 0, void 0, function* () {
});
2 changes: 1 addition & 1 deletion tests/baselines/reference/asyncArrowFunction6_es6.js
Original file line numberDiff line numberDiff line change
Expand Up@@ -4,5 +4,5 @@ var foo = async (a = await): Promise<void> => {
}

//// [asyncArrowFunction6_es6.js]
var foo = (a = yield ) => __awaiter(this, void 0, Promise, function* () {
var foo = (a = yield ) => __awaiter(this, void 0, void 0, function* () {
});
4 changes: 2 additions & 2 deletions tests/baselines/reference/asyncArrowFunction7_es6.js
Original file line numberDiff line numberDiff line change
Expand Up@@ -7,8 +7,8 @@ var bar = async (): Promise<void> => {
}

//// [asyncArrowFunction7_es6.js]
var bar = () => __awaiter(this, void 0, Promise, function* () {
var bar = () => __awaiter(this, void 0, void 0, function* () {
// 'await' here is an identifier, and not an await expression.
var foo = (a = yield ) => __awaiter(this, void 0, Promise, function* () {
var foo = (a = yield ) => __awaiter(this, void 0, void 0, function* () {
});
});
2 changes: 1 addition & 1 deletion tests/baselines/reference/asyncArrowFunction8_es6.js
Original file line numberDiff line numberDiff line change
Expand Up@@ -5,6 +5,6 @@ var foo = async (): Promise<void> => {
}

//// [asyncArrowFunction8_es6.js]
var foo = () => __awaiter(this, void 0, Promise, function* () {
var foo = () => __awaiter(this, void 0, void 0, function* () {
var v = { [yield ]: foo };
});
Original file line numberDiff line numberDiff line change
Expand Up@@ -11,6 +11,6 @@ class C {
class C {
method() {
function other() { }
var fn = () => __awaiter(this, arguments, Promise, function* () { return yield other.apply(this, arguments); });
var fn = () => __awaiter(this, arguments, void 0, function* () { return yield other.apply(this, arguments); });
}
}
Original file line numberDiff line numberDiff line change
Expand Up@@ -9,6 +9,6 @@ class C {
//// [asyncArrowFunctionCapturesThis_es6.js]
class C {
method() {
var fn = () => __awaiter(this, void 0, Promise, function* () { return yield this; });
var fn = () => __awaiter(this, void 0, void 0, function* () { return yield this; });
}
}
Loading
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Auto-enable theater mode on YouTube\n(function() {\n function tryTheater() {\n var btn = document.querySelector('button[aria-label=\"Theater mode\"], ytd-player #player button[title=\"Theater mode\"]');\n if (btn && !btn.classList.contains('activated')) {\n btn.click();\n }\n }\n \n // Try immediately\n tryTheater();\n \n // Try after navigation (SPA)\n var lastUrl = location.href;\n setInterval(function() {\n if (location.href !== lastUrl) {\n lastUrl = location.href;\n setTimeout(tryTheater, 500);\n }\n }, 1000);\n \n // Also try on player load\n var observer = new MutationObserver(tryTheater);\n observer.observe(document.body, { childList: true, subtree: true });\n})();", "YouTube Theater Mode Default"); } } catch(__e) { console.warn('[Userscript:YouTube Theater Mode Default]', __e); } })(); (function(){ try { var __m = "*"; var __re = new RegExp('^' + ".*" + '
Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
90 changes: 82 additions & 8 deletions src/compiler/checker.ts
Original file line numberDiff line numberDiff line change
Expand Up@@ -2473,10 +2473,21 @@ namespace ts {

function getDeclarationContainer(node: Node): Node {
node = getRootDeclaration(node);
while (node) {
switch (node.kind) {
case SyntaxKind.VariableDeclaration:
case SyntaxKind.VariableDeclarationList:
case SyntaxKind.ImportSpecifier:
case SyntaxKind.NamedImports:
case SyntaxKind.NamespaceImport:
case SyntaxKind.ImportClause:
node = node.parent;
break;

// Parent chain:
// VaribleDeclaration -> VariableDeclarationList -> VariableStatement -> 'Declaration Container'
return node.kind === SyntaxKind.VariableDeclaration ? node.parent.parent.parent : node.parent;
default:
return node.parent;
}
}
}

function getTypeOfPrototypeProperty(prototype: Symbol): Type {
Expand DownExpand Up@@ -2618,7 +2629,7 @@ namespace ts {
}
}
else if (declaration.kind === SyntaxKind.Parameter) {
// If it's a parameter, see if the parent has a jsdoc comment with an @param
// If it's a parameter, see if the parent has a jsdoc comment with an @param
// annotation.
const paramTag = getCorrespondingJSDocParameterTag(<ParameterDeclaration>declaration);
if (paramTag && paramTag.typeExpression) {
Expand All@@ -2633,7 +2644,7 @@ namespace ts {
function getTypeForVariableLikeDeclaration(declaration: VariableLikeDeclaration): Type {
if (declaration.parserContextFlags & ParserContextFlags.JavaScriptFile) {
// If this is a variable in a JavaScript file, then use the JSDoc type (if it has
// one as its type), otherwise fallback to the below standard TS codepaths to
// one as its type), otherwise fallback to the below standard TS codepaths to
// try to figure it out.
const type = getTypeForVariableLikeDeclarationFromJSDocComment(declaration);
if (type && type !== unknownType) {
Expand DownExpand Up@@ -4058,7 +4069,7 @@ namespace ts {
const isJSConstructSignature = isJSDocConstructSignature(declaration);
let returnType: Type = undefined;

// If this is a JSDoc construct signature, then skip the first parameter in the
// If this is a JSDoc construct signature, then skip the first parameter in the
// parameter list. The first parameter represents the return type of the construct
// signature.
for (let i = isJSConstructSignature ? 1 : 0, n = declaration.parameters.length; i < n; i++) {
Expand DownExpand Up@@ -4461,7 +4472,7 @@ namespace ts {
}

if (symbol.flags & SymbolFlags.Value && node.kind === SyntaxKind.JSDocTypeReference) {
// A JSDocTypeReference may have resolved to a value (as opposed to a type). In
// A JSDocTypeReference may have resolved to a value (as opposed to a type). In
// that case, the type of this reference is just the type of the value we resolved
// to.
return getTypeOfSymbol(symbol);
Expand DownExpand Up@@ -10468,7 +10479,7 @@ namespace ts {

/*
*TypeScript Specification 1.0 (6.3) - July 2014
* An explicitly typed function whose return type isn't the Void type,
* An explicitly typed function whose return type isn't the Void type,
* the Any type, or a union type containing the Void or Any type as a constituent
* must have at least one return statement somewhere in its body.
* An exception to this rule is if the function implementation consists of a single 'throw' statement.
Expand DownExpand Up@@ -11625,6 +11636,9 @@ namespace ts {
checkTypeAssignableTo(iterableIteratorInstantiation, returnType, node.type);
}
}
else if (isAsyncFunctionLike(node)) {
checkAsyncFunctionReturnType(<FunctionLikeDeclaration>node);
}
}
}

Expand DownExpand Up@@ -12494,6 +12508,36 @@ namespace ts {
}
}

/**
* Checks that the return type provided is an instantiation of the global Promise<T> type

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What is the reason for the compiler to reject type annotations that are known compatible with Promise? This won't catch any type errors, but rejects valid annotations. Generator functions handle this consistently IMO. Why not use the same approach here for consistency?

function*bar(){}// Returns IterableIterator<any>function*bar2(): any{}// OK: compatible annotationfunction*bar3(): Iterator<any>{}// OK: comtatible annotationfunction*bar4(): {next}{}// OK: comtatible annotationasyncfunctionfoo(){}// Returns Promise<void>asyncfunctionfoo1(): any{}// ERROR with this PR, but why?asyncfunctionfoo2(): PromiseLike<any>{}// ERROR with this PR. but why?

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The checker.ts code for generator functions uses checkTypeAssignableTo to check the return type annotation (L11625). Why is checkTypeAssignableTo not used for async functions?

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is the result of an offline discussion with Mohamed Hegazy (@mhegazy). Supporting an assignable interface is not currently compatible with our goals for future down-level support for async functions for ES5. As a result, async functions will for the time being be more restrictive than generators with respect to the return type annotation.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ES3/5 restrictions need not be applied to ES6 code. Why not this:

if(languageVersion>=ScriptTarget.ES6){// use checkTypeAssignableTo to check return type annotation}else{// more restrictive return type annotation rules for ES3/5...}

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Mohamed Hegazy (@mhegazy) you gave qualitied support back in December for treating return type annotations one way for ES3/5 (improve the error message), and another way for ES6+ (use checkTypeAssignableTo). Is this still the case?

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The current support we have in TypeScript 1.7.x for async functions allows you to specify a custom Promise implementation in the return type annotation. This was intended to support runtimes that do not yet define a global Promise implementation, and was primarily intended to support down-level emit for async functions in ES5, which is a goal for TypeScript 2.0. This allows you to write an async function in the following fashion:

import*asbluebirdfrom'bluebird';exportasyncfunctionfn(): bluebird.Promise{}console.log(fn()instanceofbluebird.Promise);// true 

However, this approach is not compatible with the ES7 approach to async functions which only leverages the native Promise implementation. As this native Promise is specified as part of ES6, we have elected to only allow you to supply a global Promise<T> return type for ES6, but reserve the previous functionality to eventually support async functions down-level.

If we were to then allow you to specify the return type of an async function as any type to which Promise<T> is assignable, then the above sample would still compile successfully, due to how "bluebird.d.ts" is currently implemented on DefinitelyTyped. The result is that upgrading from TypeScript 1.7.x to TypeScript 1.8 would result in completely different runtime semantics with no indication to the developer that anything had changed.

import*asbluebirdfrom'bluebird';exportasyncfunctionfn(): bluebird.Promise{}console.log(fn()instanceofbluebird.Promise);// false?! 

By restricting the return type annotation to only the global Promise<T> type, we have the ability to loosen this restriction in the future. Yes, this does not fully align with how we support type annotations on generator functions; however, this is the only reliable way forward. We may, in a future release, opt to add a different mechanism for defining a compatible Promise implementation for ES5 async functions. If that does indeed become the case, we would be able to loosen the return type restriction.

As this is a breaking change from TypeScript 1.7 behavior, it also is best to be more conservative for one to two releases to ensure any existing implementations have adjusted to this change before examining the feasibility of relaxing this restriction.

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The fact that the return type must be strictly equal to Promise might become a hindrance in scenarios involving interfaces and/or methods overriding.

You can work around it by adding a layer of indirection (like all CS problems 😉) but it doesn't feel nice.

Here's one example: in frameworks such as Aurelia, many methods can return a value or a Promise for a value. Not mandating a Promise is both convenient when implementing trivial methods (like canSave() { return true; }) and good for perf.

Say I have a base class that exposes such a function and is meant to be overriden. Assume that the base default implementation is async.

The natural way is this:

classBaseViewModel{asyncisValid() : boolean|Promise<boolean>{/* some async implementation, maybe server call */}}classDerivedViewModelextendsBaseViewModel{isValid() : boolean|Promise<boolean>{returntrue;// Non-async implementation}}

but this wouldn't work :(

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

jods (@jods4) good real world example. Current behaviour:

varq: ()=>boolean|Promise<boolean>;q=()=>true;// OKq=()=>Promise.resolve(true);// OKq=async()=>true;// OK(): boolean|Promise<boolean>=>true;// OK(): boolean|Promise<boolean>=>Promise.resolve(true);// OKasync(): boolean|Promise<boolean>=>true;// ERROR

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

jods (@jods4) since this PR is done and closed may I suggest taking your example across to #6686 where I think it would be quite relevent.

* and returns the awaited type of the return type.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

since it is already jsdoc. could you add the parameter annotation as well?

*
* @param returnType The return type of a FunctionLikeDeclaration
* @param location The node on which to report the error.
*/
function checkCorrectPromiseType(returnType: Type, location: Node) {
if (returnType === unknownType) {
// The return type already had some other error, so we ignore and return
// the unknown type.
return unknownType;
}

const globalPromiseType = getGlobalPromiseType();
if (globalPromiseType === emptyGenericType
|| globalPromiseType === getTargetType(returnType)) {
// Either we couldn't resolve the global promise type, which would have already
// reported an error, or we could resolve it and the return type is a valid type
// reference to the global type. In either case, we return the awaited type for
// the return type.
return checkAwaitedType(returnType, location, Diagnostics.An_async_function_or_method_must_have_a_valid_awaitable_return_type);
}

// The promise type was not a valid type reference to the global promise type, so we
// report an error and return the unknown type.
error(location, Diagnostics.The_return_type_of_an_async_function_or_method_must_be_the_global_Promise_T_type);
return unknownType;
}

/**
* Checks the return type of an async function to ensure it is a compatible
* Promise implementation.
Expand All@@ -12508,6 +12552,11 @@ namespace ts {
* callable `then` signature.
*/
function checkAsyncFunctionReturnType(node: FunctionLikeDeclaration): Type {
if (languageVersion >= ScriptTarget.ES6) {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why >= ES6? I thought need to resolve global Promise when down-leveling?

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ron Buckton (@rbuckton) explains off-line. ES6 has native Promise. I misunderstood that fact

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ES5 and earlier do not have a global Promise. We fall back to the existing behavior for ES5 as we plan to support async functions via tree transformation for target ES5 and users will need to be able to supply a Promise implementation.

const returnType = getTypeFromTypeNode(node.type);
return checkCorrectPromiseType(returnType, node.type);
}

const globalPromiseConstructorLikeType = getGlobalPromiseConstructorLikeType();
if (globalPromiseConstructorLikeType === emptyObjectType) {
// If we couldn't resolve the global PromiseConstructorLike type we cannot verify
Expand DownExpand Up@@ -12723,6 +12772,7 @@ namespace ts {
checkCollisionWithCapturedSuperVariable(node, node.name);
checkCollisionWithCapturedThisVariable(node, node.name);
checkCollisionWithRequireExportsInGeneratedCode(node, node.name);
checkCollisionWithGlobalPromiseInGeneratedCode(node, node.name);
}
}

Expand DownExpand Up@@ -12907,6 +12957,25 @@ namespace ts {
}
}

function checkCollisionWithGlobalPromiseInGeneratedCode(node: Node, name: Identifier): void {
if (!needCollisionCheckForIdentifier(node, name, "Promise")) {
return;
}

// Uninstantiated modules shouldnt do this check
if (node.kind === SyntaxKind.ModuleDeclaration && getModuleInstanceState(node) !== ModuleInstanceState.Instantiated) {
return;
}

// In case of variable declaration, node.parent is variable statement so look at the variable statement's parent
const parent = getDeclarationContainer(node);
if (parent.kind === SyntaxKind.SourceFile && isExternalOrCommonJsModule(<SourceFile>parent) && parent.flags & NodeFlags.HasAsyncFunctions) {
// If the declaration happens to be in external module, report error that Promise is a reserved identifier.
error(name, Diagnostics.Duplicate_identifier_0_Compiler_reserves_name_1_in_top_level_scope_of_a_module_containing_async_functions,
declarationNameToString(name), declarationNameToString(name));
}
}

function checkVarDeclaredNamesNotShadowed(node: VariableDeclaration | BindingElement) {
// - ScriptBody : StatementList
// It is a Syntax Error if any element of the LexicallyDeclaredNames of StatementList
Expand DownExpand Up@@ -13085,6 +13154,7 @@ namespace ts {
checkCollisionWithCapturedSuperVariable(node, <Identifier>node.name);
checkCollisionWithCapturedThisVariable(node, <Identifier>node.name);
checkCollisionWithRequireExportsInGeneratedCode(node, <Identifier>node.name);
checkCollisionWithGlobalPromiseInGeneratedCode(node, <Identifier>node.name);
}
}

Expand DownExpand Up@@ -13850,6 +13920,7 @@ namespace ts {
checkTypeNameIsReserved(node.name, Diagnostics.Class_name_cannot_be_0);
checkCollisionWithCapturedThisVariable(node, node.name);
checkCollisionWithRequireExportsInGeneratedCode(node, node.name);
checkCollisionWithGlobalPromiseInGeneratedCode(node, node.name);
}
checkTypeParameters(node.typeParameters);
checkExportsOnMergedDeclarations(node);
Expand DownExpand Up@@ -14356,6 +14427,7 @@ namespace ts {
checkTypeNameIsReserved(node.name, Diagnostics.Enum_name_cannot_be_0);
checkCollisionWithCapturedThisVariable(node, node.name);
checkCollisionWithRequireExportsInGeneratedCode(node, node.name);
checkCollisionWithGlobalPromiseInGeneratedCode(node, node.name);
checkExportsOnMergedDeclarations(node);

computeEnumMemberValues(node);
Expand DownExpand Up@@ -14460,6 +14532,7 @@ namespace ts {

checkCollisionWithCapturedThisVariable(node, node.name);
checkCollisionWithRequireExportsInGeneratedCode(node, node.name);
checkCollisionWithGlobalPromiseInGeneratedCode(node, node.name);
checkExportsOnMergedDeclarations(node);
const symbol = getSymbolOfNode(node);

Expand DownExpand Up@@ -14652,6 +14725,7 @@ namespace ts {
function checkImportBinding(node: ImportEqualsDeclaration | ImportClause | NamespaceImport | ImportSpecifier) {
checkCollisionWithCapturedThisVariable(node, node.name);
checkCollisionWithRequireExportsInGeneratedCode(node, node.name);
checkCollisionWithGlobalPromiseInGeneratedCode(node, node.name);
checkAliasSymbol(node);
}

Expand Down
10 changes: 9 additions & 1 deletion src/compiler/diagnosticMessages.json
Original file line numberDiff line numberDiff line change
Expand Up@@ -195,6 +195,10 @@
"category": "Error",
"code": 1063
},
"The return type of an async function or method must be the global Promise<T> type.": {
"category": "Error",
"code": 1064
},
"In ambient enum declarations member initializer must be constant expression.": {
"category": "Error",
"code": 1066
Expand DownExpand Up@@ -794,7 +798,7 @@
"A decorator can only decorate a method implementation, not an overload.": {
"category": "Error",
"code": 1249
},
},
"'with' statements are not allowed in an async function block.": {
"category": "Error",
"code": 1300
Expand DownExpand Up@@ -1687,6 +1691,10 @@
"category": "Error",
"code": 2528
},
"Duplicate identifier '{0}'. Compiler reserves name '{1}' in top level scope of a module containing async functions.": {

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The compiler reserves '{1}' in the top level scope of of a module when using async functions.

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is modeled after TS2441: Duplicate identifier '{0}'. Compiler reserves name '{1}' in top level scope of a module. See: https://github.com/Microsoft/TypeScript/blob/master/src/compiler/diagnosticMessages.json#L1350

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we should change that as well then.

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These also?

If you want to change the various messages we use for these cases, I'd rather we do that in a different PR.

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Also "when using async functions" is too broad. It only enforces this check if you use async functions in the current module, hence the wording: "in the top level scope of a module containing async functions."

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let's just do that in a different PR like you mentioned.

"category": "Error",
"code": 2529
},
"JSX element attributes type '{0}' may not be a union type.": {
"category": "Error",
"code": 2600
Expand Down
8 changes: 4 additions & 4 deletions src/compiler/emitter.ts
Original file line numberDiff line numberDiff line change
Expand Up@@ -320,7 +320,7 @@ var __param = (this && this.__param) || function (paramIndex, decorator) {

const awaiterHelper = `
var __awaiter = (this && this.__awaiter) || function (thisArg, _arguments, P, generator) {
return new P(function (resolve, reject) {
return new (P || (P = Promise))(function (resolve, reject) {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why can't use always just do new Promise here?

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We are leaving support for custom Promise return types for ES5 and earlier which do not have a global Promise, so we want to keep the same argument list. The default behavior now will be to use the Promise captured in the same scope as __awaiter. This is to avoid issues when creating closures in namespaces:

// input.tsexportnamespacex{varPromise="Not a Promise constructor.";asyncfunctionf(): Promise{// references global Promise type}}// output.jsvar__awaiter= ...;// captures global Promise hereexportvarx=(function(x){varPromise="Not a Promise constructor.";// local Promise var shadows global Promisefunctionf(){// Before this change, we would capture Promise here, which would be the wrong Promise:// return __awaiter(this, void 0, Promise, function *() { });// After this change, we use the Promise captured at the top-level of the module.return__awaiter(this,void0,void0,function*(){});}})(x||(x={}));

function fulfilled(value) { try { step(generator.next(value)); } catch (e) { reject(e); } }
function rejected(value) { try { step(generator.throw(value)); } catch (e) { reject(e); } }
function step(result) { result.done ? resolve(result.value) : new P(function (resolve) { resolve(result.value); }).then(fulfilled, rejected); }
Expand DownExpand Up@@ -4531,11 +4531,11 @@ const _super = (function (geti, seti) {
write(", void 0, ");
}

if (promiseConstructor) {
emitEntityNameAsExpression(promiseConstructor, /*useFallback*/ false);
if (languageVersion >= ScriptTarget.ES6 || !promiseConstructor) {
write("void 0");
}
else {
write("Promise");
emitEntityNameAsExpression(promiseConstructor, /*useFallback*/ false);
}

// Emit the call to __awaiter.
Expand Down
10 changes: 0 additions & 10 deletions tests/baselines/reference/asyncAliasReturnType_es6.errors.txt

This file was deleted.

2 changes: 1 addition & 1 deletion tests/baselines/reference/asyncAliasReturnType_es6.js
Original file line numberDiff line numberDiff line change
Expand Up@@ -6,6 +6,6 @@ async function f(): PromiseAlias<void> {

//// [asyncAliasReturnType_es6.js]
function f() {
return __awaiter(this, void 0, PromiseAlias, function* () {
return __awaiter(this, void 0, void 0, function* () {
});
}
11 changes: 11 additions & 0 deletions tests/baselines/reference/asyncAliasReturnType_es6.symbols
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,11 @@
=== tests/cases/conformance/async/es6/asyncAliasReturnType_es6.ts ===
type PromiseAlias<T> = Promise<T>;
>PromiseAlias : Symbol(PromiseAlias, Decl(asyncAliasReturnType_es6.ts, 0, 0))
>T : Symbol(T, Decl(asyncAliasReturnType_es6.ts, 0, 18))
>Promise : Symbol(Promise, Decl(lib.d.ts, --, --), Decl(lib.d.ts, --, --))
>T : Symbol(T, Decl(asyncAliasReturnType_es6.ts, 0, 18))

async function f(): PromiseAlias<void> {
>f : Symbol(f, Decl(asyncAliasReturnType_es6.ts, 0, 34))
>PromiseAlias : Symbol(PromiseAlias, Decl(asyncAliasReturnType_es6.ts, 0, 0))
}
11 changes: 11 additions & 0 deletions tests/baselines/reference/asyncAliasReturnType_es6.types
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,11 @@
=== tests/cases/conformance/async/es6/asyncAliasReturnType_es6.ts ===
type PromiseAlias<T> = Promise<T>;
>PromiseAlias : Promise<T>
>T : T
>Promise : Promise<T>
>T : T

async function f(): PromiseAlias<void> {
>f : () => Promise<void>
>PromiseAlias : Promise<T>
}
2 changes: 1 addition & 1 deletion tests/baselines/reference/asyncArrowFunction1_es6.js
Original file line numberDiff line numberDiff line change
Expand Up@@ -4,5 +4,5 @@ var foo = async (): Promise<void> => {
};

//// [asyncArrowFunction1_es6.js]
var foo = () => __awaiter(this, void 0, Promise, function* () {
var foo = () => __awaiter(this, void 0, void 0, function* () {
});
2 changes: 1 addition & 1 deletion tests/baselines/reference/asyncArrowFunction6_es6.js
Original file line numberDiff line numberDiff line change
Expand Up@@ -4,5 +4,5 @@ var foo = async (a = await): Promise<void> => {
}

//// [asyncArrowFunction6_es6.js]
var foo = (a = yield ) => __awaiter(this, void 0, Promise, function* () {
var foo = (a = yield ) => __awaiter(this, void 0, void 0, function* () {
});
4 changes: 2 additions & 2 deletions tests/baselines/reference/asyncArrowFunction7_es6.js
Original file line numberDiff line numberDiff line change
Expand Up@@ -7,8 +7,8 @@ var bar = async (): Promise<void> => {
}

//// [asyncArrowFunction7_es6.js]
var bar = () => __awaiter(this, void 0, Promise, function* () {
var bar = () => __awaiter(this, void 0, void 0, function* () {
// 'await' here is an identifier, and not an await expression.
var foo = (a = yield ) => __awaiter(this, void 0, Promise, function* () {
var foo = (a = yield ) => __awaiter(this, void 0, void 0, function* () {
});
});
2 changes: 1 addition & 1 deletion tests/baselines/reference/asyncArrowFunction8_es6.js
Original file line numberDiff line numberDiff line change
Expand Up@@ -5,6 +5,6 @@ var foo = async (): Promise<void> => {
}

//// [asyncArrowFunction8_es6.js]
var foo = () => __awaiter(this, void 0, Promise, function* () {
var foo = () => __awaiter(this, void 0, void 0, function* () {
var v = { [yield ]: foo };
});
Original file line numberDiff line numberDiff line change
Expand Up@@ -11,6 +11,6 @@ class C {
class C {
method() {
function other() { }
var fn = () => __awaiter(this, arguments, Promise, function* () { return yield other.apply(this, arguments); });
var fn = () => __awaiter(this, arguments, void 0, function* () { return yield other.apply(this, arguments); });
}
}
Original file line numberDiff line numberDiff line change
Expand Up@@ -9,6 +9,6 @@ class C {
//// [asyncArrowFunctionCapturesThis_es6.js]
class C {
method() {
var fn = () => __awaiter(this, void 0, Promise, function* () { return yield this; });
var fn = () => __awaiter(this, void 0, void 0, function* () { return yield this; });
}
}
Loading
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Remove or un-stick sticky/fixed headers that block content\n(function() {\n function unstick() {\n document.querySelectorAll('header, nav, [role=\"banner\"], .header, .navbar, .sticky, .fixed-top, [style*=\"position: fixed\"], [style*=\"position:sticky\"]').forEach(function(el) {\n if (el.style.position === 'fixed' || el.style.position === 'sticky' || \n getComputedStyle(el).position === 'fixed' || getComputedStyle(el).position === 'sticky') {\n el.style.position = 'static';\n el.style.top = 'auto';\n el.style.zIndex = 'auto';\n }\n });\n }\n \n unstick();\n \n var observer = new MutationObserver(unstick);\n observer.observe(document.body, { childList: true, subtree: true, attributes: true, attributeFilter: ['style', 'class'] });\n})();", "Kill Sticky Headers"); } } catch(__e) { console.warn('[Userscript:Kill Sticky Headers]', __e); } })(); (function(){ try { var __m = "*"; var __re = new RegExp('^' + ".*" + '
Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
90 changes: 82 additions & 8 deletions src/compiler/checker.ts
Original file line numberDiff line numberDiff line change
Expand Up@@ -2473,10 +2473,21 @@ namespace ts {

function getDeclarationContainer(node: Node): Node {
node = getRootDeclaration(node);
while (node) {
switch (node.kind) {
case SyntaxKind.VariableDeclaration:
case SyntaxKind.VariableDeclarationList:
case SyntaxKind.ImportSpecifier:
case SyntaxKind.NamedImports:
case SyntaxKind.NamespaceImport:
case SyntaxKind.ImportClause:
node = node.parent;
break;

// Parent chain:
// VaribleDeclaration -> VariableDeclarationList -> VariableStatement -> 'Declaration Container'
return node.kind === SyntaxKind.VariableDeclaration ? node.parent.parent.parent : node.parent;
default:
return node.parent;
}
}
}

function getTypeOfPrototypeProperty(prototype: Symbol): Type {
Expand DownExpand Up@@ -2618,7 +2629,7 @@ namespace ts {
}
}
else if (declaration.kind === SyntaxKind.Parameter) {
// If it's a parameter, see if the parent has a jsdoc comment with an @param
// If it's a parameter, see if the parent has a jsdoc comment with an @param
// annotation.
const paramTag = getCorrespondingJSDocParameterTag(<ParameterDeclaration>declaration);
if (paramTag && paramTag.typeExpression) {
Expand All@@ -2633,7 +2644,7 @@ namespace ts {
function getTypeForVariableLikeDeclaration(declaration: VariableLikeDeclaration): Type {
if (declaration.parserContextFlags & ParserContextFlags.JavaScriptFile) {
// If this is a variable in a JavaScript file, then use the JSDoc type (if it has
// one as its type), otherwise fallback to the below standard TS codepaths to
// one as its type), otherwise fallback to the below standard TS codepaths to
// try to figure it out.
const type = getTypeForVariableLikeDeclarationFromJSDocComment(declaration);
if (type && type !== unknownType) {
Expand DownExpand Up@@ -4058,7 +4069,7 @@ namespace ts {
const isJSConstructSignature = isJSDocConstructSignature(declaration);
let returnType: Type = undefined;

// If this is a JSDoc construct signature, then skip the first parameter in the
// If this is a JSDoc construct signature, then skip the first parameter in the
// parameter list. The first parameter represents the return type of the construct
// signature.
for (let i = isJSConstructSignature ? 1 : 0, n = declaration.parameters.length; i < n; i++) {
Expand DownExpand Up@@ -4461,7 +4472,7 @@ namespace ts {
}

if (symbol.flags & SymbolFlags.Value && node.kind === SyntaxKind.JSDocTypeReference) {
// A JSDocTypeReference may have resolved to a value (as opposed to a type). In
// A JSDocTypeReference may have resolved to a value (as opposed to a type). In
// that case, the type of this reference is just the type of the value we resolved
// to.
return getTypeOfSymbol(symbol);
Expand DownExpand Up@@ -10468,7 +10479,7 @@ namespace ts {

/*
*TypeScript Specification 1.0 (6.3) - July 2014
* An explicitly typed function whose return type isn't the Void type,
* An explicitly typed function whose return type isn't the Void type,
* the Any type, or a union type containing the Void or Any type as a constituent
* must have at least one return statement somewhere in its body.
* An exception to this rule is if the function implementation consists of a single 'throw' statement.
Expand DownExpand Up@@ -11625,6 +11636,9 @@ namespace ts {
checkTypeAssignableTo(iterableIteratorInstantiation, returnType, node.type);
}
}
else if (isAsyncFunctionLike(node)) {
checkAsyncFunctionReturnType(<FunctionLikeDeclaration>node);
}
}
}

Expand DownExpand Up@@ -12494,6 +12508,36 @@ namespace ts {
}
}

/**
* Checks that the return type provided is an instantiation of the global Promise<T> type

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What is the reason for the compiler to reject type annotations that are known compatible with Promise? This won't catch any type errors, but rejects valid annotations. Generator functions handle this consistently IMO. Why not use the same approach here for consistency?

function*bar(){}// Returns IterableIterator<any>function*bar2(): any{}// OK: compatible annotationfunction*bar3(): Iterator<any>{}// OK: comtatible annotationfunction*bar4(): {next}{}// OK: comtatible annotationasyncfunctionfoo(){}// Returns Promise<void>asyncfunctionfoo1(): any{}// ERROR with this PR, but why?asyncfunctionfoo2(): PromiseLike<any>{}// ERROR with this PR. but why?

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The checker.ts code for generator functions uses checkTypeAssignableTo to check the return type annotation (L11625). Why is checkTypeAssignableTo not used for async functions?

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is the result of an offline discussion with Mohamed Hegazy (@mhegazy). Supporting an assignable interface is not currently compatible with our goals for future down-level support for async functions for ES5. As a result, async functions will for the time being be more restrictive than generators with respect to the return type annotation.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ES3/5 restrictions need not be applied to ES6 code. Why not this:

if(languageVersion>=ScriptTarget.ES6){// use checkTypeAssignableTo to check return type annotation}else{// more restrictive return type annotation rules for ES3/5...}

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Mohamed Hegazy (@mhegazy) you gave qualitied support back in December for treating return type annotations one way for ES3/5 (improve the error message), and another way for ES6+ (use checkTypeAssignableTo). Is this still the case?

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The current support we have in TypeScript 1.7.x for async functions allows you to specify a custom Promise implementation in the return type annotation. This was intended to support runtimes that do not yet define a global Promise implementation, and was primarily intended to support down-level emit for async functions in ES5, which is a goal for TypeScript 2.0. This allows you to write an async function in the following fashion:

import*asbluebirdfrom'bluebird';exportasyncfunctionfn(): bluebird.Promise{}console.log(fn()instanceofbluebird.Promise);// true 

However, this approach is not compatible with the ES7 approach to async functions which only leverages the native Promise implementation. As this native Promise is specified as part of ES6, we have elected to only allow you to supply a global Promise<T> return type for ES6, but reserve the previous functionality to eventually support async functions down-level.

If we were to then allow you to specify the return type of an async function as any type to which Promise<T> is assignable, then the above sample would still compile successfully, due to how "bluebird.d.ts" is currently implemented on DefinitelyTyped. The result is that upgrading from TypeScript 1.7.x to TypeScript 1.8 would result in completely different runtime semantics with no indication to the developer that anything had changed.

import*asbluebirdfrom'bluebird';exportasyncfunctionfn(): bluebird.Promise{}console.log(fn()instanceofbluebird.Promise);// false?! 

By restricting the return type annotation to only the global Promise<T> type, we have the ability to loosen this restriction in the future. Yes, this does not fully align with how we support type annotations on generator functions; however, this is the only reliable way forward. We may, in a future release, opt to add a different mechanism for defining a compatible Promise implementation for ES5 async functions. If that does indeed become the case, we would be able to loosen the return type restriction.

As this is a breaking change from TypeScript 1.7 behavior, it also is best to be more conservative for one to two releases to ensure any existing implementations have adjusted to this change before examining the feasibility of relaxing this restriction.

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The fact that the return type must be strictly equal to Promise might become a hindrance in scenarios involving interfaces and/or methods overriding.

You can work around it by adding a layer of indirection (like all CS problems 😉) but it doesn't feel nice.

Here's one example: in frameworks such as Aurelia, many methods can return a value or a Promise for a value. Not mandating a Promise is both convenient when implementing trivial methods (like canSave() { return true; }) and good for perf.

Say I have a base class that exposes such a function and is meant to be overriden. Assume that the base default implementation is async.

The natural way is this:

classBaseViewModel{asyncisValid() : boolean|Promise<boolean>{/* some async implementation, maybe server call */}}classDerivedViewModelextendsBaseViewModel{isValid() : boolean|Promise<boolean>{returntrue;// Non-async implementation}}

but this wouldn't work :(

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

jods (@jods4) good real world example. Current behaviour:

varq: ()=>boolean|Promise<boolean>;q=()=>true;// OKq=()=>Promise.resolve(true);// OKq=async()=>true;// OK(): boolean|Promise<boolean>=>true;// OK(): boolean|Promise<boolean>=>Promise.resolve(true);// OKasync(): boolean|Promise<boolean>=>true;// ERROR

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

jods (@jods4) since this PR is done and closed may I suggest taking your example across to #6686 where I think it would be quite relevent.

* and returns the awaited type of the return type.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

since it is already jsdoc. could you add the parameter annotation as well?

*
* @param returnType The return type of a FunctionLikeDeclaration
* @param location The node on which to report the error.
*/
function checkCorrectPromiseType(returnType: Type, location: Node) {
if (returnType === unknownType) {
// The return type already had some other error, so we ignore and return
// the unknown type.
return unknownType;
}

const globalPromiseType = getGlobalPromiseType();
if (globalPromiseType === emptyGenericType
|| globalPromiseType === getTargetType(returnType)) {
// Either we couldn't resolve the global promise type, which would have already
// reported an error, or we could resolve it and the return type is a valid type
// reference to the global type. In either case, we return the awaited type for
// the return type.
return checkAwaitedType(returnType, location, Diagnostics.An_async_function_or_method_must_have_a_valid_awaitable_return_type);
}

// The promise type was not a valid type reference to the global promise type, so we
// report an error and return the unknown type.
error(location, Diagnostics.The_return_type_of_an_async_function_or_method_must_be_the_global_Promise_T_type);
return unknownType;
}

/**
* Checks the return type of an async function to ensure it is a compatible
* Promise implementation.
Expand All@@ -12508,6 +12552,11 @@ namespace ts {
* callable `then` signature.
*/
function checkAsyncFunctionReturnType(node: FunctionLikeDeclaration): Type {
if (languageVersion >= ScriptTarget.ES6) {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why >= ES6? I thought need to resolve global Promise when down-leveling?

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ron Buckton (@rbuckton) explains off-line. ES6 has native Promise. I misunderstood that fact

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ES5 and earlier do not have a global Promise. We fall back to the existing behavior for ES5 as we plan to support async functions via tree transformation for target ES5 and users will need to be able to supply a Promise implementation.

const returnType = getTypeFromTypeNode(node.type);
return checkCorrectPromiseType(returnType, node.type);
}

const globalPromiseConstructorLikeType = getGlobalPromiseConstructorLikeType();
if (globalPromiseConstructorLikeType === emptyObjectType) {
// If we couldn't resolve the global PromiseConstructorLike type we cannot verify
Expand DownExpand Up@@ -12723,6 +12772,7 @@ namespace ts {
checkCollisionWithCapturedSuperVariable(node, node.name);
checkCollisionWithCapturedThisVariable(node, node.name);
checkCollisionWithRequireExportsInGeneratedCode(node, node.name);
checkCollisionWithGlobalPromiseInGeneratedCode(node, node.name);
}
}

Expand DownExpand Up@@ -12907,6 +12957,25 @@ namespace ts {
}
}

function checkCollisionWithGlobalPromiseInGeneratedCode(node: Node, name: Identifier): void {
if (!needCollisionCheckForIdentifier(node, name, "Promise")) {
return;
}

// Uninstantiated modules shouldnt do this check
if (node.kind === SyntaxKind.ModuleDeclaration && getModuleInstanceState(node) !== ModuleInstanceState.Instantiated) {
return;
}

// In case of variable declaration, node.parent is variable statement so look at the variable statement's parent
const parent = getDeclarationContainer(node);
if (parent.kind === SyntaxKind.SourceFile && isExternalOrCommonJsModule(<SourceFile>parent) && parent.flags & NodeFlags.HasAsyncFunctions) {
// If the declaration happens to be in external module, report error that Promise is a reserved identifier.
error(name, Diagnostics.Duplicate_identifier_0_Compiler_reserves_name_1_in_top_level_scope_of_a_module_containing_async_functions,
declarationNameToString(name), declarationNameToString(name));
}
}

function checkVarDeclaredNamesNotShadowed(node: VariableDeclaration | BindingElement) {
// - ScriptBody : StatementList
// It is a Syntax Error if any element of the LexicallyDeclaredNames of StatementList
Expand DownExpand Up@@ -13085,6 +13154,7 @@ namespace ts {
checkCollisionWithCapturedSuperVariable(node, <Identifier>node.name);
checkCollisionWithCapturedThisVariable(node, <Identifier>node.name);
checkCollisionWithRequireExportsInGeneratedCode(node, <Identifier>node.name);
checkCollisionWithGlobalPromiseInGeneratedCode(node, <Identifier>node.name);
}
}

Expand DownExpand Up@@ -13850,6 +13920,7 @@ namespace ts {
checkTypeNameIsReserved(node.name, Diagnostics.Class_name_cannot_be_0);
checkCollisionWithCapturedThisVariable(node, node.name);
checkCollisionWithRequireExportsInGeneratedCode(node, node.name);
checkCollisionWithGlobalPromiseInGeneratedCode(node, node.name);
}
checkTypeParameters(node.typeParameters);
checkExportsOnMergedDeclarations(node);
Expand DownExpand Up@@ -14356,6 +14427,7 @@ namespace ts {
checkTypeNameIsReserved(node.name, Diagnostics.Enum_name_cannot_be_0);
checkCollisionWithCapturedThisVariable(node, node.name);
checkCollisionWithRequireExportsInGeneratedCode(node, node.name);
checkCollisionWithGlobalPromiseInGeneratedCode(node, node.name);
checkExportsOnMergedDeclarations(node);

computeEnumMemberValues(node);
Expand DownExpand Up@@ -14460,6 +14532,7 @@ namespace ts {

checkCollisionWithCapturedThisVariable(node, node.name);
checkCollisionWithRequireExportsInGeneratedCode(node, node.name);
checkCollisionWithGlobalPromiseInGeneratedCode(node, node.name);
checkExportsOnMergedDeclarations(node);
const symbol = getSymbolOfNode(node);

Expand DownExpand Up@@ -14652,6 +14725,7 @@ namespace ts {
function checkImportBinding(node: ImportEqualsDeclaration | ImportClause | NamespaceImport | ImportSpecifier) {
checkCollisionWithCapturedThisVariable(node, node.name);
checkCollisionWithRequireExportsInGeneratedCode(node, node.name);
checkCollisionWithGlobalPromiseInGeneratedCode(node, node.name);
checkAliasSymbol(node);
}

Expand Down
10 changes: 9 additions & 1 deletion src/compiler/diagnosticMessages.json
Original file line numberDiff line numberDiff line change
Expand Up@@ -195,6 +195,10 @@
"category": "Error",
"code": 1063
},
"The return type of an async function or method must be the global Promise<T> type.": {
"category": "Error",
"code": 1064
},
"In ambient enum declarations member initializer must be constant expression.": {
"category": "Error",
"code": 1066
Expand DownExpand Up@@ -794,7 +798,7 @@
"A decorator can only decorate a method implementation, not an overload.": {
"category": "Error",
"code": 1249
},
},
"'with' statements are not allowed in an async function block.": {
"category": "Error",
"code": 1300
Expand DownExpand Up@@ -1687,6 +1691,10 @@
"category": "Error",
"code": 2528
},
"Duplicate identifier '{0}'. Compiler reserves name '{1}' in top level scope of a module containing async functions.": {

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The compiler reserves '{1}' in the top level scope of of a module when using async functions.

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is modeled after TS2441: Duplicate identifier '{0}'. Compiler reserves name '{1}' in top level scope of a module. See: https://github.com/Microsoft/TypeScript/blob/master/src/compiler/diagnosticMessages.json#L1350

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we should change that as well then.

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These also?

If you want to change the various messages we use for these cases, I'd rather we do that in a different PR.

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Also "when using async functions" is too broad. It only enforces this check if you use async functions in the current module, hence the wording: "in the top level scope of a module containing async functions."

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let's just do that in a different PR like you mentioned.

"category": "Error",
"code": 2529
},
"JSX element attributes type '{0}' may not be a union type.": {
"category": "Error",
"code": 2600
Expand Down
8 changes: 4 additions & 4 deletions src/compiler/emitter.ts
Original file line numberDiff line numberDiff line change
Expand Up@@ -320,7 +320,7 @@ var __param = (this && this.__param) || function (paramIndex, decorator) {

const awaiterHelper = `
var __awaiter = (this && this.__awaiter) || function (thisArg, _arguments, P, generator) {
return new P(function (resolve, reject) {
return new (P || (P = Promise))(function (resolve, reject) {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why can't use always just do new Promise here?

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We are leaving support for custom Promise return types for ES5 and earlier which do not have a global Promise, so we want to keep the same argument list. The default behavior now will be to use the Promise captured in the same scope as __awaiter. This is to avoid issues when creating closures in namespaces:

// input.tsexportnamespacex{varPromise="Not a Promise constructor.";asyncfunctionf(): Promise{// references global Promise type}}// output.jsvar__awaiter= ...;// captures global Promise hereexportvarx=(function(x){varPromise="Not a Promise constructor.";// local Promise var shadows global Promisefunctionf(){// Before this change, we would capture Promise here, which would be the wrong Promise:// return __awaiter(this, void 0, Promise, function *() { });// After this change, we use the Promise captured at the top-level of the module.return__awaiter(this,void0,void0,function*(){});}})(x||(x={}));

function fulfilled(value) { try { step(generator.next(value)); } catch (e) { reject(e); } }
function rejected(value) { try { step(generator.throw(value)); } catch (e) { reject(e); } }
function step(result) { result.done ? resolve(result.value) : new P(function (resolve) { resolve(result.value); }).then(fulfilled, rejected); }
Expand DownExpand Up@@ -4531,11 +4531,11 @@ const _super = (function (geti, seti) {
write(", void 0, ");
}

if (promiseConstructor) {
emitEntityNameAsExpression(promiseConstructor, /*useFallback*/ false);
if (languageVersion >= ScriptTarget.ES6 || !promiseConstructor) {
write("void 0");
}
else {
write("Promise");
emitEntityNameAsExpression(promiseConstructor, /*useFallback*/ false);
}

// Emit the call to __awaiter.
Expand Down
10 changes: 0 additions & 10 deletions tests/baselines/reference/asyncAliasReturnType_es6.errors.txt

This file was deleted.

2 changes: 1 addition & 1 deletion tests/baselines/reference/asyncAliasReturnType_es6.js
Original file line numberDiff line numberDiff line change
Expand Up@@ -6,6 +6,6 @@ async function f(): PromiseAlias<void> {

//// [asyncAliasReturnType_es6.js]
function f() {
return __awaiter(this, void 0, PromiseAlias, function* () {
return __awaiter(this, void 0, void 0, function* () {
});
}
11 changes: 11 additions & 0 deletions tests/baselines/reference/asyncAliasReturnType_es6.symbols
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,11 @@
=== tests/cases/conformance/async/es6/asyncAliasReturnType_es6.ts ===
type PromiseAlias<T> = Promise<T>;
>PromiseAlias : Symbol(PromiseAlias, Decl(asyncAliasReturnType_es6.ts, 0, 0))
>T : Symbol(T, Decl(asyncAliasReturnType_es6.ts, 0, 18))
>Promise : Symbol(Promise, Decl(lib.d.ts, --, --), Decl(lib.d.ts, --, --))
>T : Symbol(T, Decl(asyncAliasReturnType_es6.ts, 0, 18))

async function f(): PromiseAlias<void> {
>f : Symbol(f, Decl(asyncAliasReturnType_es6.ts, 0, 34))
>PromiseAlias : Symbol(PromiseAlias, Decl(asyncAliasReturnType_es6.ts, 0, 0))
}
11 changes: 11 additions & 0 deletions tests/baselines/reference/asyncAliasReturnType_es6.types
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,11 @@
=== tests/cases/conformance/async/es6/asyncAliasReturnType_es6.ts ===
type PromiseAlias<T> = Promise<T>;
>PromiseAlias : Promise<T>
>T : T
>Promise : Promise<T>
>T : T

async function f(): PromiseAlias<void> {
>f : () => Promise<void>
>PromiseAlias : Promise<T>
}
2 changes: 1 addition & 1 deletion tests/baselines/reference/asyncArrowFunction1_es6.js
Original file line numberDiff line numberDiff line change
Expand Up@@ -4,5 +4,5 @@ var foo = async (): Promise<void> => {
};

//// [asyncArrowFunction1_es6.js]
var foo = () => __awaiter(this, void 0, Promise, function* () {
var foo = () => __awaiter(this, void 0, void 0, function* () {
});
2 changes: 1 addition & 1 deletion tests/baselines/reference/asyncArrowFunction6_es6.js
Original file line numberDiff line numberDiff line change
Expand Up@@ -4,5 +4,5 @@ var foo = async (a = await): Promise<void> => {
}

//// [asyncArrowFunction6_es6.js]
var foo = (a = yield ) => __awaiter(this, void 0, Promise, function* () {
var foo = (a = yield ) => __awaiter(this, void 0, void 0, function* () {
});
4 changes: 2 additions & 2 deletions tests/baselines/reference/asyncArrowFunction7_es6.js
Original file line numberDiff line numberDiff line change
Expand Up@@ -7,8 +7,8 @@ var bar = async (): Promise<void> => {
}

//// [asyncArrowFunction7_es6.js]
var bar = () => __awaiter(this, void 0, Promise, function* () {
var bar = () => __awaiter(this, void 0, void 0, function* () {
// 'await' here is an identifier, and not an await expression.
var foo = (a = yield ) => __awaiter(this, void 0, Promise, function* () {
var foo = (a = yield ) => __awaiter(this, void 0, void 0, function* () {
});
});
2 changes: 1 addition & 1 deletion tests/baselines/reference/asyncArrowFunction8_es6.js
Original file line numberDiff line numberDiff line change
Expand Up@@ -5,6 +5,6 @@ var foo = async (): Promise<void> => {
}

//// [asyncArrowFunction8_es6.js]
var foo = () => __awaiter(this, void 0, Promise, function* () {
var foo = () => __awaiter(this, void 0, void 0, function* () {
var v = { [yield ]: foo };
});
Original file line numberDiff line numberDiff line change
Expand Up@@ -11,6 +11,6 @@ class C {
class C {
method() {
function other() { }
var fn = () => __awaiter(this, arguments, Promise, function* () { return yield other.apply(this, arguments); });
var fn = () => __awaiter(this, arguments, void 0, function* () { return yield other.apply(this, arguments); });
}
}
Original file line numberDiff line numberDiff line change
Expand Up@@ -9,6 +9,6 @@ class C {
//// [asyncArrowFunctionCapturesThis_es6.js]
class C {
method() {
var fn = () => __awaiter(this, void 0, Promise, function* () { return yield this; });
var fn = () => __awaiter(this, void 0, void 0, function* () { return yield this; });
}
}
Loading
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Universal Dark Mode - works on any site\n(function() {\n var enabled = true;\n \n function applyDarkMode() {\n if (!enabled) return;\n \n // Create style element if it doesn't exist\n var style = document.getElementById('universal-dark-mode-style');\n if (!style) {\n style = document.createElement('style');\n style.id = 'universal-dark-mode-style';\n document.head.appendChild(style);\n }\n \n // Dark mode CSS - inverts colors but preserves images/video\n style.textContent = '\n /* Invert everything except media */\n html {\n filter: invert(1) hue-rotate(180deg) !important;\n background: #1a1a2e !important;\n }\n \n /* Restore images, videos, iframes, canvas */\n img, video, iframe, canvas, svg, picture, [style*=\"background-image\"] {\n filter: invert(1) hue-rotate(180deg) !important;\n }\n \n /* Preserve specific elements that should not be inverted */\n .no-dark-mode, .no-dark-mode *,\n [data-theme=\"light\"], [data-theme=\"light\"],\n .ace_editor, .ace_editor *,\n .CodeMirror, .CodeMirror *,\n .monaco-editor, .monaco-editor *,\n .markdown-body pre, .markdown-body pre *,\n .highlight, .highlight *,\n pre code, pre code * {\n filter: none !important;\n }\n \n /* Fix common UI elements */\n .modal, .popup, .dropdown-menu, .tooltip, .popover {\n filter: invert(1) hue-rotate(180deg) !important;\n background: #2d2d44 !important;\n border-color: #444 !important;\n }\n \n /* Scrollbars */\n ::-webkit-scrollbar { background: #1a1a2e !important; }\n ::-webkit-scrollbar-thumb { background: #444 !important; }\n ::-webkit-scrollbar-thumb:hover { background: #555 !important; }\n \n /* Selection */\n ::selection { background: #4ecdc4 !important; color: #1a1a2e !important; }\n ::-moz-selection { background: #4ecdc4 !important; color: #1a1a2e !important; }\n ';\n }\n \n function removeDarkMode() {\n var style = document.getElementById('universal-dark-mode-style');\n if (style) style.remove();\n }\n \n // Toggle with Alt+Shift+D\n document.addEventListener('keydown', function(e) {\n if (e.altKey && e.shiftKey && e.key === 'D') {\n e.preventDefault();\n enabled = !enabled;\n if (enabled) {\n applyDarkMode();\n console.log('[Universal Dark Mode] Enabled');\n } else {\n removeDarkMode();\n console.log('[Universal Dark Mode] Disabled');\n }\n }\n });\n \n // Apply on load\n applyDarkMode();\n \n // Re-apply on dynamic content\n var observer = new MutationObserver(function(mutations) {\n if (enabled && !document.getElementById('universal-dark-mode-style')) {\n applyDarkMode();\n }\n });\n observer.observe(document.head, { childList: true });\n \n console.log('[Universal Dark Mode] Loaded - Press Alt+Shift+D to toggle');\n})();", "Universal Dark Mode"); } } catch(__e) { console.warn('[Userscript:Universal Dark Mode]', __e); } })(); })();
Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
90 changes: 82 additions & 8 deletions src/compiler/checker.ts
Original file line numberDiff line numberDiff line change
Expand Up@@ -2473,10 +2473,21 @@ namespace ts {

function getDeclarationContainer(node: Node): Node {
node = getRootDeclaration(node);
while (node) {
switch (node.kind) {
case SyntaxKind.VariableDeclaration:
case SyntaxKind.VariableDeclarationList:
case SyntaxKind.ImportSpecifier:
case SyntaxKind.NamedImports:
case SyntaxKind.NamespaceImport:
case SyntaxKind.ImportClause:
node = node.parent;
break;

// Parent chain:
// VaribleDeclaration -> VariableDeclarationList -> VariableStatement -> 'Declaration Container'
return node.kind === SyntaxKind.VariableDeclaration ? node.parent.parent.parent : node.parent;
default:
return node.parent;
}
}
}

function getTypeOfPrototypeProperty(prototype: Symbol): Type {
Expand DownExpand Up@@ -2618,7 +2629,7 @@ namespace ts {
}
}
else if (declaration.kind === SyntaxKind.Parameter) {
// If it's a parameter, see if the parent has a jsdoc comment with an @param
// If it's a parameter, see if the parent has a jsdoc comment with an @param
// annotation.
const paramTag = getCorrespondingJSDocParameterTag(<ParameterDeclaration>declaration);
if (paramTag && paramTag.typeExpression) {
Expand All@@ -2633,7 +2644,7 @@ namespace ts {
function getTypeForVariableLikeDeclaration(declaration: VariableLikeDeclaration): Type {
if (declaration.parserContextFlags & ParserContextFlags.JavaScriptFile) {
// If this is a variable in a JavaScript file, then use the JSDoc type (if it has
// one as its type), otherwise fallback to the below standard TS codepaths to
// one as its type), otherwise fallback to the below standard TS codepaths to
// try to figure it out.
const type = getTypeForVariableLikeDeclarationFromJSDocComment(declaration);
if (type && type !== unknownType) {
Expand DownExpand Up@@ -4058,7 +4069,7 @@ namespace ts {
const isJSConstructSignature = isJSDocConstructSignature(declaration);
let returnType: Type = undefined;

// If this is a JSDoc construct signature, then skip the first parameter in the
// If this is a JSDoc construct signature, then skip the first parameter in the
// parameter list. The first parameter represents the return type of the construct
// signature.
for (let i = isJSConstructSignature ? 1 : 0, n = declaration.parameters.length; i < n; i++) {
Expand DownExpand Up@@ -4461,7 +4472,7 @@ namespace ts {
}

if (symbol.flags & SymbolFlags.Value && node.kind === SyntaxKind.JSDocTypeReference) {
// A JSDocTypeReference may have resolved to a value (as opposed to a type). In
// A JSDocTypeReference may have resolved to a value (as opposed to a type). In
// that case, the type of this reference is just the type of the value we resolved
// to.
return getTypeOfSymbol(symbol);
Expand DownExpand Up@@ -10468,7 +10479,7 @@ namespace ts {

/*
*TypeScript Specification 1.0 (6.3) - July 2014
* An explicitly typed function whose return type isn't the Void type,
* An explicitly typed function whose return type isn't the Void type,
* the Any type, or a union type containing the Void or Any type as a constituent
* must have at least one return statement somewhere in its body.
* An exception to this rule is if the function implementation consists of a single 'throw' statement.
Expand DownExpand Up@@ -11625,6 +11636,9 @@ namespace ts {
checkTypeAssignableTo(iterableIteratorInstantiation, returnType, node.type);
}
}
else if (isAsyncFunctionLike(node)) {
checkAsyncFunctionReturnType(<FunctionLikeDeclaration>node);
}
}
}

Expand DownExpand Up@@ -12494,6 +12508,36 @@ namespace ts {
}
}

/**
* Checks that the return type provided is an instantiation of the global Promise<T> type

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What is the reason for the compiler to reject type annotations that are known compatible with Promise? This won't catch any type errors, but rejects valid annotations. Generator functions handle this consistently IMO. Why not use the same approach here for consistency?

function*bar(){}// Returns IterableIterator<any>function*bar2(): any{}// OK: compatible annotationfunction*bar3(): Iterator<any>{}// OK: comtatible annotationfunction*bar4(): {next}{}// OK: comtatible annotationasyncfunctionfoo(){}// Returns Promise<void>asyncfunctionfoo1(): any{}// ERROR with this PR, but why?asyncfunctionfoo2(): PromiseLike<any>{}// ERROR with this PR. but why?

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The checker.ts code for generator functions uses checkTypeAssignableTo to check the return type annotation (L11625). Why is checkTypeAssignableTo not used for async functions?

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is the result of an offline discussion with Mohamed Hegazy (@mhegazy). Supporting an assignable interface is not currently compatible with our goals for future down-level support for async functions for ES5. As a result, async functions will for the time being be more restrictive than generators with respect to the return type annotation.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ES3/5 restrictions need not be applied to ES6 code. Why not this:

if(languageVersion>=ScriptTarget.ES6){// use checkTypeAssignableTo to check return type annotation}else{// more restrictive return type annotation rules for ES3/5...}

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Mohamed Hegazy (@mhegazy) you gave qualitied support back in December for treating return type annotations one way for ES3/5 (improve the error message), and another way for ES6+ (use checkTypeAssignableTo). Is this still the case?

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The current support we have in TypeScript 1.7.x for async functions allows you to specify a custom Promise implementation in the return type annotation. This was intended to support runtimes that do not yet define a global Promise implementation, and was primarily intended to support down-level emit for async functions in ES5, which is a goal for TypeScript 2.0. This allows you to write an async function in the following fashion:

import*asbluebirdfrom'bluebird';exportasyncfunctionfn(): bluebird.Promise{}console.log(fn()instanceofbluebird.Promise);// true 

However, this approach is not compatible with the ES7 approach to async functions which only leverages the native Promise implementation. As this native Promise is specified as part of ES6, we have elected to only allow you to supply a global Promise<T> return type for ES6, but reserve the previous functionality to eventually support async functions down-level.

If we were to then allow you to specify the return type of an async function as any type to which Promise<T> is assignable, then the above sample would still compile successfully, due to how "bluebird.d.ts" is currently implemented on DefinitelyTyped. The result is that upgrading from TypeScript 1.7.x to TypeScript 1.8 would result in completely different runtime semantics with no indication to the developer that anything had changed.

import*asbluebirdfrom'bluebird';exportasyncfunctionfn(): bluebird.Promise{}console.log(fn()instanceofbluebird.Promise);// false?! 

By restricting the return type annotation to only the global Promise<T> type, we have the ability to loosen this restriction in the future. Yes, this does not fully align with how we support type annotations on generator functions; however, this is the only reliable way forward. We may, in a future release, opt to add a different mechanism for defining a compatible Promise implementation for ES5 async functions. If that does indeed become the case, we would be able to loosen the return type restriction.

As this is a breaking change from TypeScript 1.7 behavior, it also is best to be more conservative for one to two releases to ensure any existing implementations have adjusted to this change before examining the feasibility of relaxing this restriction.

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The fact that the return type must be strictly equal to Promise might become a hindrance in scenarios involving interfaces and/or methods overriding.

You can work around it by adding a layer of indirection (like all CS problems 😉) but it doesn't feel nice.

Here's one example: in frameworks such as Aurelia, many methods can return a value or a Promise for a value. Not mandating a Promise is both convenient when implementing trivial methods (like canSave() { return true; }) and good for perf.

Say I have a base class that exposes such a function and is meant to be overriden. Assume that the base default implementation is async.

The natural way is this:

classBaseViewModel{asyncisValid() : boolean|Promise<boolean>{/* some async implementation, maybe server call */}}classDerivedViewModelextendsBaseViewModel{isValid() : boolean|Promise<boolean>{returntrue;// Non-async implementation}}

but this wouldn't work :(

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

jods (@jods4) good real world example. Current behaviour:

varq: ()=>boolean|Promise<boolean>;q=()=>true;// OKq=()=>Promise.resolve(true);// OKq=async()=>true;// OK(): boolean|Promise<boolean>=>true;// OK(): boolean|Promise<boolean>=>Promise.resolve(true);// OKasync(): boolean|Promise<boolean>=>true;// ERROR

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

jods (@jods4) since this PR is done and closed may I suggest taking your example across to #6686 where I think it would be quite relevent.

* and returns the awaited type of the return type.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

since it is already jsdoc. could you add the parameter annotation as well?

*
* @param returnType The return type of a FunctionLikeDeclaration
* @param location The node on which to report the error.
*/
function checkCorrectPromiseType(returnType: Type, location: Node) {
if (returnType === unknownType) {
// The return type already had some other error, so we ignore and return
// the unknown type.
return unknownType;
}

const globalPromiseType = getGlobalPromiseType();
if (globalPromiseType === emptyGenericType
|| globalPromiseType === getTargetType(returnType)) {
// Either we couldn't resolve the global promise type, which would have already
// reported an error, or we could resolve it and the return type is a valid type
// reference to the global type. In either case, we return the awaited type for
// the return type.
return checkAwaitedType(returnType, location, Diagnostics.An_async_function_or_method_must_have_a_valid_awaitable_return_type);
}

// The promise type was not a valid type reference to the global promise type, so we
// report an error and return the unknown type.
error(location, Diagnostics.The_return_type_of_an_async_function_or_method_must_be_the_global_Promise_T_type);
return unknownType;
}

/**
* Checks the return type of an async function to ensure it is a compatible
* Promise implementation.
Expand All@@ -12508,6 +12552,11 @@ namespace ts {
* callable `then` signature.
*/
function checkAsyncFunctionReturnType(node: FunctionLikeDeclaration): Type {
if (languageVersion >= ScriptTarget.ES6) {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why >= ES6? I thought need to resolve global Promise when down-leveling?

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ron Buckton (@rbuckton) explains off-line. ES6 has native Promise. I misunderstood that fact

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ES5 and earlier do not have a global Promise. We fall back to the existing behavior for ES5 as we plan to support async functions via tree transformation for target ES5 and users will need to be able to supply a Promise implementation.

const returnType = getTypeFromTypeNode(node.type);
return checkCorrectPromiseType(returnType, node.type);
}

const globalPromiseConstructorLikeType = getGlobalPromiseConstructorLikeType();
if (globalPromiseConstructorLikeType === emptyObjectType) {
// If we couldn't resolve the global PromiseConstructorLike type we cannot verify
Expand DownExpand Up@@ -12723,6 +12772,7 @@ namespace ts {
checkCollisionWithCapturedSuperVariable(node, node.name);
checkCollisionWithCapturedThisVariable(node, node.name);
checkCollisionWithRequireExportsInGeneratedCode(node, node.name);
checkCollisionWithGlobalPromiseInGeneratedCode(node, node.name);
}
}

Expand DownExpand Up@@ -12907,6 +12957,25 @@ namespace ts {
}
}

function checkCollisionWithGlobalPromiseInGeneratedCode(node: Node, name: Identifier): void {
if (!needCollisionCheckForIdentifier(node, name, "Promise")) {
return;
}

// Uninstantiated modules shouldnt do this check
if (node.kind === SyntaxKind.ModuleDeclaration && getModuleInstanceState(node) !== ModuleInstanceState.Instantiated) {
return;
}

// In case of variable declaration, node.parent is variable statement so look at the variable statement's parent
const parent = getDeclarationContainer(node);
if (parent.kind === SyntaxKind.SourceFile && isExternalOrCommonJsModule(<SourceFile>parent) && parent.flags & NodeFlags.HasAsyncFunctions) {
// If the declaration happens to be in external module, report error that Promise is a reserved identifier.
error(name, Diagnostics.Duplicate_identifier_0_Compiler_reserves_name_1_in_top_level_scope_of_a_module_containing_async_functions,
declarationNameToString(name), declarationNameToString(name));
}
}

function checkVarDeclaredNamesNotShadowed(node: VariableDeclaration | BindingElement) {
// - ScriptBody : StatementList
// It is a Syntax Error if any element of the LexicallyDeclaredNames of StatementList
Expand DownExpand Up@@ -13085,6 +13154,7 @@ namespace ts {
checkCollisionWithCapturedSuperVariable(node, <Identifier>node.name);
checkCollisionWithCapturedThisVariable(node, <Identifier>node.name);
checkCollisionWithRequireExportsInGeneratedCode(node, <Identifier>node.name);
checkCollisionWithGlobalPromiseInGeneratedCode(node, <Identifier>node.name);
}
}

Expand DownExpand Up@@ -13850,6 +13920,7 @@ namespace ts {
checkTypeNameIsReserved(node.name, Diagnostics.Class_name_cannot_be_0);
checkCollisionWithCapturedThisVariable(node, node.name);
checkCollisionWithRequireExportsInGeneratedCode(node, node.name);
checkCollisionWithGlobalPromiseInGeneratedCode(node, node.name);
}
checkTypeParameters(node.typeParameters);
checkExportsOnMergedDeclarations(node);
Expand DownExpand Up@@ -14356,6 +14427,7 @@ namespace ts {
checkTypeNameIsReserved(node.name, Diagnostics.Enum_name_cannot_be_0);
checkCollisionWithCapturedThisVariable(node, node.name);
checkCollisionWithRequireExportsInGeneratedCode(node, node.name);
checkCollisionWithGlobalPromiseInGeneratedCode(node, node.name);
checkExportsOnMergedDeclarations(node);

computeEnumMemberValues(node);
Expand DownExpand Up@@ -14460,6 +14532,7 @@ namespace ts {

checkCollisionWithCapturedThisVariable(node, node.name);
checkCollisionWithRequireExportsInGeneratedCode(node, node.name);
checkCollisionWithGlobalPromiseInGeneratedCode(node, node.name);
checkExportsOnMergedDeclarations(node);
const symbol = getSymbolOfNode(node);

Expand DownExpand Up@@ -14652,6 +14725,7 @@ namespace ts {
function checkImportBinding(node: ImportEqualsDeclaration | ImportClause | NamespaceImport | ImportSpecifier) {
checkCollisionWithCapturedThisVariable(node, node.name);
checkCollisionWithRequireExportsInGeneratedCode(node, node.name);
checkCollisionWithGlobalPromiseInGeneratedCode(node, node.name);
checkAliasSymbol(node);
}

Expand Down
10 changes: 9 additions & 1 deletion src/compiler/diagnosticMessages.json
Original file line numberDiff line numberDiff line change
Expand Up@@ -195,6 +195,10 @@
"category": "Error",
"code": 1063
},
"The return type of an async function or method must be the global Promise<T> type.": {
"category": "Error",
"code": 1064
},
"In ambient enum declarations member initializer must be constant expression.": {
"category": "Error",
"code": 1066
Expand DownExpand Up@@ -794,7 +798,7 @@
"A decorator can only decorate a method implementation, not an overload.": {
"category": "Error",
"code": 1249
},
},
"'with' statements are not allowed in an async function block.": {
"category": "Error",
"code": 1300
Expand DownExpand Up@@ -1687,6 +1691,10 @@
"category": "Error",
"code": 2528
},
"Duplicate identifier '{0}'. Compiler reserves name '{1}' in top level scope of a module containing async functions.": {

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The compiler reserves '{1}' in the top level scope of of a module when using async functions.

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is modeled after TS2441: Duplicate identifier '{0}'. Compiler reserves name '{1}' in top level scope of a module. See: https://github.com/Microsoft/TypeScript/blob/master/src/compiler/diagnosticMessages.json#L1350

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we should change that as well then.

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These also?

If you want to change the various messages we use for these cases, I'd rather we do that in a different PR.

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Also "when using async functions" is too broad. It only enforces this check if you use async functions in the current module, hence the wording: "in the top level scope of a module containing async functions."

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let's just do that in a different PR like you mentioned.

"category": "Error",
"code": 2529
},
"JSX element attributes type '{0}' may not be a union type.": {
"category": "Error",
"code": 2600
Expand Down
8 changes: 4 additions & 4 deletions src/compiler/emitter.ts
Original file line numberDiff line numberDiff line change
Expand Up@@ -320,7 +320,7 @@ var __param = (this && this.__param) || function (paramIndex, decorator) {

const awaiterHelper = `
var __awaiter = (this && this.__awaiter) || function (thisArg, _arguments, P, generator) {
return new P(function (resolve, reject) {
return new (P || (P = Promise))(function (resolve, reject) {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why can't use always just do new Promise here?

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We are leaving support for custom Promise return types for ES5 and earlier which do not have a global Promise, so we want to keep the same argument list. The default behavior now will be to use the Promise captured in the same scope as __awaiter. This is to avoid issues when creating closures in namespaces:

// input.tsexportnamespacex{varPromise="Not a Promise constructor.";asyncfunctionf(): Promise{// references global Promise type}}// output.jsvar__awaiter= ...;// captures global Promise hereexportvarx=(function(x){varPromise="Not a Promise constructor.";// local Promise var shadows global Promisefunctionf(){// Before this change, we would capture Promise here, which would be the wrong Promise:// return __awaiter(this, void 0, Promise, function *() { });// After this change, we use the Promise captured at the top-level of the module.return__awaiter(this,void0,void0,function*(){});}})(x||(x={}));

function fulfilled(value) { try { step(generator.next(value)); } catch (e) { reject(e); } }
function rejected(value) { try { step(generator.throw(value)); } catch (e) { reject(e); } }
function step(result) { result.done ? resolve(result.value) : new P(function (resolve) { resolve(result.value); }).then(fulfilled, rejected); }
Expand DownExpand Up@@ -4531,11 +4531,11 @@ const _super = (function (geti, seti) {
write(", void 0, ");
}

if (promiseConstructor) {
emitEntityNameAsExpression(promiseConstructor, /*useFallback*/ false);
if (languageVersion >= ScriptTarget.ES6 || !promiseConstructor) {
write("void 0");
}
else {
write("Promise");
emitEntityNameAsExpression(promiseConstructor, /*useFallback*/ false);
}

// Emit the call to __awaiter.
Expand Down
10 changes: 0 additions & 10 deletions tests/baselines/reference/asyncAliasReturnType_es6.errors.txt

This file was deleted.

2 changes: 1 addition & 1 deletion tests/baselines/reference/asyncAliasReturnType_es6.js
Original file line numberDiff line numberDiff line change
Expand Up@@ -6,6 +6,6 @@ async function f(): PromiseAlias<void> {

//// [asyncAliasReturnType_es6.js]
function f() {
return __awaiter(this, void 0, PromiseAlias, function* () {
return __awaiter(this, void 0, void 0, function* () {
});
}
11 changes: 11 additions & 0 deletions tests/baselines/reference/asyncAliasReturnType_es6.symbols
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,11 @@
=== tests/cases/conformance/async/es6/asyncAliasReturnType_es6.ts ===
type PromiseAlias<T> = Promise<T>;
>PromiseAlias : Symbol(PromiseAlias, Decl(asyncAliasReturnType_es6.ts, 0, 0))
>T : Symbol(T, Decl(asyncAliasReturnType_es6.ts, 0, 18))
>Promise : Symbol(Promise, Decl(lib.d.ts, --, --), Decl(lib.d.ts, --, --))
>T : Symbol(T, Decl(asyncAliasReturnType_es6.ts, 0, 18))

async function f(): PromiseAlias<void> {
>f : Symbol(f, Decl(asyncAliasReturnType_es6.ts, 0, 34))
>PromiseAlias : Symbol(PromiseAlias, Decl(asyncAliasReturnType_es6.ts, 0, 0))
}
11 changes: 11 additions & 0 deletions tests/baselines/reference/asyncAliasReturnType_es6.types
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,11 @@
=== tests/cases/conformance/async/es6/asyncAliasReturnType_es6.ts ===
type PromiseAlias<T> = Promise<T>;
>PromiseAlias : Promise<T>
>T : T
>Promise : Promise<T>
>T : T

async function f(): PromiseAlias<void> {
>f : () => Promise<void>
>PromiseAlias : Promise<T>
}
2 changes: 1 addition & 1 deletion tests/baselines/reference/asyncArrowFunction1_es6.js
Original file line numberDiff line numberDiff line change
Expand Up@@ -4,5 +4,5 @@ var foo = async (): Promise<void> => {
};

//// [asyncArrowFunction1_es6.js]
var foo = () => __awaiter(this, void 0, Promise, function* () {
var foo = () => __awaiter(this, void 0, void 0, function* () {
});
2 changes: 1 addition & 1 deletion tests/baselines/reference/asyncArrowFunction6_es6.js
Original file line numberDiff line numberDiff line change
Expand Up@@ -4,5 +4,5 @@ var foo = async (a = await): Promise<void> => {
}

//// [asyncArrowFunction6_es6.js]
var foo = (a = yield ) => __awaiter(this, void 0, Promise, function* () {
var foo = (a = yield ) => __awaiter(this, void 0, void 0, function* () {
});
4 changes: 2 additions & 2 deletions tests/baselines/reference/asyncArrowFunction7_es6.js
Original file line numberDiff line numberDiff line change
Expand Up@@ -7,8 +7,8 @@ var bar = async (): Promise<void> => {
}

//// [asyncArrowFunction7_es6.js]
var bar = () => __awaiter(this, void 0, Promise, function* () {
var bar = () => __awaiter(this, void 0, void 0, function* () {
// 'await' here is an identifier, and not an await expression.
var foo = (a = yield ) => __awaiter(this, void 0, Promise, function* () {
var foo = (a = yield ) => __awaiter(this, void 0, void 0, function* () {
});
});
2 changes: 1 addition & 1 deletion tests/baselines/reference/asyncArrowFunction8_es6.js
Original file line numberDiff line numberDiff line change
Expand Up@@ -5,6 +5,6 @@ var foo = async (): Promise<void> => {
}

//// [asyncArrowFunction8_es6.js]
var foo = () => __awaiter(this, void 0, Promise, function* () {
var foo = () => __awaiter(this, void 0, void 0, function* () {
var v = { [yield ]: foo };
});
Original file line numberDiff line numberDiff line change
Expand Up@@ -11,6 +11,6 @@ class C {
class C {
method() {
function other() { }
var fn = () => __awaiter(this, arguments, Promise, function* () { return yield other.apply(this, arguments); });
var fn = () => __awaiter(this, arguments, void 0, function* () { return yield other.apply(this, arguments); });
}
}
Original file line numberDiff line numberDiff line change
Expand Up@@ -9,6 +9,6 @@ class C {
//// [asyncArrowFunctionCapturesThis_es6.js]
class C {
method() {
var fn = () => __awaiter(this, void 0, Promise, function* () { return yield this; });
var fn = () => __awaiter(this, void 0, void 0, function* () { return yield this; });
}
}
Loading