Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Support using and await using declarations #54505

Merged
merged 33 commits into from
Jun 22, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
33 commits
Select commit Hold shift + click to select a range
0cb73b3
Add support for 'using' and 'await using' declarations
rbuckton May 24, 2023
0647856
Move NamedEvaluation to its own file
rbuckton May 26, 2023
8f082fc
Remove unnecessary copyStandardPrologue call
rbuckton May 26, 2023
688771f
Reorder transforms
rbuckton May 30, 2023
80d0a84
Merge branch 'main' into using-declaration
rbuckton Jun 1, 2023
a17aead
Used named evaluation transform in class fields, emit cleanup
rbuckton Jun 1, 2023
25a2fd3
Add --coverage for tests, remove unused NamedEvaluation code
rbuckton Jun 2, 2023
40f546b
Remove temporary debug/diagnostic code
rbuckton Jun 2, 2023
e0b9c73
Revert '--coverage' in favor of #54499
rbuckton Jun 2, 2023
08f41bd
Remove unused sameMapDefined function from core
rbuckton Jun 2, 2023
7360338
Revert inconsequential change in import list in debug.ts
rbuckton Jun 2, 2023
587a1a4
Revert changes to track factory source in favor of #51307
rbuckton Jun 2, 2023
3be9cf0
Clean up function names in transformers/module/module.ts
rbuckton Jun 2, 2023
0b77050
Minor jsdoc comment cleanup
rbuckton Jun 2, 2023
bba71b1
Add additional documentation comments to the 'esnext' transform
rbuckton Jun 2, 2023
2d4e506
Further cleanup of NamedEvaluation
rbuckton Jun 5, 2023
ae6cbaf
PR Feedback
rbuckton Jun 6, 2023
afa64ca
Add test verifying we now check excess decls in a for-in/of
rbuckton Jun 6, 2023
5b6adea
Add declaration emit tests
rbuckton Jun 8, 2023
7964b5f
Merge branch 'main' into using-declaration
rbuckton Jun 8, 2023
0435db0
Reduce calls to getCombinedNodeFlags
rbuckton Jun 8, 2023
246a9a3
Only cache the last combined node flags
rbuckton Jun 9, 2023
21b3e03
Cache getCombinedNodeFlags result in checker
rbuckton Jun 9, 2023
4a5ca67
Cache getCombinedModifierFlags result in checker
rbuckton Jun 9, 2023
b763f82
Add local, caching version of isVarConst to checker
rbuckton Jun 9, 2023
08a1434
Merge branch 'main' into using-declaration
rbuckton Jun 9, 2023
2fc9bcf
Merge branch 'main' into using-declaration
rbuckton Jun 12, 2023
1e8f045
Merge branch 'main' into using-declaration
rbuckton Jun 13, 2023
3dc4011
Add tests and report better errors for 'await using'
rbuckton Jun 15, 2023
8ad153a
PR feedback
rbuckton Jun 21, 2023
51870e1
Add fallback in helpers when SuppressedError does not exist
rbuckton Jun 21, 2023
df5e591
Use more iterations in evaluation tests
rbuckton Jun 21, 2023
558a502
Treat underscore-prefixed identifiers as used locals for 'using'/'awa…
rbuckton Jun 21, 2023
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
The table of contents is too big for display.
Diff view
Diff view
  •  
  •  
  •  
2 changes: 2 additions & 0 deletions src/compiler/_namespaces/ts.ts
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,8 @@ export * from "../visitorPublic";
export * from "../sourcemap";
export * from "../transformers/utilities";
export * from "../transformers/destructuring";
export * from "../transformers/classThis";
export * from "../transformers/namedEvaluation";
export * from "../transformers/taggedTemplate";
export * from "../transformers/ts";
export * from "../transformers/classFields";
Expand Down
2 changes: 1 addition & 1 deletion src/compiler/binder.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3725,7 +3725,7 @@ function isExecutableStatement(s: Statement): boolean {
// Don't remove statements that can validly be used before they appear.
return !isFunctionDeclaration(s) && !isPurelyTypeDeclaration(s) && !isEnumDeclaration(s) &&
// `var x;` may declare a variable used above
!(isVariableStatement(s) && !(getCombinedNodeFlags(s) & (NodeFlags.Let | NodeFlags.Const)) && s.declarationList.declarations.some(d => !d.initializer));
!(isVariableStatement(s) && !(getCombinedNodeFlags(s) & (NodeFlags.BlockScoped)) && s.declarationList.declarations.some(d => !d.initializer));
}

