Add support for TypeScript 3.8 (#1134)

Add support for TypeScript 3.8

This upgrades the version of TypeScript Tsickle uses as a
`devDependency` and a `peerDependency`.

Support for Private Fields
--------------------------
Tsickle mostly ignores private fields, however it no longer warns when
not generating externs for them. Externs are not generated because the
fields no not exist on the class when downleveled.

Support for `export * as ns` Syntax
-----------------------------------
Tsickle compiles:

```ts
export * as ns from './namespace';
```

to

```ts
var tsickle_module_1_ = goog.require('project.namespace');
exports.ns = tsickle_module_1_;
```

New `tslib` Functions
---------------------
Private fields require two new functions in `tslib`:
`__classPrivateFieldGet` and `__classPrivateFieldSet`. Both of these
were added to Tsickle's version of `tslib.js` along with Closure type
annotations.

Other Changes
-------------
* The `import_export_typedef_conflict` test was removed because that
  code is no longer valid in TypeScript 3.8, and produces a compile
  error.
* `tslib@1.11.0` was added as a `devDependency`. Before it was being
  pulled in as a transitive dependency, but the packages pulling it in
  depend on `1.10`. To upgrade to TypeScript 3.8 Tsickle needs the latest
  version of `tslib`.

package.json

@@ -11,7 +11,7 @@
     "src/*"
   ],
   "peerDependencies": {
-    "typescript": "~3.7.3"
+    "typescript": "~3.8.2"
   },
   "devDependencies": {
     "@bazel/bazel": "^0.29.0",
@@ -29,8 +29,9 @@
     "prettier": "1.14.0",
     "source-map": "^0.7.3",
     "source-map-support": "^0.5.6",
+    "tslib": "1.11",
     "tslint": "5.11.0",
-    "typescript": "3.7.3"
+    "typescript": "3.8.2"
   },
   "scripts": {
     "build": "bazel build //:npm_package",

src/enum_transformer.ts

@@ -196,7 +196,11 @@ export function enumTransformer(typeChecker: ts.TypeChecker, diagnostics: ts.Dia
       for (const member of node.members) {
         const memberName = member.name;
         const memberType = getEnumMemberType(typeChecker, member);
-        if (memberType !== 'number') continue;
+        // Enum members cannot be named with a private identifier, although it
+        // is technically valid in the AST.
+        if (memberType !== 'number' || ts.isPrivateIdentifier(memberName)) {
+          continue;
+        }
 
         // TypeScript enum members can have Identifier names or String names.
         // We need to emit slightly different code to support these two syntaxes:

src/externs.ts

@@ -518,6 +518,12 @@ export function generateExterns(
           debugLocationStr(exportDeclaration, namespace)}\n`);
       return;
     }
+    if (ts.isNamespaceExport(exportDeclaration.exportClause)) {
+      // TODO(#1135): Support generating externs using this syntax.
+      emit(`\n// TODO(tsickle): export * as declaration in ${
+          debugLocationStr(exportDeclaration, namespace)}\n`);
+      return;
+    }
     for (const exportSpecifier of exportDeclaration.exportClause.elements) {
       // No need to do anything for properties exported under their original name.
       if (!exportSpecifier.propertyName) continue;

src/googmodule.ts

@@ -428,6 +428,45 @@ export function commonJsToGoogmoduleTransformer(
             ts.setTextRange(ts.createStatement(createGoogCall('require', arg)), stmt), stmt);
       }
 
+      /**
+       * Rewrites code generated by `export * as ns from 'ns'` to something like:
+       *
+       * ```
+       * const tsickle_module_n_ = goog.require('ns');
+       * exports.ns = tsickle_module_n_;
+       * ```
+       *
+       * Separating the `goog.require` and `exports.ns` assignment is required by Closure to
+       * correctly infer the type of the exported namespace.
+       */
+      function maybeRewriteExportStarAsNs(stmt: ts.Statement): ts.Statement[]|null {
+        // Ensure this looks something like `exports.ns = require('ns);`.
+        if (!ts.isExpressionStatement(stmt)) return null;
+        if (!ts.isBinaryExpression(stmt.expression)) return null;
+        if (stmt.expression.operatorToken.kind !== ts.SyntaxKind.EqualsToken) return null;
+
+        // Ensure the left side of the expression is an access on `exports`.
+        if (!ts.isPropertyAccessExpression(stmt.expression.left)) return null;
+        if (!ts.isIdentifier(stmt.expression.left.expression)) return null;
+        if (stmt.expression.left.expression.escapedText !== 'exports') return null;
+
+        // Grab the call to `require`, and exit early if not calling `require`.
+        if (!ts.isCallExpression(stmt.expression.right)) return null;
+        const ident = ts.createIdentifier(nextModuleVar());
+        const require = maybeCreateGoogRequire(stmt, stmt.expression.right, ident);
+        if (!require) return null;
+
+        const exportedName = stmt.expression.left.name;
+        const exportStmt = ts.setOriginalNode(
+            ts.setTextRange(
+                ts.createStatement(ts.createAssignment(
+                    ts.createPropertyAccess(ts.createIdentifier('exports'), exportedName), ident)),
+                stmt),
+            stmt);
+
+        return [require, exportStmt];
+      }
+
       /**
        * visitTopLevelStatement implements the main CommonJS to goog.module conversion. It visits a
        * SourceFile level statement and adds a (possibly) transformed representation of it into
@@ -480,6 +519,14 @@ export function commonJsToGoogmoduleTransformer(
               return;
             }
             // Check for:
+            //   exports.ns = require('...');
+            // which is generated by the `export * as ns from` syntax.
+            const exportStarAsNs = maybeRewriteExportStarAsNs(exprStmt);
+            if (exportStarAsNs) {
+              stmts.push(...exportStarAsNs);
+              return;
+            }
+            // Check for:
             //   "require('foo');" (a require for its side effects)
             const expr = exprStmt.expression;
             if (!ts.isCallExpression(expr)) break;

src/jsdoc_transformer.ts

@@ -341,9 +341,18 @@ function createClosurePropertyDeclaration(
     optional: boolean): ts.Statement {
   const name = propertyName(prop);
   if (!name) {
-    mtt.debugWarn(prop, `handle unnamed member:\n${escapeForComment(prop.getText())}`);
-    return transformerUtil.createMultiLineComment(
-        prop, `Skipping unnamed member:\n${escapeForComment(prop.getText())}`);
+    // Skip warning for private identifiers because it is expected they are skipped in the
+    // Closure output.
+    // TODO(rdel): Once Closure Compiler determines how private properties should be represented,
+    // adjust this output accordingly.
+    if (ts.isPrivateIdentifier(prop.name)) {
+      return transformerUtil.createMultiLineComment(
+          prop, `Skipping private member:\n${escapeForComment(prop.getText())}`);
+    } else {
+      mtt.debugWarn(prop, `handle unnamed member:\n${escapeForComment(prop.getText())}`);
+      return transformerUtil.createMultiLineComment(
+          prop, `Skipping unnamed member:\n${escapeForComment(prop.getText())}`);
+    }
   }
 
   if (name === 'prototype') {
@@ -891,10 +900,13 @@ export function jsdocTransformer(
               typesToExport.push([sym.name, sym]);
             }
           }
+          const isTypeOnlyExport = false;
           exportDecl = ts.updateExportDeclaration(
               exportDecl, exportDecl.decorators, exportDecl.modifiers,
-              ts.createNamedExports(exportSpecifiers), exportDecl.moduleSpecifier);
-        } else {
+              ts.createNamedExports(exportSpecifiers), exportDecl.moduleSpecifier,
+              isTypeOnlyExport);
+        } else if (ts.isNamedExports(exportDecl.exportClause)) {
+          // export {a, b, c} from 'abc';
           for (const exp of exportDecl.exportClause.elements) {
             const exportedName = transformerUtil.getIdentifierText(exp.name);
             typesToExport.push(

test/googmodule_test.ts

@@ -464,6 +464,18 @@ console.log(starImport, file_js_1.namedImport, file_js_1.renamedFrom, starImport
 `);
     });
 
+    it('handles ESM namespace exports', () => {
+      const before = `
+        export * as ns from './exportStarAsNs.js';
+      `;
+
+      expectJsTranspilation(before).toBe(`goog.module('project.file');
+var module = module || { id: 'project/file.js' };
+var tsickle_module_1_ = goog.require('project.exportStarAsNs');
+exports.ns = tsickle_module_1_;
+`);
+    });
+
     it('elides imports of goog.js', () => {
       const before = `
         import * as goog from 'google3/javascript/closure/goog.js';

test_files/class/private_field.js

@@ -0,0 +1,40 @@
+goog.module('test_files.class.private_field');
+var module = module || { id: 'test_files/class/private_field.ts' };
+module = module;
+var _someField;
+const tslib_1 = goog.require('tslib');
+/**
+ *
+ * @fileoverview Tests the generation of private field accessors from Tsickle. They do not generate
+ * any externs, as they do not exist on the class themselves when downleveled by TypeScript.
+ *
+ * Generated from: test_files/class/private_field.ts
+ * @suppress {checkTypes,constantProperty,extraRequire,missingOverride,missingReturn,unusedPrivateMembers,uselessCode} checked by tsc
+ */
+class ContainsPrivateField {
+    /**
+     * @param {string} arg
+     */
+    constructor(arg) {
+        _someField.set(this, void 0);
+        tslib_1.__classPrivateFieldSet(this, _someField, arg);
+    }
+    /**
+     * @param {string} value
+     * @return {void}
+     */
+    setSomeField(value) {
+        tslib_1.__classPrivateFieldSet(this, _someField, value);
+    }
+    /**
+     * @return {string}
+     */
+    getSomeField() {
+        return tslib_1.__classPrivateFieldGet(this, _someField);
+    }
+}
+_someField = new WeakMap();
+if (false) {
+    /* Skipping private member:
+    #someField: string;*/
+}

test_files/class/private_field.ts

@@ -0,0 +1,22 @@
+/**
+ * @fileoverview Tests the generation of private field accessors from Tsickle. They do not generate
+ * any externs, as they do not exist on the class themselves when downleveled by TypeScript.
+ */
+
+export {};
+
+class ContainsPrivateField {
+  #someField: string;
+
+  constructor(arg: string) {
+    this.#someField = arg;
+  }
+
+  setSomeField(value: string) {
+    this.#someField = value;
+  }
+
+  getSomeField() {
+    return this.#someField;
+  }
+}

test_files/decorator/decorator.js

@@ -79,8 +79,7 @@ class DecoratorTest {
          * @param {string} x
          * @return {void}
          */
-        constructor(x) { } }) {
-    }
+        constructor(x) { } }) { }
     /**
      * @return {number}
      */

