fix(ngcc): support `defineProperty()` re-exports in CommonJS and UMD (#36989)

In TypeScript 3.9 some re-export syntaxes have changed to be getter
functions (created by calls to `Object.defineProperty()`) rather than
simple property accessors.

This commit adds support into the CommonJS and UMD reflection hosts
for this style of re-export syntax.

PR Close #36989

packages/compiler-cli/ngcc/src/host/commonjs_host.ts

@@ -14,7 +14,7 @@ import {Logger} from '../logging/logger';
 import {BundleProgram} from '../packages/bundle_program';
 import {FactoryMap, isDefined} from '../utils';
 
-import {ExportDeclaration, ExportStatement, findNamespaceOfIdentifier, findRequireCallReference, isExportStatement, isRequireCall, isWildcardReexportStatement, RequireCall, WildcardReexportStatement} from './commonjs_umd_utils';
+import {DefinePropertyReexportStatement, ExportDeclaration, ExportStatement, extractGetterFnExpression, findNamespaceOfIdentifier, findRequireCallReference, isDefinePropertyReexportStatement, isExportStatement, isExternalImport, isRequireCall, isWildcardReexportStatement, RequireCall, WildcardReexportStatement} from './commonjs_umd_utils';
 import {Esm5ReflectionHost} from './esm5_host';
 import {NgccClassSymbol} from './ngcc_host';
 
@@ -106,6 +106,11 @@ export class CommonJsReflectionHost extends Esm5ReflectionHost {
         for (const reexport of reexports) {
           moduleMap.set(reexport.name, reexport.declaration);
         }
+      } else if (isDefinePropertyReexportStatement(statement)) {
+        const exportDeclaration = this.extractCommonJsDefinePropertyExportDeclaration(statement);
+        if (exportDeclaration !== null) {
+          moduleMap.set(exportDeclaration.name, exportDeclaration.declaration);
+        }
       }
     }
     return moduleMap;
@@ -139,7 +144,7 @@ export class CommonJsReflectionHost extends Esm5ReflectionHost {
       return [];
     }
 
