From 27a4adebcbfea1ea05ebc1cf3762a4fe2822dd29 Mon Sep 17 00:00:00 2001
From: Pete Bacon Darwin <pete@bacondarwin.com>
Date: Thu, 22 Oct 2020 10:34:52 +0100
Subject: [PATCH] refactor(compiler-cli): support namespaced references
 (#39346)

The compiler uses a `Reference` abstraction to refer to TS nodes
that it needs to refer to from other parts of the source. Such
references keep track of any identifiers that represent the referenced
node.

Prior to this commit, the compiler (and specifically `ReferenceEmitter`
classes) assumed that the reference identifiers are always free standing.
In other words a reference identifier would be an expression like
`FooDirective` in the expression `class FooDirective {}`.

But in UMD/CommonJS source, a reference can actually refer to an "exports"
declaration of the form `exports.FooDirective = ...`.
In such cases the `FooDirective` identifier is not free-standing
since it is part of a property access, so the `ReferenceEmitter`
should take this into account when emitting an expression that
refers to such a `Reference`.

This commit changes the `LocalIdentifierStrategy` reference emitter
so that if the `node` being referenced is not a declaration itself and
is in the current file, then it should be used directly, rather than
trying to use one of its identifiers.

PR Close #39346
---
 .../compiler-cli/src/ngtsc/imports/src/emitter.ts  | 14 ++++++++++++--
 1 file changed, 12 insertions(+), 2 deletions(-)

diff --git a/packages/compiler-cli/src/ngtsc/imports/src/emitter.ts b/packages/compiler-cli/src/ngtsc/imports/src/emitter.ts
index 4835935d0c..aa77bb1884 100644
--- a/packages/compiler-cli/src/ngtsc/imports/src/emitter.ts
+++ b/packages/compiler-cli/src/ngtsc/imports/src/emitter.ts
@@ -104,13 +104,23 @@ export class ReferenceEmitter {
  */
 export class LocalIdentifierStrategy implements ReferenceEmitStrategy {
   emit(ref: Reference<ts.Node>, context: ts.SourceFile, importFlags: ImportFlags): Expression|null {
+    const refSf = getSourceFile(ref.node);
+
     // If the emitter has specified ForceNewImport, then LocalIdentifierStrategy should not use a
     // local identifier at all, *except* in the source file where the node is actually declared.
-    if (importFlags & ImportFlags.ForceNewImport &&
-        getSourceFile(ref.node) !== getSourceFile(context)) {
+    if (importFlags & ImportFlags.ForceNewImport && refSf !== context) {
       return null;
     }
 
+    // If referenced node is not an actual TS declaration (e.g. `class Foo` or `function foo() {}`,
+    // etc) and it is in the current file then just use it directly.
+    // This is important because the reference could be a property access (e.g. `exports.foo`). In
+    // such a case, the reference's `identities` property would be `[foo]`, which would result in an
+    // invalid emission of a free-standing `foo` identifier, rather than `exports.foo`.
+    if (!isDeclaration(ref.node) && refSf === context) {
+      return new WrappedNodeExpr(ref.node);
+    }
+
     // A Reference can have multiple identities in different files, so it may already have an
     // Identifier in the requested context file.
     const identifier = ref.getIdentityIn(context);