fix(compiler-cli): readConfiguration existing options should override options in tsconfig

packages/bazel/src/ngc-wrapped/index.ts

@@ -44,81 +44,54 @@ export function runOneBuild(args: string[], inputs?: {[path: string]: string}):
   const project = args[0].replace(/^@+/, '');
 
   const [parsedOptions, errors] = parseTsconfig(project);
-  if (errors && errors.length) {
+  if (errors?.length) {
     console.error(ng.formatDiagnostics(errors));
     return false;
   }
-  const {options: tsOptions, bazelOpts, files, config} = parsedOptions;
-  const angularCompilerOptions: {[k: string]: unknown} = config['angularCompilerOptions'] || {};
-
-  // Allow Bazel users to control some of the bazel options.
-  // Since TypeScript's "extends" mechanism applies only to "compilerOptions"
-  // we have to repeat some of their logic to get the user's "angularCompilerOptions".
-  if (config['extends']) {
-    // Load the user's config file
-    // Note: this doesn't handle recursive extends so only a user's top level
-    // `angularCompilerOptions` will be considered. As this code is going to be
-    // removed with Ivy, the added complication of handling recursive extends
-    // is likely not needed.
-    let userConfigFile = resolveNormalizedPath(path.dirname(project), config['extends']);
-    if (!userConfigFile.endsWith('.json')) userConfigFile += '.json';
-    const {config: userConfig, error} = ts.readConfigFile(userConfigFile, ts.sys.readFile);
-    if (error) {
-      console.error(ng.formatDiagnostics([error]));
-      return false;
-    }
 
-    // All user angularCompilerOptions values that a user has control
-    // over should be collected here
-    if (userConfig.angularCompilerOptions) {
-      angularCompilerOptions['diagnostics'] =
-          angularCompilerOptions['diagnostics'] || userConfig.angularCompilerOptions.diagnostics;
-      angularCompilerOptions['trace'] =
-          angularCompilerOptions['trace'] || userConfig.angularCompilerOptions.trace;
-
-      angularCompilerOptions['disableExpressionLowering'] =
-          angularCompilerOptions['disableExpressionLowering'] ||
-          userConfig.angularCompilerOptions.disableExpressionLowering;
-      angularCompilerOptions['disableTypeScriptVersionCheck'] =
-          angularCompilerOptions['disableTypeScriptVersionCheck'] ||
-          userConfig.angularCompilerOptions.disableTypeScriptVersionCheck;
-
-      angularCompilerOptions['i18nOutLocale'] = angularCompilerOptions['i18nOutLocale'] ||
-          userConfig.angularCompilerOptions.i18nOutLocale;
-      angularCompilerOptions['i18nOutFormat'] = angularCompilerOptions['i18nOutFormat'] ||
-          userConfig.angularCompilerOptions.i18nOutFormat;
-      angularCompilerOptions['i18nOutFile'] =
-          angularCompilerOptions['i18nOutFile'] || userConfig.angularCompilerOptions.i18nOutFile;
-
-      angularCompilerOptions['i18nInFormat'] =
-          angularCompilerOptions['i18nInFormat'] || userConfig.angularCompilerOptions.i18nInFormat;
-      angularCompilerOptions['i18nInLocale'] =
-          angularCompilerOptions['i18nInLocale'] || userConfig.angularCompilerOptions.i18nInLocale;
-      angularCompilerOptions['i18nInFile'] =
-          angularCompilerOptions['i18nInFile'] || userConfig.angularCompilerOptions.i18nInFile;
-
-      angularCompilerOptions['i18nInMissingTranslations'] =
-          angularCompilerOptions['i18nInMissingTranslations'] ||
-          userConfig.angularCompilerOptions.i18nInMissingTranslations;
-      angularCompilerOptions['i18nUseExternalIds'] = angularCompilerOptions['i18nUseExternalIds'] ||
-          userConfig.angularCompilerOptions.i18nUseExternalIds;
-
-      angularCompilerOptions['preserveWhitespaces'] =
-          angularCompilerOptions['preserveWhitespaces'] ||
-          userConfig.angularCompilerOptions.preserveWhitespaces;
-
-      angularCompilerOptions.createExternalSymbolFactoryReexports =
-          angularCompilerOptions.createExternalSymbolFactoryReexports ||
-          userConfig.angularCompilerOptions.createExternalSymbolFactoryReexports;
-    }
+  const {bazelOpts, options: tsOptions, files, config} = parsedOptions;
+  const {errors: userErrors, options: userOptions} = ng.readConfiguration(project);
+
+  if (userErrors?.length) {
+    console.error(ng.formatDiagnostics(userErrors));
+    return false;
   }
 
+  const allowedNgCompilerOptionsOverrides = new Set<string>([
+    'diagnostics',
+    'trace',
+    'disableExpressionLowering',
+    'disableTypeScriptVersionCheck',
+    'i18nOutLocale',
+    'i18nOutFormat',
+    'i18nOutFile',
+    'i18nInLocale',
+    'i18nInFile',
+    'i18nInFormat',
+    'i18nUseExternalIds',
+    'i18nInMissingTranslations',
+    'preserveWhitespaces',
+    'createExternalSymbolFactoryReexports',
+  ]);
+
+  const userOverrides = Object.entries(userOptions)
+                            .filter(([key]) => allowedNgCompilerOptionsOverrides.has(key))
+                            .reduce((obj, [key, value]) => {
+                              obj[key] = value;
+
+                              return obj;
+                            }, {});
+
+  const compilerOpts: ng.AngularCompilerOptions = {
+    ...userOverrides,
+    ...config['angularCompilerOptions'],
+    ...tsOptions,
+  };
+
   // These are options passed through from the `ng_module` rule which aren't supported
   // by the `@angular/compiler-cli` and are only intended for `ngc-wrapped`.
   const {expectedOut, _useManifestPathsAsModuleName} = config['angularCompilerOptions'];
 
-  const {basePath} = ng.calcProjectFileAndBasePath(project);
-  const compilerOpts = ng.createNgCompilerOptions(basePath, config, tsOptions);
   const tsHost = ts.createCompilerHost(compilerOpts, true);
   const {diagnostics} = compile({
     allDepsCompiledWithBazel: ALL_DEPS_COMPILED_WITH_BAZEL,

packages/compiler-cli/src/perform_compile.ts

@@ -9,7 +9,8 @@
 import {isSyntaxError, Position} from '@angular/compiler';
 import * as ts from 'typescript';
 
-import {absoluteFrom, AbsoluteFsPath, getFileSystem, ReadonlyFileSystem, relative, resolve} from '../src/ngtsc/file_system';
+import {absoluteFrom, AbsoluteFsPath, FileSystem, getFileSystem, ReadonlyFileSystem, relative, resolve} from '../src/ngtsc/file_system';
+import {NgCompilerOptions} from './ngtsc/core/api';
 
 import {replaceTsWithNgInErrors} from './ngtsc/diagnostics';
 import * as api from './transformers/api';
@@ -132,58 +133,44 @@ export function calcProjectFileAndBasePath(
   return {projectFile, basePath};
 }
 
-export function createNgCompilerOptions(
-    basePath: string, config: any, tsOptions: ts.CompilerOptions): api.CompilerOptions {
-  // enableIvy `ngtsc` is an alias for `true`.
-  const {angularCompilerOptions = {}} = config;
-  const {enableIvy} = angularCompilerOptions;
-  angularCompilerOptions.enableIvy = enableIvy !== false && enableIvy !== 'tsc';
-
-  return {...tsOptions, ...angularCompilerOptions, genDir: basePath, basePath};
-}
-
 export function readConfiguration(
-    project: string, existingOptions?: ts.CompilerOptions,
+    project: string, existingOptions?: api.CompilerOptions,
     host: ConfigurationHost = getFileSystem()): ParsedConfiguration {
   try {
-    const {projectFile, basePath} = calcProjectFileAndBasePath(project, host);
+    const fs = getFileSystem();
 
-    const readExtendedConfigFile =
-        (configFile: string, existingConfig?: any): {config?: any, error?: ts.Diagnostic} => {
-          const {config, error} =
-              ts.readConfigFile(configFile, file => host.readFile(host.resolve(file)));
+    const readConfigFile = (configFile: string) =>
+        ts.readConfigFile(configFile, file => host.readFile(host.resolve(file)));
+    const readAngularCompilerOptions =
+        (configFile: string, parentOptions: NgCompilerOptions = {}): NgCompilerOptions => {
+          const {config, error} = readConfigFile(configFile);
 
           if (error) {
-            return {error};
+            // Errors are handled later on by 'parseJsonConfigFileContent'
+            return parentOptions;
           }
 
           // we are only interested into merging 'angularCompilerOptions' as
           // other options like 'compilerOptions' are merged by TS
-          const baseConfig = existingConfig || config;
-          if (existingConfig) {
-            baseConfig.angularCompilerOptions = {
-              ...config.angularCompilerOptions,
-              ...baseConfig.angularCompilerOptions
-            };
-          }
+          const existingNgCompilerOptions = {...config.angularCompilerOptions, ...parentOptions};
 
-          if (config.extends) {
-            let extendedConfigPath = host.resolve(host.dirname(configFile), config.extends);
-            extendedConfigPath = host.extname(extendedConfigPath) ?
-                extendedConfigPath :
-                absoluteFrom(`${extendedConfigPath}.json`);
+          if (config.extends && typeof config.extends === 'string') {
+            const extendedConfigPath = getExtendedConfigPath(
+                configFile, config.extends, host, fs,
+            );
 
-            if (host.exists(extendedConfigPath)) {
-              // Call read config recursively as TypeScript only merges CompilerOptions
-              return readExtendedConfigFile(extendedConfigPath, baseConfig);
+            if (extendedConfigPath !== null) {
+              // Call readAngularCompilerOptions recursively to merge NG Compiler options
+              return readAngularCompilerOptions(extendedConfigPath, existingNgCompilerOptions);
             }
           }
 
-          return {config: baseConfig};
+          return existingNgCompilerOptions;
         };
 
-    const {config, error} = readExtendedConfigFile(projectFile);
-
+    const {projectFile, basePath} = calcProjectFileAndBasePath(project, host);
+    const configFileName = host.resolve(host.pwd(), projectFile);
+    const {config, error} = readConfigFile(projectFile);
     if (error) {
       return {
         project,
@@ -193,34 +180,29 @@ export function readConfiguration(
         emitFlags: api.EmitFlags.Default
       };
     }
-    const parseConfigHost = {
-      useCaseSensitiveFileNames: true,
-      fileExists: host.exists.bind(host),
-      readDirectory: ts.sys.readDirectory,
-      readFile: ts.sys.readFile
+    const existingCompilerOptions: api.CompilerOptions = {
+      genDir: basePath,
+      basePath,
+      ...readAngularCompilerOptions(configFileName),
+      ...existingOptions,
     };
-    const configFileName = host.resolve(host.pwd(), projectFile);
-    const parsed = ts.parseJsonConfigFileContent(
-        config, parseConfigHost, basePath, existingOptions, configFileName);
-    const rootNames = parsed.fileNames;
-    const projectReferences = parsed.projectReferences;
 
-    const options = createNgCompilerOptions(basePath, config, parsed.options);
+    const parseConfigHost = createParseConfigHost(host, fs);
+    const {options, errors, fileNames: rootNames, projectReferences} =
+        ts.parseJsonConfigFileContent(
+            config, parseConfigHost, basePath, existingCompilerOptions, configFileName);
+
+    // Coerce to boolean as `enableIvy` can be `ngtsc|true|false|undefined` here.
+    options.enableIvy = !!(options.enableIvy ?? true);
+
     let emitFlags = api.EmitFlags.Default;
     if (!(options.skipMetadataEmit || options.flatModuleOutFile)) {
       emitFlags |= api.EmitFlags.Metadata;
     }
     if (options.skipTemplateCodegen) {
       emitFlags = emitFlags & ~api.EmitFlags.Codegen;
     }
-    return {
-      project: projectFile,
-      rootNames,
-      projectReferences,
-      options,
-      errors: parsed.errors,
-      emitFlags
-    };
+    return {project: projectFile, rootNames, projectReferences, options, errors, emitFlags};
   } catch (e) {
     const errors: ts.Diagnostic[] = [{
       category: ts.DiagnosticCategory.Error,
@@ -235,6 +217,47 @@ export function readConfiguration(
   }
 }
 
+function createParseConfigHost(host: ConfigurationHost, fs = getFileSystem()): ts.ParseConfigHost {
+  return {
+    fileExists: host.exists.bind(host),
+    readDirectory: ts.sys.readDirectory,
+    readFile: host.readFile.bind(host),
+    useCaseSensitiveFileNames: fs.isCaseSensitive(),
+  };
+}
+
+function getExtendedConfigPath(
+    configFile: string, extendsValue: string, host: ConfigurationHost,
+    fs: FileSystem): AbsoluteFsPath|null {
+  let extendedConfigPath: AbsoluteFsPath|null = null;
+
+  if (extendsValue.startsWith('.') || fs.isRooted(extendsValue)) {
+    extendedConfigPath = host.resolve(host.dirname(configFile), extendsValue);
+    extendedConfigPath = host.extname(extendedConfigPath) ?
+        extendedConfigPath :
+        absoluteFrom(`${extendedConfigPath}.json`);
+  } else {
+    const parseConfigHost = createParseConfigHost(host, fs);
+
+    // Path isn't a rooted or relative path, resolve like a module.
+    const {
+      resolvedModule,
+    } =
+        ts.nodeModuleNameResolver(
+            extendsValue, configFile,
+            {moduleResolution: ts.ModuleResolutionKind.NodeJs, resolveJsonModule: true},
+            parseConfigHost);
+    if (resolvedModule) {
+      extendedConfigPath = absoluteFrom(resolvedModule.resolvedFileName);
+    }
+  }
+
+  if (extendedConfigPath !== null && host.exists(extendedConfigPath)) {
+    return extendedConfigPath;
+  }
+
+  return null;
+}
 export interface PerformCompilationResult {
   diagnostics: Diagnostics;
   program?: api.Program;

packages/compiler-cli/test/BUILD.bazel

@@ -135,6 +135,7 @@ ts_library(
         ":test_utils",
         "//packages/compiler",
         "//packages/compiler-cli",
+        "@npm//typescript",
     ],
 )
 

packages/compiler-cli/test/perform_compile_spec.ts

@@ -7,6 +7,7 @@
  */
 
 import * as path from 'path';
+import * as ts from 'typescript';
 
 import {readConfiguration} from '../src/perform_compile';
 
@@ -77,4 +78,62 @@ describe('perform_compile', () => {
        const {options} = readConfiguration(path.resolve(basePath, 'tsconfig-level-1.json'));
        expect(options.enableIvy).toBe(false);
      });
+
+  it('should override options defined in tsconfig with those defined in `existingOptions`', () => {
+    support.writeFiles({
+      'tsconfig-level-1.json': `{
+          "compilerOptions": {
+            "target": "es2020"
+          },
+          "angularCompilerOptions": {
+            "annotateForClosureCompiler": true
+          }
+        }
+      `
+    });
+
+    const {options} = readConfiguration(
+        path.resolve(basePath, 'tsconfig-level-1.json'),
+        {annotateForClosureCompiler: false, target: ts.ScriptTarget.ES2015, enableIvy: false});
+
+    expect(options).toEqual(jasmine.objectContaining({
+      enableIvy: false,
+      target: ts.ScriptTarget.ES2015,
+      annotateForClosureCompiler: false,
+    }));
+  });
+
+  it('should merge tsconfig "angularCompilerOptions" when extends point to node package', () => {
+    support.writeFiles({
+      'tsconfig-level-1.json': `{
+          "extends": "@angular-ru/tsconfig",
+          "angularCompilerOptions": {
+            "enableIvy": false
+          }
+        }
+      `,
+      'node_modules/@angular-ru/tsconfig/tsconfig.json': `{
+          "compilerOptions": {
+            "strict": true
+          },
+          "angularCompilerOptions": {
+            "skipMetadataEmit": true
+          }
+        }
+      `,
+      'node_modules/@angular-ru/tsconfig/package.json': `{
+        "name": "@angular-ru/tsconfig",
+        "version": "0.0.0",
+        "main": "./tsconfig.json"
+      }
+    `,
+    });
+
+    const {options} = readConfiguration(path.resolve(basePath, 'tsconfig-level-1.json'));
+    expect(options).toEqual(jasmine.objectContaining({
+      strict: true,
+      skipMetadataEmit: true,
+      enableIvy: false,
+    }));
+  });
 });