Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

fix(ngcc): use path-mappings from tsconfig in dependency resolution #36180

Closed
wants to merge 3 commits into from
Closed

fix(ngcc): use path-mappings from tsconfig in dependency resolution #36180

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
84cf70e
fix(ngcc): use path-mappings from tsconfig in dependency resolution
petebacondarwin Mar 20, 2020
0fa4986
fixup! fix(ngcc): use path-mappings from tsconfig in dependency resol…
petebacondarwin Mar 23, 2020
deb1ccb
fixup! fix(ngcc): use path-mappings from tsconfig in dependency resol…
petebacondarwin Mar 23, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
1 change: 1 addition & 0 deletions packages/compiler-cli/ngcc/BUILD.bazel
View file
Open in desktop
Expand Up @@ -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",
Expand Down
13 changes: 12 additions & 1 deletion packages/compiler-cli/ngcc/main-ngcc.ts
View file
Open in desktop
Expand Up @@ -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);
Expand All @@ -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 {
Expand All @@ -126,7 +137,7 @@ if (require.main === module) {
createNewEntryPointFormats,
logger,
enableI18nLegacyMessageIdFormat,
async: options['async'], invalidateEntryPointManifest, errorOnFailedEntryPoint,
async: options['async'], invalidateEntryPointManifest, errorOnFailedEntryPoint, tsConfigPath
});

if (logger) {
Expand Down
42 changes: 35 additions & 7 deletions packages/compiler-cli/ngcc/src/main.ts
View file
Open in desktop
Expand Up @@ -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';

Expand Down Expand Up @@ -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;

Expand Down Expand Up @@ -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.
*/
petebacondarwin marked this conversation as resolved.
Show resolved Hide resolved
tsConfigPath?: string|null;
}

/**
Expand All @@ -165,12 +181,12 @@ export type NgccOptions = AsyncNgccOptions | SyncNgccOptions;
*/
export function mainNgcc(options: AsyncNgccOptions): Promise<void>;
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<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, tsConfigPath}: NgccOptions): void|Promise<void> {
if (!!targetEntryPointPath) {
// targetEntryPointPath forces us to error if an entry-point fails.
errorOnFailedEntryPoint = true;
Expand All @@ -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) :
Expand Down
174 changes: 167 additions & 7 deletions packages/compiler-cli/ngcc/test/integration/ngcc_spec.ts
View file
Open in desktop
Expand Up @@ -1230,22 +1230,134 @@ runInEachFileSystem(() => {
});

describe('with pathMappings', () => {
it('should find and compile packages accessible via the pathMappings', () => {
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',
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: '/node_modules',
basePath: '/dist',
propertiesToConsider: ['es2015'],
pathMappings: {paths: {'*': ['dist/*']}, baseUrl: '/'},
tsConfigPath: _('/tsconfig.app.json'), logger
});
expect(loadPackage('@angular/core').__processed_by_ivy_ngcc__).toEqual({
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', _('/dist')).__processed_by_ivy_ngcc__).toEqual({
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__)
petebacondarwin marked this conversation as resolved.
Show resolved Hide resolved
.toBeUndefined();
expect(logger.logs.debug).toContain([
`Invalid entry-point ${_('/dist/local-package-2')}.`,
'It is missing required dependencies:\n - @app/local-package'
]);
});
petebacondarwin marked this conversation as resolved.
Show resolved Hide resolved
});

describe('with configuration files', () => {
Expand Down Expand Up @@ -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'),
Expand All @@ -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
Expand Down