-    const viaModule = importPath.startsWith('./') ? null : importPath;
+    const viaModule = isExternalImport(importPath) ? importPath : null;
     const reexports: ExportDeclaration[] = [];
     importedExports.forEach((declaration, name) => {
       if (viaModule !== null && declaration.viaModule === null) {
@@ -150,6 +155,17 @@ export class CommonJsReflectionHost extends Esm5ReflectionHost {
     return reexports;
   }
 
+  private extractCommonJsDefinePropertyExportDeclaration(
+      statement: DefinePropertyReexportStatement): ExportDeclaration|null {
+    const args = statement.expression.arguments;
+    const name = args[1].text;
+    const getterFnExpression = extractGetterFnExpression(statement);
+    if (getterFnExpression === null) {
+      return null;
+    }
+    return this.extractCommonJsExportDeclaration(name, getterFnExpression);
+  }
+
   private findCommonJsImport(id: ts.Identifier): RequireCall|null {
     // Is `id` a namespaced property access, e.g. `Directive` in `core.Directive`?
     // If so capture the symbol of the namespace, e.g. `core`.
@@ -187,7 +203,7 @@ export class CommonJsReflectionHost extends Esm5ReflectionHost {
     if (module === undefined) {
       return null;
     }
-    const viaModule = importPath.startsWith('./') ? null : importPath;
+    const viaModule = isExternalImport(importPath) ? importPath : null;
     return {node: module, known: null, viaModule, identity: null};
   }
 

packages/compiler-cli/ngcc/src/host/commonjs_umd_utils.ts

@@ -45,6 +45,20 @@ export interface WildcardReexportStatement extends ts.ExpressionStatement {
   expression: ts.CallExpression;
 }
 
+/**
+ * A CommonJS or UMD re-export statement using an `Object.defineProperty()` call.
+ * For example:
+ *
+ * ```
+ * Object.defineProperty(exports, "<exported-id>",
+ *     { enumerable: true, get: function () { return <imported-id>; } });
+ * ```
+ */
+export interface DefinePropertyReexportStatement extends ts.ExpressionStatement {
+  expression: ts.CallExpression&
+      {arguments: [ts.Identifier, ts.StringLiteral, ts.ObjectLiteralExpression]};
+}
+
 export interface RequireCall extends ts.CallExpression {
   arguments: ts.CallExpression['arguments']&[ts.StringLiteral];
 }
@@ -133,6 +147,69 @@ export function isWildcardReexportStatement(stmt: ts.Statement): stmt is Wildcar
   return stmt.expression.arguments.length > 0;
 }
 
+
+/**
+ * Check whether the statement is a re-export of the form:
+ *
+ * ```
+ * Object.defineProperty(exports, "<export-name>",
+ *     { enumerable: true, get: function () { return <import-name>; } });
+ * ```
+ */
+export function isDefinePropertyReexportStatement(stmt: ts.Statement):
+    stmt is DefinePropertyReexportStatement {
+  if (!ts.isExpressionStatement(stmt) || !ts.isCallExpression(stmt.expression)) {
+    return false;
+  }
+
+  // Check for Object.defineProperty
+  if (!ts.isPropertyAccessExpression(stmt.expression.expression) ||
+      !ts.isIdentifier(stmt.expression.expression.expression) ||
+      stmt.expression.expression.expression.text !== 'Object' ||
+      !ts.isIdentifier(stmt.expression.expression.name) ||
+      stmt.expression.expression.name.text !== 'defineProperty') {
+    return false;
+  }
+
+  const args = stmt.expression.arguments;
+  if (args.length !== 3) {
+    return false;
+  }
+  const exportsObject = args[0];
+  if (!ts.isIdentifier(exportsObject) || exportsObject.text !== 'exports') {
+    return false;
+  }
+
+  const propertyKey = args[1];
+  if (!ts.isStringLiteral(propertyKey)) {
+    return false;
+  }
+
+  const propertyDescriptor = args[2];
+  if (!ts.isObjectLiteralExpression(propertyDescriptor)) {
+    return false;
+  }
+
+  return (propertyDescriptor.properties.some(
+      prop => prop.name !== undefined && ts.isIdentifier(prop.name) && prop.name.text === 'get'));
+}
+
+export function extractGetterFnExpression(statement: DefinePropertyReexportStatement):
+    ts.Expression|null {
+  const args = statement.expression.arguments;
+  const getterFn = args[2].properties.find(
+      prop => prop.name !== undefined && ts.isIdentifier(prop.name) && prop.name.text === 'get');
+  if (getterFn === undefined || !ts.isPropertyAssignment(getterFn) ||
+      !ts.isFunctionExpression(getterFn.initializer)) {
+    return null;
+  }
+  const returnStatement = getterFn.initializer.body.statements[0];
+  if (!ts.isReturnStatement(returnStatement) || returnStatement.expression === undefined) {
+    return null;
+  }
+  return returnStatement.expression;
+}
+
 /**
  * Check whether the specified `ts.Node` represents a `require()` call, i.e. an call expression of
  * the form: `require('<foo>')`
@@ -142,3 +219,7 @@ export function isRequireCall(node: ts.Node): node is RequireCall {
       node.expression.text === 'require' && node.arguments.length === 1 &&
       ts.isStringLiteral(node.arguments[0]);
 }
+
+export function isExternalImport(path: string): boolean {
+  return !/^\.\.?(\/|$)/.test(path);
+}

packages/compiler-cli/ngcc/src/host/umd_host.ts

@@ -14,7 +14,7 @@ import {Logger} from '../logging/logger';
 import {BundleProgram} from '../packages/bundle_program';
 import {FactoryMap, getTsHelperFnFromIdentifier, stripExtension} from '../utils';
 
-import {ExportDeclaration, ExportStatement, findNamespaceOfIdentifier, findRequireCallReference, isExportStatement, isRequireCall, isWildcardReexportStatement, WildcardReexportStatement} from './commonjs_umd_utils';
+import {DefinePropertyReexportStatement, ExportDeclaration, ExportStatement, extractGetterFnExpression, findNamespaceOfIdentifier, findRequireCallReference, isDefinePropertyReexportStatement, isExportStatement, isExternalImport, isRequireCall, isWildcardReexportStatement, WildcardReexportStatement} from './commonjs_umd_utils';
 import {Esm5ReflectionHost} from './esm5_host';
 import {stripParentheses} from './utils';
 
@@ -44,7 +44,8 @@ export class UmdReflectionHost extends Esm5ReflectionHost {
   }
 
   getDeclarationOfIdentifier(id: ts.Identifier): Declaration|null {
-    return this.getUmdImportedDeclaration(id) || super.getDeclarationOfIdentifier(id);
+    return this.getUmdModuleDeclaration(id) || this.getUmdDeclaration(id) ||
+        super.getDeclarationOfIdentifier(id);
   }
 
   getExportsOfModule(module: ts.Node): Map<string, Declaration>|null {
@@ -90,13 +91,18 @@ export class UmdReflectionHost extends Esm5ReflectionHost {
     const moduleMap = new Map<string, Declaration>();
     for (const statement of this.getModuleStatements(sourceFile)) {
       if (isExportStatement(statement)) {
-        const exportDeclaration = this.extractUmdExportDeclaration(statement);
+        const exportDeclaration = this.extractBasicUmdExportDeclaration(statement);
         moduleMap.set(exportDeclaration.name, exportDeclaration.declaration);
       } else if (isWildcardReexportStatement(statement)) {
         const reexports = this.extractUmdWildcardReexports(statement, sourceFile);
         for (const reexport of reexports) {
           moduleMap.set(reexport.name, reexport.declaration);
         }
+      } else if (isDefinePropertyReexportStatement(statement)) {
+        const exportDeclaration = this.extractUmdDefinePropertyExportDeclaration(statement);
+        if (exportDeclaration !== null) {
+          moduleMap.set(exportDeclaration.name, exportDeclaration.declaration);
+        }
       }
     }
     return moduleMap;
@@ -126,23 +132,10 @@ export class UmdReflectionHost extends Esm5ReflectionHost {
     return importPath;
   }
 
-  private extractUmdExportDeclaration(statement: ExportStatement): ExportDeclaration {
+  private extractBasicUmdExportDeclaration(statement: ExportStatement): ExportDeclaration {
-    const exportExpression = statement.expression.right;
-    const declaration = this.getDeclarationOfExpression(exportExpression);
     const name = statement.expression.left.name.text;
-    if (declaration !== null) {
+    const exportExpression = statement.expression.right;
-      return {name, declaration};
+    return this.extractUmdExportDeclaration(name, exportExpression);
-    } else {
-      return {
-        name,
-        declaration: {
-          node: null,
-          known: null,
-          expression: exportExpression,
-          viaModule: null,
-        },
-      };
-    }
   }
 
   private extractUmdWildcardReexports(
@@ -192,6 +185,28 @@ export class UmdReflectionHost extends Esm5ReflectionHost {
     return reexports;
   }
 
+  private extractUmdDefinePropertyExportDeclaration(statement: DefinePropertyReexportStatement):
+      ExportDeclaration|null {
+    const args = statement.expression.arguments;
+    const name = args[1].text;
+    const getterFnExpression = extractGetterFnExpression(statement);
+    if (getterFnExpression === null) {
+      return null;
+    }
+    return this.extractUmdExportDeclaration(name, getterFnExpression);
+  }
+
+  private extractUmdExportDeclaration(name: string, expression: ts.Expression): ExportDeclaration {
+    const declaration = this.getDeclarationOfExpression(expression);
+    if (declaration !== null) {
+      return {name, declaration};
+    }
+    return {
+      name,
+      declaration: {node: null, known: null, expression, viaModule: null},
+    };
+  }
+
   /**
    * Is the identifier a parameter on a UMD factory function, e.g. `function factory(this, core)`?
    * If so then return its declaration.
@@ -202,25 +217,67 @@ export class UmdReflectionHost extends Esm5ReflectionHost {
     return declaration && ts.isParameter(declaration) ? declaration : null;
   }
 
-  private getUmdImportedDeclaration(id: ts.Identifier): Declaration|null {
+  private getUmdDeclaration(id: ts.Identifier): Declaration|null {
-    const importInfo = this.getImportOfIdentifier(id);
+    const nsIdentifier = findNamespaceOfIdentifier(id);
-    if (importInfo === null) {
+    if (nsIdentifier === null) {
+      return null;
+    }
+    const moduleDeclaration = this.getUmdModuleDeclaration(nsIdentifier);
+    if (moduleDeclaration === null || moduleDeclaration.node === null ||
+        !ts.isSourceFile(moduleDeclaration.node)) {
       return null;
     }
 
-    const importedFile = this.resolveModuleName(importInfo.from, id.getSourceFile());
+    const moduleExports = this.getExportsOfModule(moduleDeclaration.node);
-    if (importedFile === undefined) {
+    if (moduleExports === null) {
       return null;
     }
 
-    // We need to add the `viaModule` because  the `getExportsOfModule()` call
+    // We need to compute the `viaModule` because  the `getExportsOfModule()` call
     // did not know that we were importing the declaration.
-    return {
+    const declaration = moduleExports.get(id.text)!;
-      node: importedFile,
+
-      known: getTsHelperFnFromIdentifier(id),
+    if (!moduleExports.has(id.text)) {
-      viaModule: importInfo.from,
+      return null;
-      identity: null
+    }
-    };
+
+    // We need to compute the `viaModule` because  the `getExportsOfModule()` call
+    // did not know that we were importing the declaration.
+    const viaModule =
+        declaration.viaModule === null ? moduleDeclaration.viaModule : declaration.viaModule;
+
+    return {...declaration, viaModule, known: getTsHelperFnFromIdentifier(id)};
+  }
+
+  private getUmdModuleDeclaration(id: ts.Identifier): Declaration|null {
+    const importPath = this.getImportPathFromParameter(id) || this.getImportPathFromRequireCall(id);
+    if (importPath === null) {
+      return null;
+    }
+
+    const module = this.resolveModuleName(importPath, id.getSourceFile());
+    if (module === undefined) {
+      return null;
+    }
+
+    const viaModule = isExternalImport(importPath) ? importPath : null;
+    return {node: module, viaModule, known: null, identity: null};
+  }
+
+  private getImportPathFromParameter(id: ts.Identifier): string|null {
+    const importParameter = this.findUmdImportParameter(id);
+    if (importParameter === null) {
+      return null;
+    }
+    return this.getUmdImportPath(importParameter);
+  }
+
+  private getImportPathFromRequireCall(id: ts.Identifier): string|null {
+    const requireCall = findRequireCallReference(id, this.checker);
+    if (requireCall === null) {
+      return null;
+    }
+    return requireCall.arguments[0].text;
   }
 
   private resolveModuleName(moduleName: string, containingFile: ts.SourceFile): ts.SourceFile

packages/compiler-cli/ngcc/test/host/commonjs_host_spec.ts

@@ -517,8 +517,8 @@ var c = file_a.a;
               `var b_module = require('./b_module');\n` +
               `var xtra_module = require('./xtra_module');\n` +
               `var wildcard_reexports_emitted_helpers = require('./wildcard_reexports_emitted_helpers');\n` +
-              `var wildcard_reexports_imported_helpers = require('./wildcard_reexports_imported_helpers');\n`,
+              `var wildcard_reexports_imported_helpers = require('./wildcard_reexports_imported_helpers');\n` +
-
+              `var define_property_reexports = require('./define_property_reexports');\n`
         },
         {
           name: _('/a_module.js'),
@@ -570,6 +570,11 @@ var c = file_a.a;
               `tslib_1.__exportStar(b_module, exports);\n` +
               `tslib_1.__exportStar(require("./xtra_module"), exports);\n`,
         },
+        {
+          name: _('/define_property_reexports.js'),
+          contents: `var moduleA = require("./a_module");\n` +
+              `Object.defineProperty(exports, "newA", { enumerable: true, get: function () { return moduleA.a; } });`,
+        }
       ];
 
       FUNCTION_BODY_FILE = {
@@ -2180,6 +2185,22 @@ exports.MissingClass2 = MissingClass2;
                 ['__unknownHelper', null],
               ]);
         });
+
+        it('should define property exports from a module', () => {
+          loadFakeCore(getFileSystem());
+          loadTestFiles(EXPORTS_FILES);
+          const bundle = makeTestBundleProgram(_('/index.js'));
+          const host =
+              createHost(bundle, new CommonJsReflectionHost(new MockLogger(), false, bundle));
+          const file = getSourceFileOrError(bundle.program, _('/define_property_reexports.js'));
+          const exportDeclarations = host.getExportsOfModule(file);
+          expect(exportDeclarations).not.toBe(null);
+          expect(Array.from(exportDeclarations!.entries())
+                     .map(entry => [entry[0], entry[1].node!.getText(), entry[1].viaModule]))
+              .toEqual([
+                ['newA', `a = 'a'`, null],
+              ]);
+        });
       });
 
       describe('getClassSymbol()', () => {

packages/compiler-cli/ngcc/test/host/umd_host_spec.ts

@@ -571,10 +571,10 @@ runInEachFileSystem(() => {
         {
           name: _('/index.js'),
           contents: `(function (global, factory) {\n` +
-              `  typeof exports === 'object' && typeof module !== 'undefined' ? factory(exports, require('./a_module'), require('./b_module'), require('./wildcard_reexports'), require('./wildcard_reexports_imported_helpers'), require('./wildcard_reexports_with_require')) :\n` +
+              `  typeof exports === 'object' && typeof module !== 'undefined' ? factory(exports, require('./a_module'), require('./b_module'), require('./wildcard_reexports'), require('./wildcard_reexports_imported_helpers'), require('./wildcard_reexports_with_require'), require('./define_property_reexports')) :\n` +
-              `  typeof define === 'function' && define.amd ? define('index', ['exports', './a_module', './b_module', './wildcard_reexports', './wildcard_reexports_imported_helpers', './wildcard_reexports_with_require'], factory) :\n` +
+              `  typeof define === 'function' && define.amd ? define('index', ['exports', './a_module', './b_module', './wildcard_reexports', './wildcard_reexports_imported_helpers', './wildcard_reexports_with_require', './define_property_reexports'], factory) :\n` +
-              `  (factory(global.index, global.a_module, global.b_module, global.wildcard_reexports, global.wildcard_reexports_imported_helpers, global.wildcard_reexports_with_require));\n` +
+              `  (factory(global.index, global.a_module, global.b_module, global.wildcard_reexports, global.wildcard_reexports_imported_helpers, global.wildcard_reexports_with_require, global.define_property_reexports));\n` +
-              `}(this, (function (exports, a_module, b_module, wildcard_reexports, wildcard_reexports_imported_helpers, wildcard_reexports_with_require) { 'use strict';\n` +
+              `}(this, (function (exports, a_module, b_module, wildcard_reexports, wildcard_reexports_imported_helpers, wildcard_reexports_with_require, define_property_reexports) { 'use strict';\n` +
               `})));\n`
         },
         {
@@ -663,7 +663,17 @@ runInEachFileSystem(() => {
               `  __export(b_module);\n` +
               `  __export(require('./xtra_module'));\n` +
               `})));\n`,
-        }
+        },
+        {
+          name: _('/define_property_reexports.js'),
+          contents: `(function (global, factory) {\n` +
+              `  typeof exports === 'object' && typeof module !== 'undefined' ? factory(require, exports) :\n` +
+              `  typeof define === 'function' && define.amd ? define('define_property_reexports', ['require', 'exports'], factory);\n` +
+              `}(this, (function (require, exports) { 'use strict';\n` +
+              `var moduleA = require("./a_module");\n` +
+              `Object.defineProperty(exports, "newA", { enumerable: true, get: function () { return moduleA.a; } });\n` +
+              `})));`,
+        },
       ];
 
       FUNCTION_BODY_FILE = {
@@ -2285,6 +2295,21 @@ runInEachFileSystem(() => {
               ['__unknownHelper', null],
             ]);
       });
+
+      it('should define property exports from a module', () => {
+        loadFakeCore(getFileSystem());
+        loadTestFiles(EXPORTS_FILES);
+        const bundle = makeTestBundleProgram(_('/index.js'));
+        const host = createHost(bundle, new UmdReflectionHost(new MockLogger(), false, bundle));
+        const file = getSourceFileOrError(bundle.program, _('/define_property_reexports.js'));
+        const exportDeclarations = host.getExportsOfModule(file);
+        expect(exportDeclarations).not.toBe(null);
+        expect(Array.from(exportDeclarations!.entries())
+                   .map(entry => [entry[0], entry[1].node!.getText(), entry[1].viaModule]))
+            .toEqual([
+              ['newA', `a = 'a'`, null],
+            ]);
+      });
     });
 
     describe('getClassSymbol()', () => {