fix(ngcc): support element accesses for export declarations (#44669)

Bundlers like Rollup may use an element access expression for an export
declaration, which causes ngcc to ignore those export declarations possibly
resulting in incomplete processing of packages.

Element access syntax may be used when the declared name is not considered
as valid JS identifier, but bundlers may be conservative in determining whether
an identifier can be used (to emit a property access) and opt for a string
literal in an element access instead.

The element access syntax introduces a problem for ngcc, where it wouldn't
consider such export as class declaration, causing them to be skipped. The
ngtsc compiler is implemented with the assumption that all class declarations
use a `ts.Identifier` as name, whereas the element access is using a string
literal for the declared name. This makes it troublesome for ngcc to support
this syntax form in UMD bundles.

To work around the problem, this function transforms these access expressions
into regular property accesses. The source text is parsed to an AST to allow
finding the element accesses in a robust way, after which the affected text
ranges are replaced with property accesses in the original source text.

Closes #44037

PR Close #44669

packages/compiler-cli/ngcc/src/packages/entry_point_bundle.ts

@@ -6,9 +6,12 @@
  * found in the LICENSE file at https://angular.io/license
  */
 import ts from 'typescript';
+
 import {AbsoluteFsPath, FileSystem, ReadonlyFileSystem} from '../../../src/ngtsc/file_system';
 import {DtsProcessing} from '../execution/tasks/api';
 import {PathMappings} from '../path_mappings';
+
+import {adjustElementAccessExports} from './adjust_cjs_umd_exports';
 import {BundleProgram, makeBundleProgram} from './bundle_program';
 import {EntryPoint, EntryPointFormat} from './entry_point';
 import {NgccDtsCompilerHost, NgccSourcesCompilerHost} from './ngcc_compiler_host';
@@ -30,6 +33,20 @@ export interface EntryPointBundle {
   enableI18nLegacyMessageIdFormat: boolean;
 }
 
+/**
+ * When processing UMD or CommonJS bundles the source text is preprocessed to transform export
+ * declarations using element access expressions into property access expressions, as otherwise ngcc
+ * would not recognize these export declarations. See `adjustElementAccessExports` for more
+ * information.
+ */
+function createSourceTextProcessor(format: EntryPointFormat): (sourceText: string) => string {
+  if (format === 'umd' || format === 'commonjs') {
+    return adjustElementAccessExports;
+  } else {
+    return sourceText => sourceText;
+  }
+}
+
 /**
  * Get an object that describes a formatted bundle for an entry-point.
  * @param fs The current file-system being used.
@@ -56,7 +73,8 @@ export function makeEntryPointBundle(
   const rootDir = entryPoint.packagePath;
   const options: ts
       .CompilerOptions = {allowJs: true, maxNodeModuleJsDepth: Infinity, rootDir, ...pathMappings};
-  const entryPointCache = new EntryPointFileCache(fs, sharedFileCache);
+  const processSourceText = createSourceTextProcessor(format);
+  const entryPointCache = new EntryPointFileCache(fs, sharedFileCache, processSourceText);
   const dtsHost = new NgccDtsCompilerHost(fs, options, entryPointCache, moduleResolutionCache);
   const srcHost = new NgccSourcesCompilerHost(
       fs, options, entryPointCache, moduleResolutionCache, entryPoint.packagePath);

packages/compiler-cli/ngcc/src/packages/source_file_cache.ts

@@ -6,8 +6,11 @@
  * found in the LICENSE file at https://angular.io/license
  */
 import ts from 'typescript';
+
 import {AbsoluteFsPath, ReadonlyFileSystem} from '../../../src/ngtsc/file_system';
 
+import {adjustElementAccessExports} from './adjust_cjs_umd_exports';
+
 /**
  * A cache that holds on to source files that can be shared for processing all entry-points in a
  * single invocation of ngcc. In particular, the following files are shared across all entry-points
@@ -147,7 +150,9 @@ function isFile(
 export class EntryPointFileCache {
   private readonly sfCache = new Map<AbsoluteFsPath, ts.SourceFile>();
 
-  constructor(private fs: ReadonlyFileSystem, private sharedFileCache: SharedFileCache) {}
+  constructor(
+      private fs: ReadonlyFileSystem, private sharedFileCache: SharedFileCache,
+      private processSourceText: (sourceText: string) => string) {}
 
   /**
    * Returns and caches a parsed `ts.SourceFile` for the provided `fileName`. If the `fileName` is
@@ -172,7 +177,8 @@ export class EntryPointFileCache {
     if (content === undefined) {
       return undefined;
     }
-    const sf = ts.createSourceFile(fileName, content, languageVersion);
+    const processed = this.processSourceText(content);
+    const sf = ts.createSourceFile(fileName, processed, languageVersion);
     this.sfCache.set(absPath, sf);
     return sf;
   }

packages/compiler-cli/ngcc/test/helpers/utils.ts

@@ -73,7 +73,8 @@ export function makeTestBundleProgram(
   const options: ts.CompilerOptions =
       {allowJs: true, maxNodeModuleJsDepth: Infinity, checkJs: false, rootDir, rootDirs: [rootDir]};
   const moduleResolutionCache = createModuleResolutionCache(fs);
-  const entryPointFileCache = new EntryPointFileCache(fs, new SharedFileCache(fs));
+  const entryPointFileCache =
+      new EntryPointFileCache(fs, new SharedFileCache(fs), sourceText => sourceText);
   const host =
       new NgccSourcesCompilerHost(fs, options, entryPointFileCache, moduleResolutionCache, rootDir);
   return makeBundleProgram(

packages/compiler-cli/ngcc/test/integration/ngcc_spec.ts

@@ -1399,6 +1399,69 @@ runInEachFileSystem(() => {
               'ɵngcc0.ɵɵsetNgModuleScope(FooModule, { declarations: function () { return [exports.FooDirective]; } });');
     });
 
+    it('should support inline UMD/CommonJS exports declarations using an element access as export',
+       () => {
+         // Setup an Angular entry-point in UMD module format that uses element access syntax for
+         // export declarations, as a bundler like Rollup may have generated.
+         loadTestFiles([
+           {
+             name: _('/node_modules/test-package/package.json'),
+             contents: '{"name": "test-package", "main": "./index.js", "typings": "./index.d.ts"}'
+           },
+           {
+             name: _('/node_modules/test-package/index.js'),
+             contents: `
+          (function (global, factory) {
+            typeof exports === 'object' && typeof module !== 'undefined' ? factory(exports, require('@angular/core')) :
+            typeof define === 'function' && define.amd ? define('test', ['exports', 'core'], factory) :
+            (factory(global.test, global.core));
+          }(this, (function (exports, core) { 'use strict';
+            exports['FooModule'] = /** @class */ (function () {
+              function FooModule() {}
+              FooModule = __decorate([
+                  core.NgModule({declarations: exports.declarations})
+              ], FooModule);
+              return FooModule;
+            }());
+
+            exports['declarations'] = [exports['FooDirective']];
+
+            exports['FooDirective'] = /** @class */ (function () {
+              function FooDirective() {}
+              FooDirective = __decorate([
+                core.Directive({selector: '[foo]'})
+              ], FooDirective);
+              return FooDirective;
+            }());
+          })));
+          `
+           },
+           {
+             name: _('/node_modules/test-package/index.d.ts'),
+             contents: `
+          export declare class FooModule { }
+          export declare class FooDirective { }
+          `
+           },
+           {name: _('/node_modules/test-package/index.metadata.json'), contents: 'DUMMY DATA'},
+         ]);
+
+         expect(() => mainNgcc({
+                  basePath: '/node_modules',
+                  targetEntryPointPath: 'test-package',
+                  propertiesToConsider: ['main'],
+                }))
+             .not.toThrow();
+
+         const processedFile = fs.readFile(_('/node_modules/test-package/index.js'));
+         expect(processedFile)
+             .toContain(
+                 'FooModule.ɵmod = /*@__PURE__*/ ɵngcc0.ɵɵdefineNgModule({ type: FooModule });');
+         expect(processedFile)
+             .toContain(
+                 'ɵngcc0.ɵɵsetNgModuleScope(FooModule, { declarations: function () { return [exports.FooDirective]; } });');
+       });
+
     it('should not be able to evaluate code in external packages when no .d.ts files are present',
        () => {
          loadTestFiles([

packages/compiler-cli/ngcc/test/packages/adjust_cjs_exports_spec.ts

@@ -0,0 +1,48 @@
+/**
+ * @license
+ * Copyright Google LLC All Rights Reserved.
+ *
+ * Use of this source code is governed by an MIT-style license that can be
+ * found in the LICENSE file at https://angular.io/license
+ */
+import {adjustElementAccessExports} from '../../src/packages/adjust_cjs_umd_exports';
+
+describe('adjustElementAccessExports', () => {
+  it('should replace exports using element access syntax', () => {
+    expectChanged(`exports['Foo']`, `exports. Foo  `);
+    expectChanged(
+        `exports['Foo']; exports['Bar']; exports['Baz'];`,
+        `exports. Foo  ; exports. Bar  ; exports. Baz  ;`);
+    expectChanged(
+        `function(exports) { exports['Foo'] = Foo; }`,
+        `function(exports) { exports. Foo   = Foo; }`);
+    expectChanged(
+        `function(exports) { exports['ɵFoo'] = Foo; }`,
+        `function(exports) { exports. ɵFoo   = Foo; }`);
+  });
+
+  it('should not replace invalid identifier names', () => {
+    expectUnchanged(`exports['']`);
+    expectUnchanged(`exports['let']`);
+    expectUnchanged(`exports['var']`);
+    expectUnchanged(`exports['const']`);
+  });
+
+  it('should not replace export syntax in comments and strings', () => {
+    expectUnchanged(`/* exports['Foo'] = Foo; */`);
+    expectUnchanged(`"exports['Foo'] = Foo;"`);
+    expectChanged(
+        `/* exports['Foo'] = Foo; */ exports['Foo'] = Foo;`,
+        `/* exports['Foo'] = Foo; */ exports. Foo   = Foo;`);
+  });
+});
+
+function expectChanged(input: string, expected: string) {
+  const result = adjustElementAccessExports(input);
+  expect(result).toEqual(expected);
+}
+
+function expectUnchanged(input: string) {
+  const result = adjustElementAccessExports(input);
+  expect(result).toEqual(input);
+}

packages/compiler-cli/ngcc/test/packages/source_file_cache_spec.ts

@@ -167,21 +167,21 @@ runInEachFileSystem(() => {
       });
 
       it('should prefer source files cached in SharedFileCache', () => {
-        const cache1 = new EntryPointFileCache(fs, sharedFileCache);
+        const cache1 = new EntryPointFileCache(fs, sharedFileCache, sourceText => sourceText);
         const libEs5_1 = cache1.getCachedSourceFile(
             '/node_modules/typescript/lib/lib.es5.d.ts', ts.ScriptTarget.ESNext)!;
         expect(libEs5_1).not.toBeUndefined();
         expect(libEs5_1.text).toContain('Array');
         expect(libEs5_1.languageVersion).toBe(ts.ScriptTarget.ES2015);
 
-        const cache2 = new EntryPointFileCache(fs, sharedFileCache);
+        const cache2 = new EntryPointFileCache(fs, sharedFileCache, sourceText => sourceText);
         const libEs5_2 = cache2.getCachedSourceFile(
             '/node_modules/typescript/lib/lib.es5.d.ts', ts.ScriptTarget.ESNext)!;
         expect(libEs5_1).toBe(libEs5_2);
       });
 
       it('should cache source files that are not default library files', () => {
-        const cache = new EntryPointFileCache(fs, sharedFileCache);
+        const cache = new EntryPointFileCache(fs, sharedFileCache, sourceText => sourceText);
         const index = cache.getCachedSourceFile('/index.ts', ts.ScriptTarget.ESNext)!;
         expect(index).not.toBeUndefined();
         expect(index.text).toContain('index');
@@ -200,8 +200,8 @@ runInEachFileSystem(() => {
       });
 
       it('should not share non-library files across multiple cache instances', () => {
-        const cache1 = new EntryPointFileCache(fs, sharedFileCache);
-        const cache2 = new EntryPointFileCache(fs, sharedFileCache);
+        const cache1 = new EntryPointFileCache(fs, sharedFileCache, sourceText => sourceText);
+        const cache2 = new EntryPointFileCache(fs, sharedFileCache, sourceText => sourceText);
 
         const index1 = cache1.getCachedSourceFile('/index.ts', ts.ScriptTarget.ESNext)!;
         const index2 = cache2.getCachedSourceFile('/index.ts', ts.ScriptTarget.ESNext)!;
@@ -209,15 +209,23 @@ runInEachFileSystem(() => {
       });
 
       it('should return undefined if the file does not exist', () => {
-        const cache = new EntryPointFileCache(fs, sharedFileCache);
+        const cache = new EntryPointFileCache(fs, sharedFileCache, sourceText => sourceText);
         expect(cache.getCachedSourceFile('/nonexistent.ts', ts.ScriptTarget.ESNext))
             .toBeUndefined();
       });
 
       it('should return undefined if the path is a directory', () => {
-        const cache = new EntryPointFileCache(fs, sharedFileCache);
+        const cache = new EntryPointFileCache(fs, sharedFileCache, sourceText => sourceText);
         expect(cache.getCachedSourceFile('/node_modules', ts.ScriptTarget.ESNext)).toBeUndefined();
       });
+
+      it('should apply the source text transform', () => {
+        const cache = new EntryPointFileCache(
+            fs, sharedFileCache, sourceText => sourceText.replace('main', 'replaced'));
+        const main = cache.getCachedSourceFile('/main.ts', ts.ScriptTarget.ESNext)!;
+        expect(main).not.toBeUndefined();
+        expect(main.text).toBe(`export const replaced = true;`);
+      });
     });
   });
 });