From 0d8deb0795c6fca583508815f3bb0d0f51399edd Mon Sep 17 00:00:00 2001
From: Alex Rickabaugh <alx+alxhub@alxandria.net>
Date: Fri, 2 Mar 2018 15:02:06 -0800
Subject: [PATCH] fix(compiler-cli): generate proper exports.* identifiers in
 cjs output (#22564)

When the compiler generates a reference to an exported variable in the
same file, it inserts a synthetic ts.Identifier node. In CommonJS
output, this synthetic node would not be properly rewritten with an
`exports.` prefix.

This change sets the TS original node property on the synthetic node
we generate, which ensures TS knows to rewrite it in CommonJS output.

PR Close #22564
---
 .../src/transformers/node_emitter.ts          | 45 ++++++++++++++++---
 packages/compiler-cli/test/ngc_spec.ts        | 25 +++++++++++
 2 files changed, 65 insertions(+), 5 deletions(-)

diff --git a/packages/compiler-cli/src/transformers/node_emitter.ts b/packages/compiler-cli/src/transformers/node_emitter.ts
index 5f9408750c..03503accc0 100644
--- a/packages/compiler-cli/src/transformers/node_emitter.ts
+++ b/packages/compiler-cli/src/transformers/node_emitter.ts
@@ -63,6 +63,8 @@ export function updateSourceFile(
     sourceFile: ts.SourceFile, module: PartialModule,
     context: ts.TransformationContext): [ts.SourceFile, Map<ts.Node, Node>] {
   const converter = new _NodeEmitterVisitor();
+  converter.loadExportedVariableIdentifiers(sourceFile);
+
   const prefixStatements = module.statements.filter(statement => !(statement instanceof ClassStmt));
   const classes =
       module.statements.filter(statement => statement instanceof ClassStmt) as ClassStmt[];
@@ -158,6 +160,11 @@ function createLiteral(value: any) {
   }
 }
 
+function isExportTypeStatement(statement: ts.Statement): boolean {
+  return !!statement.modifiers &&
+      statement.modifiers.some(mod => mod.kind === ts.SyntaxKind.ExportKeyword);
+}
+
 /**
  * Visits an output ast and produces the corresponding TypeScript synthetic nodes.
  */
@@ -166,6 +173,26 @@ class _NodeEmitterVisitor implements StatementVisitor, ExpressionVisitor {
   private _importsWithPrefixes = new Map<string, string>();
   private _reexports = new Map<string, {name: string, as: string}[]>();
   private _templateSources = new Map<ParseSourceFile, ts.SourceMapSource>();
+  private _exportedVariableIdentifiers = new Map<string, ts.Identifier>();
+
+  /**
+   * Process the source file and collect exported identifiers that refer to variables.
+   *
+   * Only variables are collected because exported classes still exist in the module scope in
+   * CommonJS, whereas variables have their declarations moved onto the `exports` object, and all
+   * references are updated accordingly.
+   */
+  loadExportedVariableIdentifiers(sourceFile: ts.SourceFile): void {
+    sourceFile.statements.forEach(statement => {
+      if (ts.isVariableStatement(statement) && isExportTypeStatement(statement)) {
+        statement.declarationList.declarations.forEach(declaration => {
+          if (ts.isIdentifier(declaration.name)) {
+            this._exportedVariableIdentifiers.set(declaration.name.text, declaration.name);
+          }
+        });
+      }
+    });
+  }
 
   getReexports(): ts.Statement[] {
     return Array.from(this._reexports.entries())
@@ -612,7 +639,8 @@ class _NodeEmitterVisitor implements StatementVisitor, ExpressionVisitor {
   }
 
   private _visitIdentifier(value: ExternalReference): ts.Expression {
-    const {name, moduleName} = value;
+    // name can only be null during JIT which never executes this code.
+    const moduleName = value.moduleName, name = value.name !;
     let prefixIdent: ts.Identifier|null = null;
     if (moduleName) {
       let prefix = this._importsWithPrefixes.get(moduleName);
@@ -622,10 +650,17 @@ class _NodeEmitterVisitor implements StatementVisitor, ExpressionVisitor {
       }
       prefixIdent = ts.createIdentifier(prefix);
     }
-    // name can only be null during JIT which never executes this code.
-    let result: ts.Expression =
-        prefixIdent ? ts.createPropertyAccess(prefixIdent, name !) : ts.createIdentifier(name !);
-    return result;
+    if (prefixIdent) {
+      return ts.createPropertyAccess(prefixIdent, name);
+    } else {
+      const id = ts.createIdentifier(name);
+      if (this._exportedVariableIdentifiers.has(name)) {
+        // In order for this new identifier node to be properly rewritten in CommonJS output,
+        // it must have its original node set to a parsed instance of the same identifier.
+        ts.setOriginalNode(id, this._exportedVariableIdentifiers.get(name));
+      }
+      return id;
+    }
   }
 }
 
diff --git a/packages/compiler-cli/test/ngc_spec.ts b/packages/compiler-cli/test/ngc_spec.ts
index 0f059a49e2..d70889ae94 100644
--- a/packages/compiler-cli/test/ngc_spec.ts
+++ b/packages/compiler-cli/test/ngc_spec.ts
@@ -2158,5 +2158,30 @@ describe('ngc transformer command-line', () => {
       expect(source).toMatch(/ngInjectableDef.*token: Service/);
       expect(source).toMatch(/ngInjectableDef.*scope: .+\.Module/);
     });
+
+    it('generates exports.* references when outputting commonjs', () => {
+      writeConfig(`{
+        "extends": "./tsconfig-base.json",
+        "compilerOptions": {
+          "module": "commonjs"
+        },
+        "files": ["service.ts"]
+      }`);
+      const source = compileService(`
+        import {Inject, Injectable, InjectionToken, APP_ROOT_SCOPE} from '@angular/core';
+        import {Module} from './module';
+
+        export const TOKEN = new InjectionToken<string>('test token', {
+          scope: APP_ROOT_SCOPE,
+          factory: () => 'this is a test',
+        });
+
+        @Injectable({scope: APP_ROOT_SCOPE})
+        export class Service {
+          constructor(@Inject(TOKEN) token: any) {}
+        }
+      `);
+      expect(source).toMatch(/new Service\(i0\.inject\(exports\.TOKEN\)\);/);
+    });
   });
 });