Skip to content

Commit

Permalink
Add support for TypeScript 3.8
Browse files Browse the repository at this point in the history
This upgrades the version of TypeScript Tsickle uses as a
`devDependency` and a `peerDependency`, which in turn upgrades the
`tslib` version.

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.
  • Loading branch information
rrdelaney committed Feb 27, 2020
1 parent 153aaac commit 55cc2db
Show file tree
Hide file tree
Showing 20 changed files with 357 additions and 163 deletions.
4 changes: 2 additions & 2 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@
"src/*"
],
"peerDependencies": {
"typescript": "~3.7.3"
"typescript": "~3.8.2"
},
"devDependencies": {
"@bazel/bazel": "^0.29.0",
Expand All @@ -30,7 +30,7 @@
"source-map": "^0.7.3",
"source-map-support": "^0.5.6",
"tslint": "5.11.0",
"typescript": "3.7.3"
"typescript": "3.8.2"
},
"scripts": {
"build": "bazel build //:npm_package",
Expand Down
6 changes: 5 additions & 1 deletion src/enum_transformer.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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:
Expand Down
5 changes: 5 additions & 0 deletions src/externs.ts
Original file line number Diff line number Diff line change
Expand Up @@ -518,6 +518,11 @@ export function generateExterns(
debugLocationStr(exportDeclaration, namespace)}\n`);
return;
}
if (ts.isNamespaceExport(exportDeclaration.exportClause)) {
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;
Expand Down
47 changes: 47 additions & 0 deletions src/googmodule.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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;
Expand Down
20 changes: 15 additions & 5 deletions src/jsdoc_transformer.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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') {
Expand Down Expand Up @@ -893,8 +902,9 @@ export function jsdocTransformer(
}
exportDecl = ts.updateExportDeclaration(
exportDecl, exportDecl.decorators, exportDecl.modifiers,
ts.createNamedExports(exportSpecifiers), exportDecl.moduleSpecifier);
} else {
ts.createNamedExports(exportSpecifiers), exportDecl.moduleSpecifier, false);
} 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(
Expand Down
5 changes: 4 additions & 1 deletion test/googmodule_test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -420,6 +420,7 @@ goog.loadedModules_['legacy.bar.baz'] = { exports: exports, type: goog.ModuleTyp
import * as starImport from './relpath.js';
import {namedImport, renamedFrom as renamedTo} from '../dotdot/file.js';
export * from './exportStar.js';
export * as ns from './exportStarAsNs.js';
export {namedRexport, renamedExportFrom as renamedExportTo} from './namedExport.js';
import 'google3/workspace/rooted/file.js';
import * as starImportWorkspaceRooted from 'google3/workspace/rooted/otherFile.js';
Expand All @@ -432,10 +433,12 @@ var starImport = goog.require('project.relpath');
var file_js_1 = goog.require('dotdot.file');
var tsickle_module_1_ = goog.require('project.exportStar');
tslib_1.__exportStar(tsickle_module_1_, exports);
var tsickle_module_2_ = goog.require('project.exportStarAsNs');
exports.ns = tsickle_module_2_;
var namedExport_js_1 = goog.require('project.namedExport');
exports.namedRexport = namedExport_js_1.namedRexport;
exports.renamedExportTo = namedExport_js_1.renamedExportFrom;
var tsickle_module_2_ = goog.require('google3.workspace.rooted.file');
var tsickle_module_3_ = goog.require('google3.workspace.rooted.file');
var starImportWorkspaceRooted = goog.require('google3.workspace.rooted.otherFile');
console.log(starImport, file_js_1.namedImport, file_js_1.renamedFrom, starImportWorkspaceRooted);
`);
Expand Down
40 changes: 40 additions & 0 deletions test_files/class/private_field.js
Original file line number Diff line number Diff line change
@@ -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;*/
}
22 changes: 22 additions & 0 deletions test_files/class/private_field.ts
Original file line number Diff line number Diff line change
@@ -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;
}
}
3 changes: 1 addition & 2 deletions test_files/decorator/decorator.js
Original file line number Diff line number Diff line change
Expand Up @@ -79,8 +79,7 @@ class DecoratorTest {
* @param {string} x
* @return {void}
*/
constructor(x) { } }) {
}
constructor(x) { } }) { }
/**
* @return {number}
*/
Expand Down
3 changes: 1 addition & 2 deletions test_files/decorator_nested_scope/decorator_nested_scope.js
Original file line number Diff line number Diff line change
Expand Up @@ -16,8 +16,7 @@ function main() {
/**
* @param {!SomeService} service
*/
constructor(service) {
}
constructor(service) { }
}
TestComp3.decorators = [
{ type: Component },
Expand Down
12 changes: 12 additions & 0 deletions test_files/export_star_as_ns/ns.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
/**
* @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;
/** @type {number} */
exports.x = 10;
/** @type {string} */
exports.y = 'y!';
2 changes: 2 additions & 0 deletions test_files/export_star_as_ns/ns.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
export const x = 10;
export const y = 'y!';
15 changes: 15 additions & 0 deletions test_files/export_star_as_ns/star_as_ns.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
/**
*
* @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;
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_;
7 changes: 7 additions & 0 deletions test_files/export_star_as_ns/star_as_ns.ts
Original file line number Diff line number Diff line change
@@ -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';
10 changes: 0 additions & 10 deletions test_files/import_export_typedef_conflict/exporter.js

This file was deleted.

1 change: 0 additions & 1 deletion test_files/import_export_typedef_conflict/exporter.ts

This file was deleted.

This file was deleted.

This file was deleted.

26 changes: 26 additions & 0 deletions third_party/tslib/tslib.js
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}
Loading

0 comments on commit 55cc2db

Please sign in to comment.