test_files/decorator_nested_scope/decorator_nested_scope.js

@@ -17,8 +17,7 @@ function main() {
         /**
          * @param {!SomeService} service
          */
-        constructor(service) {
-        }
+        constructor(service) { }
     }
     TestComp3.decorators = [
         { type: Component },

test_files/export_star_as_ns/ns.js

@@ -0,0 +1,13 @@
+/**
+ * @fileoverview added by tsickle
+ * Generated from: test_files/export_star_as_ns/ns.ts
+ * @suppress {checkTypes,constantProperty,extraRequire,missingOverride,missingReturn,unusedPrivateMembers,uselessCode} checked by tsc
+ */
+goog.module('test_files.export_star_as_ns.ns');
+var module = module || { id: 'test_files/export_star_as_ns/ns.ts' };
+module = module;
+goog.require('tslib');
+/** @type {number} */
+exports.x = 10;
+/** @type {string} */
+exports.y = 'y!';

test_files/export_star_as_ns/ns.ts

@@ -0,0 +1,2 @@
+export const x = 10;
+export const y = 'y!';

test_files/export_star_as_ns/star_as_ns.js

@@ -0,0 +1,16 @@
+/**
+ *
+ * @fileoverview Tests exporting a namespace with a given name from Closure. This doesn't expand
+ * each export like the `export * from '...'` syntax, so it's output just an assignment of the
+ * imported module to a property on `exports`.
+ *
+ * Generated from: test_files/export_star_as_ns/star_as_ns.ts
+ * @suppress {checkTypes,constantProperty,extraRequire,missingOverride,missingReturn,unusedPrivateMembers,uselessCode} checked by tsc
+ */
+goog.module('test_files.export_star_as_ns.star_as_ns');
+var module = module || { id: 'test_files/export_star_as_ns/star_as_ns.ts' };
+module = module;
+goog.require('tslib');
+const tsickle_ns_1 = goog.requireType("test_files.export_star_as_ns.ns");
+const tsickle_module_1_ = goog.require('test_files.export_star_as_ns.ns');
+exports.ns = tsickle_module_1_;

test_files/export_star_as_ns/star_as_ns.ts

@@ -0,0 +1,7 @@
+/**
+ * @fileoverview Tests exporting a namespace with a given name from Closure. This doesn't expand
+ * each export like the `export * from '...'` syntax, so it's output just an assignment of the
+ * imported module to a property on `exports`.
+ */
+
+export * as ns from './ns';

third_party/tslib/tslib.js

@@ -369,3 +369,29 @@ exports.__makeTemplateObject = function(cooked, raw) {
     if (Object.defineProperty) { Object.defineProperty(cooked, "raw", { value: raw }); } else { cooked.raw = raw; }
     return cooked;
 };
+
+/**
+ * @param {?} receiver
+ * @param {?} privateMap
+ * @return {?}
+ */
+exports.__classPrivateFieldGet = function (receiver, privateMap) {
+  if (!privateMap.has(receiver)) {
+      throw new TypeError("attempted to get private field on non-instance");
+  }
+  return privateMap.get(receiver);
+};
+
+/**
+ * @param {?} receiver
+ * @param {?} privateMap
+ * @param {?} value
+ * @return {?}
+ */
+exports.__classPrivateFieldSet = function (receiver, privateMap, value) {
+  if (!privateMap.has(receiver)) {
+      throw new TypeError("attempted to set private field on non-instance");
+  }
+  privateMap.set(receiver, value);
+  return value;
+}

yarn.lock

@@ -1166,6 +1166,11 @@ test-exclude@^5.2.2:
     read-pkg-up "^4.0.0"
     require-main-filename "^2.0.0"
 
+tslib@1.11:
+  version "1.11.0"
+  resolved "https://registry.yarnpkg.com/tslib/-/tslib-1.11.0.tgz#f1f3528301621a53220d58373ae510ff747a66bc"
+  integrity sha512-BmndXUtiTn/VDDrJzQE7Mm22Ix3PxgLltW9bSNLoeCY31gnG2OPx0QqJnuc9oMIKioYrz487i6K9o4Pdn0j+Kg==
+
 tslib@^1.8.0, tslib@^1.8.1:
   version "1.10.0"
   resolved "https://registry.yarnpkg.com/tslib/-/tslib-1.10.0.tgz#c3c19f95973fb0a62973fb09d90d961ee43e5c8a"
@@ -1203,10 +1208,10 @@ tsutils@^2.27.2:
   dependencies:
     tslib "^1.8.1"
 
-typescript@3.7.3:
-  version "3.7.3"
-  resolved "https://registry.yarnpkg.com/typescript/-/typescript-3.7.3.tgz#b36840668a16458a7025b9eabfad11b66ab85c69"
-  integrity sha512-Mcr/Qk7hXqFBXMN7p7Lusj1ktCBydylfQM/FZCk5glCNQJrCUKPkMHdo9R0MTFWsC/4kPFvDS0fDPvukfCkFsw==
+typescript@3.8.2:
+  version "3.8.2"
+  resolved "https://registry.yarnpkg.com/typescript/-/typescript-3.8.2.tgz#91d6868aaead7da74f493c553aeff76c0c0b1d5a"
+  integrity sha512-EgOVgL/4xfVrCMbhYKUQTdF37SQn4Iw73H5BgCrF1Abdun7Kwy/QZsE/ssAy0y4LxBbvua3PIbFsbRczWWnDdQ==
 
 uglify-js@^3.1.4:
   version "3.6.0"