From 380de1e7b464609663b36e67f303ce6933a1e3a6 Mon Sep 17 00:00:00 2001 From: Pete Bacon Darwin Date: Fri, 20 Mar 2020 22:09:40 +0000 Subject: [PATCH] fix(ngcc): use path-mappings from tsconfig in dependency resolution (#36180) When computing the dependencies between packages which are not in node_modules, we may need to rely upon path-mappings to find the path to the imported entry-point. This commit allows ngcc to use the path-mappings from a tsconfig file to find dependencies. By default any tsconfig.json file in the directory above the `basePath` is loaded but it is possible to use a path to a specific file by providing the `tsConfigPath` property to mainNgcc, or to turn off loading any tsconfig file by setting `tsConfigPath` to `null`. At the command line this is controlled via the `--tsconfig` option. Fixes #36119 PR Close #36180 --- packages/compiler-cli/ngcc/BUILD.bazel | 1 + packages/compiler-cli/ngcc/main-ngcc.ts | 13 +- packages/compiler-cli/ngcc/src/main.ts | 42 ++++- .../ngcc/test/integration/ngcc_spec.ts | 178 +++++++++++++++++- 4 files changed, 217 insertions(+), 17 deletions(-) diff --git a/packages/compiler-cli/ngcc/BUILD.bazel b/packages/compiler-cli/ngcc/BUILD.bazel index db91dca21a..3d8553c095 100644 --- a/packages/compiler-cli/ngcc/BUILD.bazel +++ b/packages/compiler-cli/ngcc/BUILD.bazel @@ -12,6 +12,7 @@ ts_library( deps = [ "//packages:types", "//packages/compiler", + "//packages/compiler-cli", "//packages/compiler-cli/src/ngtsc/annotations", "//packages/compiler-cli/src/ngtsc/cycles", "//packages/compiler-cli/src/ngtsc/diagnostics", diff --git a/packages/compiler-cli/ngcc/main-ngcc.ts b/packages/compiler-cli/ngcc/main-ngcc.ts index 394d9cb52d..cd88da9518 100644 --- a/packages/compiler-cli/ngcc/main-ngcc.ts +++ b/packages/compiler-cli/ngcc/main-ngcc.ts @@ -92,6 +92,13 @@ if (require.main === module) { type: 'boolean', default: false, }) + .option('tsconfig', { + describe: + 'A path to a tsconfig.json file that will be used to configure the Angular compiler and module resolution used by ngcc.\n' + + 'If not provided, ngcc will attempt to read a `tsconfig.json` file from the folder above that given by the `-s` option.\n' + + 'Set to false (via `--no-tsconfig`) if you do not want ngcc to use any `tsconfig.json` file.', + type: 'string', + }) .strict() .help() .parse(args); @@ -113,6 +120,10 @@ if (require.main === module) { const enableI18nLegacyMessageIdFormat = options['legacy-message-ids']; const invalidateEntryPointManifest = options['invalidate-entry-point-manifest']; const errorOnFailedEntryPoint = options['error-on-failed-entry-point']; + // yargs is not so great at mixed string+boolean types, so we have to test tsconfig against a + // string "false" to capture the `tsconfig=false` option. + // And we have to convert the option to a string to handle `no-tsconfig`, which will be `false`. + const tsConfigPath = `${options['tsconfig']}` === 'false' ? null : options['tsconfig']; (async() => { try { @@ -126,7 +137,7 @@ if (require.main === module) { createNewEntryPointFormats, logger, enableI18nLegacyMessageIdFormat, - async: options['async'], invalidateEntryPointManifest, errorOnFailedEntryPoint, + async: options['async'], invalidateEntryPointManifest, errorOnFailedEntryPoint, tsConfigPath }); if (logger) { diff --git a/packages/compiler-cli/ngcc/src/main.ts b/packages/compiler-cli/ngcc/src/main.ts index c22839a539..6a202be471 100644 --- a/packages/compiler-cli/ngcc/src/main.ts +++ b/packages/compiler-cli/ngcc/src/main.ts @@ -12,6 +12,7 @@ import {DepGraph} from 'dependency-graph'; import * as os from 'os'; import * as ts from 'typescript'; +import {readConfiguration} from '../..'; import {replaceTsWithNgInErrors} from '../../src/ngtsc/diagnostics'; import {AbsoluteFsPath, FileSystem, absoluteFrom, dirname, getFileSystem, resolve} from '../../src/ngtsc/file_system'; @@ -94,6 +95,9 @@ export interface SyncNgccOptions { /** * Paths mapping configuration (`paths` and `baseUrl`), as found in `ts.CompilerOptions`. * These are used to resolve paths to locally built Angular libraries. + * + * Note that `pathMappings` specified here take precedence over any `pathMappings` loaded from a + * TS config file. */ pathMappings?: PathMappings; @@ -143,6 +147,18 @@ export interface SyncNgccOptions { * Default: `false` (i.e. the manifest will be used if available) */ invalidateEntryPointManifest?: boolean; + + /** + * An absolute path to a TS config file (e.g. `tsconfig.json`) or a directory containing one, that + * will be used to configure module resolution with things like path mappings, if not specified + * explicitly via the `pathMappings` property to `mainNgcc`. + * + * If `undefined`, ngcc will attempt to load a `tsconfig.json` file from the directory above the + * `basePath`. + * + * If `null`, ngcc will not attempt to load any TS config file at all. + */ + tsConfigPath?: string|null; } /** @@ -165,12 +181,12 @@ export type NgccOptions = AsyncNgccOptions | SyncNgccOptions; */ export function mainNgcc(options: AsyncNgccOptions): Promise; export function mainNgcc(options: SyncNgccOptions): void; -export function mainNgcc({basePath, targetEntryPointPath, - propertiesToConsider = SUPPORTED_FORMAT_PROPERTIES, - compileAllFormats = true, createNewEntryPointFormats = false, - logger = new ConsoleLogger(LogLevel.info), pathMappings, async = false, - errorOnFailedEntryPoint = false, enableI18nLegacyMessageIdFormat = true, - invalidateEntryPointManifest = false}: NgccOptions): void|Promise { +export function mainNgcc( + {basePath, targetEntryPointPath, propertiesToConsider = SUPPORTED_FORMAT_PROPERTIES, + compileAllFormats = true, createNewEntryPointFormats = false, + logger = new ConsoleLogger(LogLevel.info), pathMappings, async = false, + errorOnFailedEntryPoint = false, enableI18nLegacyMessageIdFormat = true, + invalidateEntryPointManifest = false, tsConfigPath}: NgccOptions): void|Promise { if (!!targetEntryPointPath) { // targetEntryPointPath forces us to error if an entry-point fails. errorOnFailedEntryPoint = true; @@ -184,7 +200,19 @@ export function mainNgcc({basePath, targetEntryPointPath, // master/worker process. const fileSystem = getFileSystem(); const absBasePath = absoluteFrom(basePath); - const config = new NgccConfiguration(fileSystem, dirname(absBasePath)); + const projectPath = dirname(absBasePath); + const config = new NgccConfiguration(fileSystem, projectPath); + const tsConfig = tsConfigPath !== null ? readConfiguration(tsConfigPath || projectPath) : null; + + // If `pathMappings` is not provided directly, then try getting it from `tsConfig`, if available. + if (tsConfig !== null && pathMappings === undefined && tsConfig.options.baseUrl !== undefined && + tsConfig.options.paths) { + pathMappings = { + baseUrl: resolve(projectPath, tsConfig.options.baseUrl), + paths: tsConfig.options.paths, + }; + } + const dependencyResolver = getDependencyResolver(fileSystem, logger, config, pathMappings); const entryPointManifest = invalidateEntryPointManifest ? new InvalidatingEntryPointManifest(fileSystem, config, logger) : diff --git a/packages/compiler-cli/ngcc/test/integration/ngcc_spec.ts b/packages/compiler-cli/ngcc/test/integration/ngcc_spec.ts index 888c854415..bde8f468a0 100644 --- a/packages/compiler-cli/ngcc/test/integration/ngcc_spec.ts +++ b/packages/compiler-cli/ngcc/test/integration/ngcc_spec.ts @@ -1230,22 +1230,134 @@ runInEachFileSystem(() => { }); describe('with pathMappings', () => { - it('should find and compile packages accessible via the pathMappings', () => { - mainNgcc({ - basePath: '/node_modules', - propertiesToConsider: ['es2015'], - pathMappings: {paths: {'*': ['dist/*']}, baseUrl: '/'}, - }); - expect(loadPackage('@angular/core').__processed_by_ivy_ngcc__).toEqual({ + it('should infer the @app pathMapping from a local tsconfig.json path', () => { + fs.writeFile( + _('/tsconfig.json'), + JSON.stringify({compilerOptions: {paths: {'@app/*': ['dist/*']}, baseUrl: './'}})); + const logger = new MockLogger(); + mainNgcc({basePath: '/dist', propertiesToConsider: ['es2015'], logger}); + expect(loadPackage('local-package', _('/dist')).__processed_by_ivy_ngcc__).toEqual({ es2015: '0.0.0-PLACEHOLDER', - fesm2015: '0.0.0-PLACEHOLDER', typings: '0.0.0-PLACEHOLDER', }); + expect(loadPackage('local-package-2', _('/dist')).__processed_by_ivy_ngcc__).toEqual({ + es2015: '0.0.0-PLACEHOLDER', + typings: '0.0.0-PLACEHOLDER', + }); + // The local-package-3 and local-package-4 will not be processed because there is no path + // mappings for `@x` and plain local imports. + expect(loadPackage('local-package-3', _('/dist')).__processed_by_ivy_ngcc__) + .toBeUndefined(); + expect(logger.logs.debug).toContain([ + `Invalid entry-point ${_('/dist/local-package-3')}.`, + 'It is missing required dependencies:\n - @x/local-package' + ]); + expect(loadPackage('local-package-4', _('/dist')).__processed_by_ivy_ngcc__) + .toBeUndefined(); + expect(logger.logs.debug).toContain([ + `Invalid entry-point ${_('/dist/local-package-4')}.`, + 'It is missing required dependencies:\n - local-package' + ]); + }); + + it('should read the @x pathMapping from a specified tsconfig.json path', () => { + fs.writeFile( + _('/tsconfig.app.json'), + JSON.stringify({compilerOptions: {paths: {'@x/*': ['dist/*']}, baseUrl: './'}})); + const logger = new MockLogger(); + mainNgcc({ + basePath: '/dist', + propertiesToConsider: ['es2015'], + tsConfigPath: _('/tsconfig.app.json'), logger + }); expect(loadPackage('local-package', _('/dist')).__processed_by_ivy_ngcc__).toEqual({ es2015: '0.0.0-PLACEHOLDER', typings: '0.0.0-PLACEHOLDER', }); + expect(loadPackage('local-package-3', _('/dist')).__processed_by_ivy_ngcc__).toEqual({ + es2015: '0.0.0-PLACEHOLDER', + typings: '0.0.0-PLACEHOLDER', + }); + // The local-package-2 and local-package-4 will not be processed because there is no path + // mappings for `@app` and plain local imports. + expect(loadPackage('local-package-2', _('/dist')).__processed_by_ivy_ngcc__) + .toBeUndefined(); + expect(logger.logs.debug).toContain([ + `Invalid entry-point ${_('/dist/local-package-2')}.`, + 'It is missing required dependencies:\n - @app/local-package' + ]); + expect(loadPackage('local-package-4', _('/dist')).__processed_by_ivy_ngcc__) + .toBeUndefined(); + expect(logger.logs.debug).toContain([ + `Invalid entry-point ${_('/dist/local-package-4')}.`, + 'It is missing required dependencies:\n - local-package' + ]); }); + + it('should use the explicit `pathMappings`, ignoring the local tsconfig.json settings', + () => { + const logger = new MockLogger(); + fs.writeFile( + _('/tsconfig.json'), + JSON.stringify({compilerOptions: {paths: {'@app/*': ['dist/*']}, baseUrl: './'}})); + mainNgcc({ + basePath: '/node_modules', + propertiesToConsider: ['es2015'], + pathMappings: {paths: {'*': ['dist/*']}, baseUrl: '/'}, logger + }); + expect(loadPackage('@angular/core').__processed_by_ivy_ngcc__).toEqual({ + es2015: '0.0.0-PLACEHOLDER', + fesm2015: '0.0.0-PLACEHOLDER', + typings: '0.0.0-PLACEHOLDER', + }); + expect(loadPackage('local-package', _('/dist')).__processed_by_ivy_ngcc__).toEqual({ + es2015: '0.0.0-PLACEHOLDER', + typings: '0.0.0-PLACEHOLDER', + }); + expect(loadPackage('local-package-4', _('/dist')).__processed_by_ivy_ngcc__).toEqual({ + es2015: '0.0.0-PLACEHOLDER', + typings: '0.0.0-PLACEHOLDER', + }); + // The local-package-2 and local-package-3 will not be processed because there is no path + // mappings for `@app` and `@x` local imports. + expect(loadPackage('local-package-2', _('/dist')).__processed_by_ivy_ngcc__) + .toBeUndefined(); + expect(logger.logs.debug).toContain([ + `Invalid entry-point ${_('/dist/local-package-2')}.`, + 'It is missing required dependencies:\n - @app/local-package' + ]); + expect(loadPackage('local-package-3', _('/dist')).__processed_by_ivy_ngcc__) + .toBeUndefined(); + expect(logger.logs.debug).toContain([ + `Invalid entry-point ${_('/dist/local-package-3')}.`, + 'It is missing required dependencies:\n - @x/local-package' + ]); + }); + + it('should not use pathMappings from a local tsconfig.json path if tsConfigPath is null', + () => { + const logger = new MockLogger(); + fs.writeFile( + _('/tsconfig.json'), + JSON.stringify({compilerOptions: {paths: {'@app/*': ['dist/*']}, baseUrl: './'}})); + mainNgcc({ + basePath: '/dist', + propertiesToConsider: ['es2015'], + tsConfigPath: null, logger, + }); + expect(loadPackage('local-package', _('/dist')).__processed_by_ivy_ngcc__).toEqual({ + es2015: '0.0.0-PLACEHOLDER', + typings: '0.0.0-PLACEHOLDER', + }); + // Since the tsconfig is not loaded, the `@app/local-package` import in `local-package-2` + // is not path-mapped correctly, and so it fails to be processed. + expect(loadPackage('local-package-2', _('/dist')).__processed_by_ivy_ngcc__) + .toBeUndefined(); + expect(logger.logs.debug).toContain([ + `Invalid entry-point ${_('/dist/local-package-2')}.`, + 'It is missing required dependencies:\n - @app/local-package' + ]); + }); }); describe('with configuration files', () => { @@ -1748,7 +1860,7 @@ runInEachFileSystem(() => { }, ]); - // An Angular package that has been built locally and stored in the `dist` directory. + // Angular packages that have been built locally and stored in the `dist` directory. loadTestFiles([ { name: _('/dist/local-package/package.json'), @@ -1764,6 +1876,54 @@ runInEachFileSystem(() => { name: _('/dist/local-package/index.d.ts'), contents: `export declare class AppComponent {};` }, + // local-package-2 depends upon local-package, via an `@app` aliased import. + { + name: _('/dist/local-package-2/package.json'), + contents: '{"name": "local-package-2", "es2015": "./index.js", "typings": "./index.d.ts"}' + }, + {name: _('/dist/local-package-2/index.metadata.json'), contents: 'DUMMY DATA'}, + { + name: _('/dist/local-package-2/index.js'), + contents: + `import {Component} from '@angular/core';\nexport {AppComponent} from '@app/local-package';` + }, + { + name: _('/dist/local-package-2/index.d.ts'), + contents: + `import {Component} from '@angular/core';\nexport {AppComponent} from '@app/local-package';` + }, + // local-package-3 depends upon local-package, via an `@x` aliased import. + { + name: _('/dist/local-package-3/package.json'), + contents: '{"name": "local-package-3", "es2015": "./index.js", "typings": "./index.d.ts"}' + }, + {name: _('/dist/local-package-3/index.metadata.json'), contents: 'DUMMY DATA'}, + { + name: _('/dist/local-package-3/index.js'), + contents: + `import {Component} from '@angular/core';\nexport {AppComponent} from '@x/local-package';` + }, + { + name: _('/dist/local-package-3/index.d.ts'), + contents: + `import {Component} from '@angular/core';\nexport {AppComponent} from '@x/local-package';` + }, + // local-package-4 depends upon local-package, via a plain import. + { + name: _('/dist/local-package-4/package.json'), + contents: '{"name": "local-package-", "es2015": "./index.js", "typings": "./index.d.ts"}' + }, + {name: _('/dist/local-package-4/index.metadata.json'), contents: 'DUMMY DATA'}, + { + name: _('/dist/local-package-4/index.js'), + contents: + `import {Component} from '@angular/core';\nexport {AppComponent} from 'local-package';` + }, + { + name: _('/dist/local-package-4/index.d.ts'), + contents: + `import {Component} from '@angular/core';\nexport {AppComponent} from 'local-package';` + }, ]); // An Angular package that has a missing dependency