function isPurelyTypeDeclaration(s: Statement): boolean {
Expand Down
271 changes: 200 additions & 71 deletions src/compiler/checker.ts

Large diffs are not rendered by default.

1 change: 1 addition & 0 deletions src/compiler/commandLineParser.ts
Original file line number Diff line number Diff line change
Expand Up @@ -218,6 +218,7 @@ const libEntries: [string, string][] = [
["esnext.symbol", "lib.es2019.symbol.d.ts"],
["esnext.asynciterable", "lib.es2018.asynciterable.d.ts"],
["esnext.intl", "lib.esnext.intl.d.ts"],
["esnext.disposable", "lib.esnext.disposable.d.ts"],
["esnext.bigint", "lib.es2020.bigint.d.ts"],
["esnext.string", "lib.es2022.string.d.ts"],
["esnext.promise", "lib.es2021.promise.d.ts"],
Expand Down
56 changes: 48 additions & 8 deletions src/compiler/diagnosticMessages.json
Original file line number Diff line number Diff line change
Expand Up @@ -451,18 +451,14 @@
"category": "Error",
"code": 1149
},
"'const' declarations must be initialized.": {
"'{0}' declarations must be initialized.": {
"category": "Error",
"code": 1155
},
"'const' declarations can only be declared inside a block.": {
"'{0}' declarations can only be declared inside a block.": {
"category": "Error",
"code": 1156
},
"'let' declarations can only be declared inside a block.": {
"category": "Error",
"code": 1157
},
"Unterminated template literal.": {
"category": "Error",
"code": 1160
Expand Down Expand Up @@ -1601,6 +1597,26 @@
"category": "Error",
"code": 1490
},
"'{0}' modifier cannot appear on a 'using' declaration.": {
"category": "Error",
"code": 1491
},
rbuckton marked this conversation as resolved.
Show resolved Hide resolved
"'{0}' declarations may not have binding patterns.": {
"category": "Error",
"code": 1492
},
"The left-hand side of a 'for...in' statement cannot be a 'using' declaration.": {
"category": "Error",
"code": 1493
},
"The left-hand side of a 'for...in' statement cannot be an 'await using' declaration.": {
"category": "Error",
"code": 1494
},
"'{0}' modifier cannot appear on an 'await using' declaration.": {
"category": "Error",
"code": 1495
},

"The types of '{0}' are incompatible between these types.": {
"category": "Error",
Expand Down Expand Up @@ -3623,6 +3639,26 @@
"category": "Error",
"code": 2849
},
"The initializer of a 'using' declaration must be either an object with a '[Symbol.dispose]()' method, or be 'null' or 'undefined'.": {
"category": "Error",
"code": 2850
},
"The initializer of an 'await using' declaration must be either an object with a '[Symbol.asyncDispose]()' or '[Symbol.dispose]()' method, or be 'null' or 'undefined'.": {
"category": "Error",
"code": 2851
},
"'await using' statements are only allowed within async functions and at the top levels of modules.": {
"category": "Error",
"code": 2852
},
rbuckton marked this conversation as resolved.
Show resolved Hide resolved
"'await using' statements are only allowed at the top level of a file when that file is a module, but this file has no imports or exports. Consider adding an empty 'export {}' to make this file a module.": {
"category": "Error",
"code": 2853
},
"Top-level 'await using' statements are only allowed when the 'module' option is set to 'es2022', 'esnext', 'system', 'node16', or 'nodenext', and the 'target' option is set to 'es2017' or higher.": {
"category": "Error",
"code": 2854
},

"Import declaration '{0}' is using private name '{1}'.": {
"category": "Error",
Expand Down Expand Up @@ -7749,11 +7785,11 @@
"category": "Error",
"code": 18036
},
"Await expression cannot be used inside a class static block.": {
"'await' expression cannot be used inside a class static block.": {
"category": "Error",
"code": 18037
},
"'For await' loops cannot be used inside a class static block.": {
"'for await' loops cannot be used inside a class static block.": {
"category": "Error",
"code": 18038
},
Expand Down Expand Up @@ -7812,5 +7848,9 @@
"Its type '{0}' is not a valid JSX element type.": {
"category": "Error",
"code": 18053
},
"'await using' statements cannot be used inside a class static block.": {
"category": "Error",
"code": 18054
}
rbuckton marked this conversation as resolved.
Show resolved Hide resolved
}
15 changes: 14 additions & 1 deletion src/compiler/emitter.ts
Original file line number Diff line number Diff line change
Expand Up @@ -254,8 +254,10 @@ import {
isUnparsedNode,
isUnparsedPrepend,
isUnparsedSource,
isVarAwaitUsing,
isVarConst,
isVariableStatement,
isVarUsing,
JSDoc,
JSDocAugmentsTag,
JSDocCallbackTag,
Expand Down Expand Up @@ -3685,7 +3687,18 @@ export function createPrinter(printerOptions: PrinterOptions = {}, handlers: Pri
}

function emitVariableDeclarationList(node: VariableDeclarationList) {
writeKeyword(isLet(node) ? "let" : isVarConst(node) ? "const" : "var");
if (isVarAwaitUsing(node)) {
writeKeyword("await");
writeSpace();
writeKeyword("using");
}
else {
const head = isLet(node) ? "let" :
isVarConst(node) ? "const" :
isVarUsing(node) ? "using" :
"var";
writeKeyword(head);
}
writeSpace();
emitList(node, node.declarations, ListFormat.VariableDeclarationList);
}
Expand Down
91 changes: 89 additions & 2 deletions src/compiler/factory/emitHelpers.ts
Original file line number Diff line number Diff line change
Expand Up @@ -136,6 +136,9 @@ export interface EmitHelperFactory {
createClassPrivateFieldGetHelper(receiver: Expression, state: Identifier, kind: PrivateIdentifierKind, f: Identifier | undefined): Expression;
createClassPrivateFieldSetHelper(receiver: Expression, state: Identifier, value: Expression, kind: PrivateIdentifierKind, f: Identifier | undefined): Expression;
createClassPrivateFieldInHelper(state: Identifier, receiver: Expression): Expression;
// 'using' helpers
createAddDisposableResourceHelper(envBinding: Expression, value: Expression, async: boolean): Expression;
createDisposeResourcesHelper(envBinding: Expression): Expression;
}

/** @internal */
Expand Down Expand Up @@ -183,7 +186,10 @@ export function createEmitHelperFactory(context: TransformationContext): EmitHel
// Class Fields Helpers
createClassPrivateFieldGetHelper,
createClassPrivateFieldSetHelper,
createClassPrivateFieldInHelper
createClassPrivateFieldInHelper,
// 'using' helpers
createAddDisposableResourceHelper,
createDisposeResourcesHelper,
};

/**
Expand Down Expand Up @@ -666,6 +672,20 @@ export function createEmitHelperFactory(context: TransformationContext): EmitHel
context.requestEmitHelper(classPrivateFieldInHelper);
return factory.createCallExpression(getUnscopedHelperName("__classPrivateFieldIn"), /*typeArguments*/ undefined, [state, receiver]);
}

function createAddDisposableResourceHelper(envBinding: Expression, value: Expression, async: boolean): Expression {
context.requestEmitHelper(addDisposableResourceHelper);
return factory.createCallExpression(
getUnscopedHelperName("__addDisposableResource"),
/*typeArguments*/ undefined,
[envBinding, value, async ? factory.createTrue() : factory.createFalse()]
);
}

function createDisposeResourcesHelper(envBinding: Expression) {
context.requestEmitHelper(disposeResourcesHelper);
return factory.createCallExpression(getUnscopedHelperName("__disposeResources"), /*typeArguments*/ undefined, [envBinding]);
}
}

/** @internal */
Expand Down Expand Up @@ -1367,6 +1387,71 @@ export const classPrivateFieldInHelper: UnscopedEmitHelper = {
};`
};

/**
* @internal
*/
export const addDisposableResourceHelper: UnscopedEmitHelper = {
name: "typescript:addDisposableResource",
importName: "__addDisposableResource",
scoped: false,
text: `
var __addDisposableResource = (this && this.__addDisposableResource) || function (env, value, async) {
if (value !== null && value !== void 0) {
if (typeof value !== "object") throw new TypeError("Object expected.");
var dispose;
if (async) {
if (!Symbol.asyncDispose) throw new TypeError("Symbol.asyncDispose is not defined.");
dispose = value[Symbol.asyncDispose];
}
if (dispose === void 0) {
if (!Symbol.dispose) throw new TypeError("Symbol.dispose is not defined.");
dispose = value[Symbol.dispose];

Choose a reason for hiding this comment

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

In Babel we use Symbol.for("Symbol.dispose") and Symbol.for("Symbol.asyncDispose") as fallbacks, so that users can use polyfills that do not modify the global scope. This is very useful for libraries, so that they can do something

class MyResource {
  [Symbol.dispose || Symbol.for("Symbol.dispose")]() {}
}

and they will be usable both natively and in older environments.

It might be great to align on this.

Copy link
Member Author

Choose a reason for hiding this comment

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

TypeScript helpers generally don't polyfill/shim Symbols in this way. We usually depend on the developer to introduce any necessary global shim instead. I'm curious if @DanielRosenwasser has any thoughts on this approach, however.

Copy link
Member Author

Choose a reason for hiding this comment

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

We discussed this in design meeting and don't believe TypeScript should support Symbol.for() as a fallback. For one, Symbol.for("Symbol.dispose") is only marginally an improvement over just using a string like "@@dispose", and we don't treat the result of Symbol.for() as a meaningfully unique symbol, which is a requirement for us to properly type check it as a property name. As a result, a property name of [Symbol.dispose || Symbol.for("Symbol.dispose")] produces a compilation error, so you wouldn't be able to produce that code without turning off error checking. Even if we did allow Symbol.for() to produce a meaningfully unique symbol that we can type check, we would essentially have to bifurcate Disposable to support each method name, and deal with the complexity of that only working in older compilation targets, since the Symbol.for() fallback wouldn't be supported natively in --target esnext.

Babel supporting for Symbol.for() is actually a bit of a hazard for TypeScript. If someone were to publish a package that leveraged Babel's support for a Symbol.for() fallback, they might in turn indicate that their objects are "disposable" in their documentation. That could lead to someone else putting together types for that package on DefinitelyTyped that indicate a class has a Symbol.dispose method when it does not. This in turn could lead to errors during type check if the package's consumer isn't running with --lib esnext.disposable, which could then result in them changing their --lib to make the error go away without ensuring that their runtime environment actually supports a global Symbol.dispose.

While I think it is commendable to support users' efforts to avoid global scope modifications, I don't think this is an approach TypeScript can take here. I'd even go so far as to recommend that Babel not provide such support, but given that Babel's support for Symbol.for("Symbol.dispose") only has an exteremly remote possibility of causing problems for TypeScript projects, I'll leave that up to you.

I'll also note that, since the native DisposableStack has a built-in mechanism to adopt non-disposable resources via stack.adopt(value, v => v.close()) and stack.defer(() => { value.close(); }), a workaround like Symbol.for() may not even be warranted.

}
if (typeof dispose !== "function") throw new TypeError("Object not disposable.");
env.stack.push({ value: value, dispose: dispose, async: async });
Copy link
Member

Choose a reason for hiding this comment

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

Unrelated: at what point will it be OK for us to use es6 features like object shorthands in our esnext downlevel helpers?

Copy link
Member Author

Choose a reason for hiding this comment

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

If our helpers were an AST instead of a string, then we could arguably downlevel them on demand. Unfortunately, that wouldn't work for tslib. Since we aren't downleveling the helpers, I'd say we can use new syntax only if we retire --target es5 and --target es3.

}
else if (async) {
env.stack.push({ async: true });
}
return value;
};`
};

/**
* @internal
*/
export const disposeResourcesHelper: UnscopedEmitHelper = {
name: "typescript:disposeResources",
importName: "__disposeResources",
scoped: false,
text: `
var __disposeResources = (this && this.__disposeResources) || (function (SuppressedError) {
return function (env) {
function fail(e) {
env.error = env.hasError ? new SuppressedError(e, env.error, "An error was suppressed during disposal.") : e;
env.hasError = true;
}
function next() {
while (env.stack.length) {
var rec = env.stack.pop();
try {
var result = rec.dispose && rec.dispose.call(rec.value);
if (rec.async) return Promise.resolve(result).then(next, function(e) { fail(e); return next(); });
}
catch (e) {
fail(e);
}
}
if (env.hasError) throw env.error;
}
return next();
};
})(typeof SuppressedError === "function" ? SuppressedError : function (error, suppressed, message) {
var e = new Error(message);
return e.name = "SuppressedError", e.error = error, e.suppressed = suppressed, e;
});`
};

let allUnscopedEmitHelpers: ReadonlyMap<string, UnscopedEmitHelper> | undefined;

/** @internal */
Expand Down Expand Up @@ -1399,7 +1484,9 @@ export function getAllUnscopedEmitHelpers() {
classPrivateFieldSetHelper,
classPrivateFieldInHelper,
createBindingHelper,
setModuleDefaultHelper
setModuleDefaultHelper,
addDisposableResourceHelper,
disposeResourcesHelper,
], helper => helper.name));
}

Expand Down
23 changes: 22 additions & 1 deletion src/compiler/factory/nodeConverters.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ import {
BindingOrAssignmentPattern,
Block,
cast,
ClassDeclaration,
ConciseBody,
Debug,
Expression,
Expand All @@ -17,6 +18,8 @@ import {
isBindingElement,
isBindingPattern,
isBlock,
isDefaultModifier,
isExportModifier,
isExpression,
isIdentifier,
isObjectBindingPattern,
Expand All @@ -39,6 +42,7 @@ export function createNodeConverters(factory: NodeFactory): NodeConverters {
return {
convertToFunctionBlock,
convertToFunctionExpression,
convertToClassExpression,
convertToArrayAssignmentElement,
convertToObjectAssignmentElement,
convertToAssignmentPattern,
Expand All @@ -59,7 +63,7 @@ export function createNodeConverters(factory: NodeFactory): NodeConverters {
function convertToFunctionExpression(node: FunctionDeclaration) {
if (!node.body) return Debug.fail(`Cannot convert a FunctionDeclaration without a body`);
const updated = factory.createFunctionExpression(
getModifiers(node),
getModifiers(node)?.filter(modifier => !isExportModifier(modifier) && !isDefaultModifier(modifier)),
node.asteriskToken,
node.name,
node.typeParameters,
Expand All @@ -75,6 +79,22 @@ export function createNodeConverters(factory: NodeFactory): NodeConverters {
return updated;
}

function convertToClassExpression(node: ClassDeclaration) {
const updated = factory.createClassExpression(
node.modifiers?.filter(modifier => !isExportModifier(modifier) && !isDefaultModifier(modifier)),
node.name,
node.typeParameters,
node.heritageClauses,
node.members
);
setOriginalNode(updated, node);
setTextRange(updated, node);
if (getStartsOnNewLine(node)) {
setStartsOnNewLine(updated, /*newLine*/ true);
}
return updated;
}

function convertToArrayAssignmentElement(element: ArrayBindingOrAssignmentElement) {
if (isBindingElement(element)) {
if (element.dotDotDotToken) {
Expand Down Expand Up @@ -163,6 +183,7 @@ export function createNodeConverters(factory: NodeFactory): NodeConverters {
export const nullNodeConverters: NodeConverters = {
convertToFunctionBlock: notImplemented,
convertToFunctionExpression: notImplemented,
convertToClassExpression: notImplemented,
convertToArrayAssignmentElement: notImplemented,
convertToObjectAssignmentElement: notImplemented,
convertToAssignmentPattern: notImplemented,
Expand Down