Add wrapper to emit statics/decorators inside es5 class #16120

rbuckton · 2017-05-27T20:16:04Z

This PR changes our class emit when targeting ES5 to emit static initializers and decorators inside of the ES5 class IIFE.

This is accomplished by first wrapping the ES6 class and related statements inside of an IIFE in the ts transformer, then unwrapping the IIFE and merging it in the es2015 transformer.

This PR also adds some diagnostic information to Node and Type that is only exposed when debugging, to assist with debugging the compiler. This also adds the capability to remove Debug.assert, Debug.fail and related debugging assertions from the call stack on NodeJS to better interpret where an assertion failure occurred when running tests (as our test harness sets the stack trace limit to 1).

Fixes #15857

ghost

(Note: I didn't review changes to .sourcemap.txt baselines)

ghost · 2017-05-30T13:50:10Z

src/compiler/core.ts

+ return value !== undefined && test(value) ? value : undefined;
+ }
+
+ export function cast<TOut extends TIn, TIn = any>(value: TIn | undefined, test: (value: TIn) => value is TOut): TOut {


Don't see any reason to allow an undefined parameter since the function will always fail in that case.

If we ever turn on --strictNullChecks there may be cases where value may be typed undefined.

ghost · 2017-05-30T13:53:56Z

src/compiler/utilities.ts

+ return false;
+ }
+
+ const returnStatement = tryCast(elementAt(statements, isVariableStatement(lastOrUndefined(statements)) ? -2 : -1), isReturnStatement);


This could be just:

const returnStatement = statements[statements.length - (isVariableStatement(lastOrUndefined(statements)) ? 2 : 1)]; return isReturnStatement(returnStatement) && returnStatement.expression && isIdentifier(skipOuterExpressions(returnStatement.expression));

I still like this better as just return condition instead of if (!condition) { return false; } return true;

ghost · 2017-05-30T13:59:27Z

src/compiler/factory.ts

@@ -3714,6 +3736,21 @@ namespace ts {
 All = Parentheses | Assertions | PartiallyEmittedExpressions
 }

+ export type OuterExpression = ParenthesizedExpression | TypeAssertion | AsExpression | PartiallyEmittedExpression;


Why not NonNullExpression?

It didn't exist when skipOuterExpressions was initially added.

ghost · 2017-05-30T14:07:11Z

src/compiler/utilities.ts

+ return false;
+ }
+
+ const func = <FunctionExpression>skipParentheses((<CallExpression>node).expression);


What makes this cast valid?

isImmediatelyInvokedFunctionExpression already ensured the node has the correct shape.

ghost · 2017-05-30T14:09:33Z

src/compiler/transformers/es2015.ts

+
+ // We skip any outer expressions in a number of places to get to the innermost
+ // expression, but we will restore them later to preserve comments and source maps.
+ const body = cast(skipOuterExpressions(node.expression), isFunctionExpression).body;


In isTypeScriptClassWrapper, this just skips parentheses.

ghost · 2017-05-30T14:41:42Z

src/compiler/types.ts

@@ -3074,6 +3074,7 @@ namespace ts {
 export interface Type {
 flags: TypeFlags; // Flags
 /* @internal */ id: number; // Unique ID
+ /* @internal */ checker: TypeChecker;


This should definitely be documented as only being present in debug mode... better, don't define it here at all and require a cast to DebugType.

It's not only present in debug mode. It's also present when using the language service. See https://github.com/Microsoft/TypeScript/blob/master/src/services/services.ts#L382

ghost · 2017-05-30T14:43:48Z

src/compiler/types.ts

@@ -3977,34 +3978,34 @@ namespace ts {
 }

 export const enum EmitFlags {
- SingleLine = 1 << 0, // The contents of this node should be emitted on a single line.


What's the point of the whitespace change here? It seems arbitrary how much to use.

Trying to keep comments aligned for readability, except where the enum member is significantly long.

ghost · 2017-05-30T14:47:33Z

src/compiler/utilities.ts

@@ -850,10 +850,88 @@ namespace ts {
 return node && (node.kind === SyntaxKind.GetAccessor || node.kind === SyntaxKind.SetAccessor);
 }

- export function isClassLike(node: Node): node is ClassLikeDeclaration {
+ export function isClassLike(node: Node | undefined): node is ClassLikeDeclaration {


I would rather see type predicates like this take defined inputs. I have a PR (#16121) making this public and would prefer to keep just Node as the parameter type; we can keep the node && in the body for now, but if we ever start using --strictNullChecks it should become the caller's responsibility.

ghost · 2017-05-30T14:48:42Z

src/compiler/utilities.ts

 return node && (node.kind === SyntaxKind.ClassDeclaration || node.kind === SyntaxKind.ClassExpression);
 }

+ export function isImmediatelyInvokedFunctionExpression(node: Node) {


No need to export. isTypeScriptClassWrapper could also be moved closer to its use.

ghost · 2017-05-30T14:53:55Z

src/compiler/utilities.ts

+ }
+
+ const syntaxKindCache: string[] = [];
+ export function formatSyntaxKind(kind: SyntaxKind): string {


If this is just for debugging, it shouldn't need a cache?

Its for debugging, yes. It probably doesn't need one, no.

yuit · 2017-05-30T18:52:42Z