refactor: remove remaining dynamic require usages in package output (#43431)

Removes the remaining usages of dynamic require statements in the
package output. Since we declare all shipped packages as strict ESM,
we cannot use dynamic require statements anymore. This commit switches
these usages to actual `import` statements.

Note: Tsickle continues to remain an optional dependency since bundling
does not work with its UMD package output. Also tsickle is rarely used by
consumers, if at all, so bundling does not really provide any significant
value. To continue keeping tsickle optional (since it's still needed by the
`annotateForClosureCompiler` option which is also respected in ngtsc), we
pass-through a tsickle instance as a parameter to `main`. This allows us to
keep the compile functions synchronous without having to refactor the majority
of the watch compilation code, and majority of tests for ngc, ngtsc.

Consumers (like the `ngc` bin entry-point) can then load tsickle based on their
module format. e.g. tsickle can be imported through `require` to keep everything
sync, but in ESM, the dynamic import can be used beforehand to pass `tsickle` to
the `main` function. We can revisit this in the future but for now this does the
trick without exceeding the scope of this commit..

PR Close #43431

packages/compiler-cli/BUILD.bazel

@@ -41,6 +41,7 @@ esbuild(
     external = [
         "@angular/compiler",
         "typescript",
+        "tsickle",
         "@babel/core",
         "reflect-metadata",
         "minimist",
@@ -96,6 +97,7 @@ ts_library(
         "//packages/compiler-cli/src/ngtsc/typecheck",
         "//packages/compiler-cli/src/ngtsc/typecheck/api",
         "@npm//@bazel/typescript",
+        "@npm//@types/minimist",
         "@npm//@types/node",
         "@npm//chokidar",
         "@npm//minimist",

packages/compiler-cli/ngcc/src/dependencies/dependency_resolver.ts

@@ -7,6 +7,7 @@
  */
 
 import {DepGraph} from 'dependency-graph';
+import module from 'module';
 
 import {AbsoluteFsPath, ReadonlyFileSystem} from '../../../src/ngtsc/file_system';
 import {Logger} from '../../../src/ngtsc/logging';
@@ -16,7 +17,7 @@ import {PartiallyOrderedList} from '../utils';
 
 import {createDependencyInfo, DependencyHost, EntryPointWithDependencies} from './dependency_host';
 
-const builtinNodeJsModules = new Set<string>(require('module').builtinModules);
+const builtinNodeJsModules = new Set<string>(module.builtinModules);
 
 /**
  * Holds information about entry points that are removed because

packages/compiler-cli/src/bin/ngc.ts

@@ -13,8 +13,26 @@ import 'reflect-metadata';
 import {NodeJSFileSystem, setFileSystem} from '../ngtsc/file_system';
 import {main} from '../main';
 
-process.title = 'Angular Compiler (ngc)';
-const args = process.argv.slice(2);
-// We are running the real compiler so run against the real file-system
-setFileSystem(new NodeJSFileSystem());
-process.exitCode = main(args);
+async function runNgcComamnd() {
+  process.title = 'Angular Compiler (ngc)';
+  const args = process.argv.slice(2);
+  // We are running the real compiler so run against the real file-system
+  setFileSystem(new NodeJSFileSystem());
+
+  let tsickleModule: typeof import('tsickle')|undefined;
+
+  // Load tsickle if it's available. We load it here because tsickle
+  // is not needed in all Angular projects directly using `ngc`.
+  try {
+    tsickleModule = (await import('tsickle')).default;
+  } catch {
+  }
+
+  process.exitCode =
+      main(args, undefined, undefined, undefined, undefined, undefined, tsickleModule);
+}
+
+runNgcComamnd().catch(e => {
+  console.error(e);
+  process.exitCode = 1;
+});

packages/compiler-cli/src/extract_i18n.ts

@@ -10,9 +10,11 @@
  * Extract i18n messages from source code
  */
 
-import * as api from './transformers/api';
-import {ParsedConfiguration} from './perform_compile';
+import minimist from 'minimist';
+
 import {main, readCommandLineAndConfiguration} from './main';
+import {ParsedConfiguration} from './perform_compile';
+import * as api from './transformers/api';
 
 export function mainXi18n(
     args: string[], consoleError: (msg: string) => void = console.error): number {
@@ -22,7 +24,7 @@ export function mainXi18n(
 
 function readXi18nCommandLineAndConfiguration(args: string[]): ParsedConfiguration {
   const options: api.CompilerOptions = {};
-  const parsedArgs = require('minimist')(args);
+  const parsedArgs = minimist(args);
   if (parsedArgs.outFile) options.i18nOutFile = parsedArgs.outFile;
   if (parsedArgs.i18nFormat) options.i18nOutFormat = parsedArgs.i18nFormat;
   if (parsedArgs.locale) options.i18nOutLocale = parsedArgs.locale;

packages/compiler-cli/src/main.ts

@@ -6,20 +6,23 @@
  * found in the LICENSE file at https://angular.io/license
  */
 
-import * as tsickle from 'tsickle';
+import minimist from 'minimist';
 import ts from 'typescript';
 
+import type {TsickleHost} from 'tsickle';
 import {Diagnostics, exitCodeFromResult, filterErrorsAndWarnings, formatDiagnostics, ParsedConfiguration, performCompilation, readConfiguration} from './perform_compile';
 import {createPerformWatchHost, performWatchCompilation} from './perform_watch';
 import * as api from './transformers/api';
 import {GENERATED_FILES} from './transformers/util';
 
+type TsickleModule = typeof import('tsickle');
+
 export function main(
     args: string[], consoleError: (s: string) => void = console.error,
     config?: NgcParsedConfiguration, customTransformers?: api.CustomTransformers, programReuse?: {
       program: api.Program|undefined,
     },
-    modifiedResourceFiles?: Set<string>|null): number {
+    modifiedResourceFiles?: Set<string>|null, tsickle?: TsickleModule): number {
   let {project, rootNames, options, errors: configErrors, watch, emitFlags} =
       config || readNgcCommandLineAndConfiguration(args);
   if (configErrors.length) {
@@ -40,7 +43,7 @@ export function main(
     options,
     emitFlags,
     oldProgram,
-    emitCallback: createEmitCallback(options),
+    emitCallback: createEmitCallback(options, tsickle),
     customTransformers,
     modifiedResourceFiles
   });
@@ -52,8 +55,8 @@ export function main(
 
 export function mainDiagnosticsForTest(
     args: string[], config?: NgcParsedConfiguration,
-    programReuse?: {program: api.Program|undefined},
-    modifiedResourceFiles?: Set<string>|null): ReadonlyArray<ts.Diagnostic|api.Diagnostic> {
+    programReuse?: {program: api.Program|undefined}, modifiedResourceFiles?: Set<string>|null,
+    tsickle?: TsickleModule): ReadonlyArray<ts.Diagnostic|api.Diagnostic> {
   let {project, rootNames, options, errors: configErrors, watch, emitFlags} =
       config || readNgcCommandLineAndConfiguration(args);
   if (configErrors.length) {
@@ -71,7 +74,7 @@ export function mainDiagnosticsForTest(
     emitFlags,
     oldProgram,
     modifiedResourceFiles,
-    emitCallback: createEmitCallback(options),
+    emitCallback: createEmitCallback(options, tsickle),
   });
 
   if (programReuse !== undefined) {
@@ -81,12 +84,16 @@ export function mainDiagnosticsForTest(
   return compileDiags;
 }
 
-function createEmitCallback(options: api.CompilerOptions): api.TsEmitCallback|undefined {
+function createEmitCallback(
+    options: api.CompilerOptions, tsickle?: TsickleModule): api.TsEmitCallback|undefined {
   if (!options.annotateForClosureCompiler) {
     return undefined;
   }
+  if (tsickle == undefined) {
+    throw Error('Tsickle is not provided but `annotateForClosureCompiler` is enabled.')
+  }
   const tsickleHost: Pick<
-      tsickle.TsickleHost,
+      TsickleHost,
       'shouldSkipTsickleProcessing'|'pathToModuleName'|'shouldIgnoreWarningsForPath'|
       'fileNameToModuleId'|'googmodule'|'untyped'|'convertIndexImportShorthand'|
       'transformDecorators'|'transformTypesToClosure'> = {
@@ -115,13 +122,12 @@ function createEmitCallback(options: api.CompilerOptions): api.TsEmitCallback|un
            host,
            options
          }) =>
-             // tslint:disable-next-line:no-require-imports only depend on tsickle if requested
-      require('tsickle').emitWithTsickle(
-          program, {...tsickleHost, options, host, moduleResolutionHost: host}, host, options,
-          targetSourceFile, writeFile, cancellationToken, emitOnlyDtsFiles, {
-            beforeTs: customTransformers.before,
-            afterTs: customTransformers.after,
-          });
+             tsickle.emitWithTsickle(
+                 program, {...tsickleHost, options, moduleResolutionHost: host}, host, options,
+                 targetSourceFile, writeFile, cancellationToken, emitOnlyDtsFiles, {
+                   beforeTs: customTransformers.before,
+                   afterTs: customTransformers.after,
+                 });
 }
 
 export interface NgcParsedConfiguration extends ParsedConfiguration {
@@ -130,7 +136,7 @@ export interface NgcParsedConfiguration extends ParsedConfiguration {
 
 export function readNgcCommandLineAndConfiguration(args: string[]): NgcParsedConfiguration {
   const options: api.CompilerOptions = {};
-  const parsedArgs = require('minimist')(args);
+  const parsedArgs = minimist(args);
   if (parsedArgs.i18nFile) options.i18nInFile = parsedArgs.i18nFile;
   if (parsedArgs.i18nFormat) options.i18nInFormat = parsedArgs.i18nFormat;
   if (parsedArgs.locale) options.i18nInLocale = parsedArgs.locale;

packages/compiler-cli/src/transformers/compiler_host.ts

@@ -7,6 +7,7 @@
  */
 
 import {AotCompilerHost, collectExternalReferences, EmitterVisitorContext, GeneratedFile, ParseSourceSpan, syntaxError, TypeScriptEmitter} from '@angular/compiler';
+import fs from 'fs';
 import * as path from 'path';
 import ts from 'typescript';
 
@@ -253,7 +254,8 @@ export class TsCompilerAotCompilerTypeCheckHostAdapter implements ts.CompilerHos
         try {
           const modulePath = importedFile.substring(0, importedFile.length - moduleName.length) +
               importedFilePackageName;
-          const packageJson = require(modulePath + '/package.json');
+          const packageJson =
+              JSON.parse(fs.readFileSync(modulePath + '/package.json', 'utf8')) as any;
           const packageTypings = join(modulePath, packageJson.typings);
           if (packageTypings === originalImportedFile) {
             moduleName = importedFilePackageName;

packages/compiler-cli/test/BUILD.bazel

@@ -68,6 +68,7 @@ ts_library(
         ":test_utils",
         "//packages/compiler",
         "//packages/compiler-cli",
+        "@npm//tsickle",
         "@npm//typescript",
     ],
 )
@@ -92,7 +93,6 @@ jasmine_node_test(
         "//packages/core",
         "@npm//minimist",
         "@npm//rxjs",
-        "@npm//tsickle",
     ],
 )
 

packages/compiler-cli/test/ngc_spec.ts

@@ -8,9 +8,11 @@
 
 import * as fs from 'fs';
 import * as path from 'path';
+import * as tsickle from 'tsickle';
 import ts from 'typescript';
 
 import {main, mainDiagnosticsForTest, readCommandLineAndConfiguration, watchMode} from '../src/main';
+
 import {setup, stripAnsi} from './test_support';
 
 describe('ngc transformer command-line', () => {
@@ -676,7 +678,8 @@ describe('ngc transformer command-line', () => {
         export class MyModule {}
       `);
 
-        const exitCode = main(['-p', basePath], errorSpy);
+        const exitCode =
+            main(['-p', basePath], errorSpy, undefined, undefined, undefined, undefined, tsickle);
         expect(exitCode).toEqual(0);
 
         const mymodulejs = path.resolve(outDir, 'mymodule.js');
@@ -708,7 +711,8 @@ describe('ngc transformer command-line', () => {
         export class MyModule {}
       `);
 
-        const exitCode = main(['-p', basePath], errorSpy);
+        const exitCode =
+            main(['-p', basePath], errorSpy, undefined, undefined, undefined, undefined, tsickle);
         expect(exitCode).toEqual(0);
 
         const mymodulejs = path.resolve(outDir, 'mymodule.js');
@@ -749,7 +753,8 @@ describe('ngc transformer command-line', () => {
         export class MyModule {}
     `);
 
-      const exitCode = main(['-p', basePath], errorSpy);
+      const exitCode =
+          main(['-p', basePath], errorSpy, undefined, undefined, undefined, undefined, tsickle);
       expect(exitCode).toEqual(0);
       const mymodulejs = path.resolve(outDir, 'mymodule.js');
       const mymoduleSource = fs.readFileSync(mymodulejs, 'utf8');
@@ -2118,10 +2123,12 @@ describe('ngc transformer command-line', () => {
   });
 
   describe('tree shakeable services', () => {
-    function compileService(source: string): string {
+    function compileService(source: string, withTsickle = false): string {
       write('service.ts', source);
 
-      const exitCode = main(['-p', path.join(basePath, 'tsconfig.json')], errorSpy);
+      const exitCode = main(
+          ['-p', path.join(basePath, 'tsconfig.json')], errorSpy, undefined, undefined, undefined,
+          undefined, withTsickle ? tsickle : undefined);
       expect(exitCode).toEqual(0);
 
       const servicePath = path.resolve(outDir, 'service.js');
@@ -2203,7 +2210,7 @@ describe('ngc transformer command-line', () => {
         },
         "files": ["service.ts"]
       }`);
-      const source = compileService(`
+      const input = `
         import {Injectable} from '@angular/core';
         import {Module} from './module';
 
@@ -2211,7 +2218,9 @@ describe('ngc transformer command-line', () => {
           providedIn: Module,
         })
         export class Service {}
-      `);
+      `;
+
+      const source = compileService(input, /* withTsickle */ true);
       expect(source).toMatch(/\/\*\* @nocollapse \*\/ Service\.ɵprov =/);
     });
 

packages/compiler-cli/test/ngtsc/BUILD.bazel

@@ -16,6 +16,7 @@ ts_library(
         "//packages/compiler-cli/src/ngtsc/util",
         "//packages/compiler-cli/test:test_utils",
         "@npm//source-map",
+        "@npm//tsickle",
         "@npm//typescript",
     ],
 )

packages/compiler-cli/test/ngtsc/env.ts

@@ -8,6 +8,7 @@
 
 import {CustomTransformers, defaultGatherDiagnostics, Program} from '@angular/compiler-cli';
 import * as api from '@angular/compiler-cli/src/transformers/api';
+import * as tsickle from 'tsickle';
 import ts from 'typescript';
 
 import {createCompilerHost, createProgram} from '../../index';
@@ -21,7 +22,6 @@ import {DeclarationNode} from '../../src/ngtsc/reflection';
 import {NgtscTestCompilerHost} from '../../src/ngtsc/testing';
 import {setWrapHostForTest} from '../../src/transformers/compiler_host';
 
-
 /**
  * Manages a temporary testing directory structure and environment for testing ngtsc by feeding it
  * TypeScript code.
@@ -220,7 +220,7 @@ export class NgtscTestEnvironment {
     }
     const exitCode = main(
         this.commandLineArgs, errorSpy, undefined, customTransformers, reuseProgram,
-        this.changedResources);
+        this.changedResources, tsickle);
     expect(errorSpy).not.toHaveBeenCalled();
     expect(exitCode).toBe(0);
     if (this.multiCompileHostExt !== null) {
@@ -241,7 +241,7 @@ export class NgtscTestEnvironment {
     }
 
     const diags = mainDiagnosticsForTest(
-        this.commandLineArgs, undefined, reuseProgram, this.changedResources);
+        this.commandLineArgs, undefined, reuseProgram, this.changedResources, tsickle);
 
 
     if (this.multiCompileHostExt !== null) {

packages/compiler/test/aot/compiler_spec.ts

@@ -56,11 +56,10 @@ describe('compiler (unbundled Angular)', () => {
           genFile => genFile.srcFileUrl === componentPath && genFile.genFileUrl.endsWith('.ts'))!;
     }
 
-    function findLineAndColumn(
-        file: string, token: string): {line: number|null, column: number|null} {
+    function findLineAndColumn(file: string, token: string): {line: number, column: number}|null {
       const index = file.indexOf(token);
       if (index === -1) {
-        return {line: null, column: null};
+        return null;
       }
       const linesUntilToken = file.slice(0, index).split('\n');
       const line = linesUntilToken.length;
@@ -149,7 +148,7 @@ describe('compiler (unbundled Angular)', () => {
         const genFile = compileApp();
         const genSource = toTypeScript(genFile);
         const sourceMap = extractSourceMap(genSource)!;
-        expect(originalPositionFor(sourceMap, findLineAndColumn(genSource, `'span'`)))
+        expect(originalPositionFor(sourceMap, findLineAndColumn(genSource, `'span'`)!))
             .toEqual({line: 2, column: 3, source: ngUrl});
       });
 
@@ -161,7 +160,7 @@ describe('compiler (unbundled Angular)', () => {
         const genFile = compileApp();
         const genSource = toTypeScript(genFile);
         const sourceMap = extractSourceMap(genSource)!;
-        expect(originalPositionFor(sourceMap, findLineAndColumn(genSource, `someMethod()`)))
+        expect(originalPositionFor(sourceMap, findLineAndColumn(genSource, `someMethod()`)!))
             .toEqual({line: 2, column: 9, source: ngUrl});
       });
 
@@ -173,7 +172,7 @@ describe('compiler (unbundled Angular)', () => {
         const genFile = compileApp();
         const genSource = toTypeScript(genFile);
         const sourceMap = extractSourceMap(genSource)!;
-        expect(originalPositionFor(sourceMap, findLineAndColumn(genSource, `someMethod()`)))
+        expect(originalPositionFor(sourceMap, findLineAndColumn(genSource, `someMethod()`)!))
             .toEqual({line: 2, column: 9, source: ngUrl});
       });
 

packages/compiler/testing/BUILD.bazel

@@ -17,5 +17,7 @@ ng_module(
         "//packages/compiler",
         "//packages/core",
         "@npm//@types/node",
+        "@npm//base64-js",
+        "@npm//source-map",
     ],
 )

packages/compiler/testing/src/output/source_map_util.ts

@@ -7,8 +7,8 @@
  */
 
 import {SourceMap} from '@angular/compiler';
-const b64 = require('base64-js');
-const SourceMapConsumer = require('source-map').SourceMapConsumer;
+import b64 from 'base64-js';
+import {SourceMapConsumer} from 'source-map';
 
 export interface SourceLocation {
   line: number;
@@ -17,8 +17,10 @@ export interface SourceLocation {
 }
 
 export function originalPositionFor(
-    sourceMap: SourceMap, genPosition: {line: number|null, column: number|null}): SourceLocation {
-  const smc = new SourceMapConsumer(sourceMap);
+    sourceMap: SourceMap, genPosition: {line: number, column: number}): SourceLocation {
+  // Note: The `SourceMap` type from the compiler is different to `RawSourceMap`
+  // from the `source-map` package, but the method we rely on works as expected.
+  const smc = new SourceMapConsumer(sourceMap as any);
   // Note: We don't return the original object as it also contains a `name` property
   // which is always null and we don't want to include that in our assertions...
   const {line, column, source} = smc.originalPositionFor(genPosition);