honeymoose
/
angular-cn
1
0
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

fix(ngcc): correctly get config for sub-entry-points when primary entry-point is ignored (#37040) 

Previously, when an entry-point was ignored via an ngcc config, ngcc
would scan sub-directories for sub-entry-points, but would not use the
correct `packagePath`. For example, if `@angular/common` was ignored, it
would look at `@angular/common/http` but incorrectly use
`.../@angular/common/http` as the `packagePath` (instead of
`.../@angular/common`). As a result, it would not retrieve the correct
ngcc config for the actual package.

This commit fixes it by ensuring the correct `packagePath` is used, even
if the primary entry-point corresponding to that path is ignored. In
order to do this, a new return value for `getEntryPointInfo()` is added:
`IGNORED_ENTRY_POINT`. This is used to differentiate between directories
that correspond to no or an incompatible entry-point and those that
correspond to an entry-point that could otherwise be valid but is
explicitly ignored. Consumers of `getEntryPointInfo()` can then use this
info to discard ignored entry-points, but still use the correct
`packagePath` when scanning their sub-directories for secondary
entry-points.

PR Close #37040
This commit is contained in:
George Kalpakas 2020-06-08 22:04:25 +03:00 committed by Misko Hevery
parent ab9bc8a9ec
commit e7a0e87c41
11 changed files with 193 additions and 49 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

18
packages/compiler-cli/ngcc/src/entry_point_finder/directory_walker_entry_point_finder.ts
View File

@ -10,7 +10,7 @@ import {EntryPointWithDependencies} from '../dependencies/dependency_host';
import {DependencyResolver, SortedEntryPointsInfo} from '../dependencies/dependency_resolver';
import {Logger} from '../logging/logger';
import {NgccConfiguration} from '../packages/configuration';
import {getEntryPointInfo, INCOMPATIBLE_ENTRY_POINT, NO_ENTRY_POINT} from '../packages/entry_point';
import {getEntryPointInfo, IGNORED_ENTRY_POINT, INCOMPATIBLE_ENTRY_POINT, isEntryPoint, NO_ENTRY_POINT} from '../packages/entry_point';
import {EntryPointManifest} from '../packages/entry_point_manifest';
import {PathMappings} from '../path_mappings';
import {NGCC_DIRECTORY} from '../writing/new_entry_point_file_writer';
@ -79,7 +79,9 @@ export class DirectoryWalkerEntryPointFinder implements EntryPointFinder {
const entryPoints: EntryPointWithDependencies[] = [];
if (primaryEntryPoint !== NO_ENTRY_POINT) {
entryPoints.push(this.resolver.getEntryPointWithDependencies(primaryEntryPoint));
if (primaryEntryPoint !== IGNORED_ENTRY_POINT) {
entryPoints.push(this.resolver.getEntryPointWithDependencies(primaryEntryPoint));
}
this.collectSecondaryEntryPoints(
entryPoints, sourceDirectory, sourceDirectory, this.fs.readdir(sourceDirectory));
@ -148,14 +150,12 @@ export class DirectoryWalkerEntryPointFinder implements EntryPointFinder {
}
// If the path is a JS file then strip its extension and see if we can match an
// entry-point.
// entry-point (even if it is an ignored one).
const possibleEntryPointPath = isDirectory ? absolutePath : stripJsExtension(absolutePath);
let isEntryPoint = false;
const subEntryPoint =
getEntryPointInfo(this.fs, this.config, this.logger, packagePath, possibleEntryPointPath);
if (subEntryPoint !== NO_ENTRY_POINT && subEntryPoint !== INCOMPATIBLE_ENTRY_POINT) {
if (isEntryPoint(subEntryPoint)) {
entryPoints.push(this.resolver.getEntryPointWithDependencies(subEntryPoint));
isEntryPoint = true;
}
if (!isDirectory) {
@ -163,9 +163,11 @@ export class DirectoryWalkerEntryPointFinder implements EntryPointFinder {
continue;
}
// This directory may contain entry-points of its own.
// If not an entry-point itself, this directory may contain entry-points of its own.
const canContainEntryPoints =
subEntryPoint === NO_ENTRY_POINT || subEntryPoint === INCOMPATIBLE_ENTRY_POINT;
const childPaths = this.fs.readdir(absolutePath);
if (!isEntryPoint &&
if (canContainEntryPoints &&
childPaths.some(
childPath => childPath.endsWith('.js') &&
this.fs.stat(this.fs.resolve(absolutePath, childPath)).isFile())) {

8
packages/compiler-cli/ngcc/src/entry_point_finder/tracing_entry_point_finder.ts
View File

@ -11,7 +11,7 @@ import {EntryPointWithDependencies} from '../dependencies/dependency_host';
import {DependencyResolver, SortedEntryPointsInfo} from '../dependencies/dependency_resolver';
import {Logger} from '../logging/logger';
import {NgccConfiguration} from '../packages/configuration';
import {EntryPoint, getEntryPointInfo, INCOMPATIBLE_ENTRY_POINT, NO_ENTRY_POINT} from '../packages/entry_point';
import {EntryPoint, getEntryPointInfo, isEntryPoint} from '../packages/entry_point';
import {PathMappings} from '../path_mappings';
import {EntryPointFinder} from './interface';
@ -61,10 +61,8 @@ export abstract class TracingEntryPointFinder implements EntryPointFinder {
const packagePath = this.computePackagePath(entryPointPath);
const entryPoint =
getEntryPointInfo(this.fs, this.config, this.logger, packagePath, entryPointPath);
if (entryPoint === NO_ENTRY_POINT || entryPoint === INCOMPATIBLE_ENTRY_POINT) {
return null;
}
return entryPoint;
return isEntryPoint(entryPoint) ? entryPoint : null;
}
private processNextPath(): void {

29
packages/compiler-cli/ngcc/src/packages/entry_point.ts
View File

@ -80,12 +80,16 @@ export const SUPPORTED_FORMAT_PROPERTIES: EntryPointJsonProperty[] =
/**
* The path does not represent an entry-point:
* * there is no package.json at the path and there is no config to force an entry-point
* * or the entrypoint is `ignored` by a config.
* The path does not represent an entry-point, i.e. there is no package.json at the path and there
* is no config to force an entry-point.
*/
export const NO_ENTRY_POINT = 'no-entry-point';
/**
* The path represents an entry-point that is `ignored` by an ngcc config.
*/
export const IGNORED_ENTRY_POINT = 'ignored-entry-point';
/**
* The path has a package.json, but it is not a valid entry-point for ngcc processing.
*/
@ -100,7 +104,8 @@ export const INCOMPATIBLE_ENTRY_POINT = 'incompatible-entry-point';
* * INCOMPATIBLE_ENTRY_POINT - the path was a non-processable entry-point that should be searched
* for sub-entry-points
*/
export type GetEntryPointResult = EntryPoint|typeof INCOMPATIBLE_ENTRY_POINT|typeof NO_ENTRY_POINT;
export type GetEntryPointResult =
EntryPoint|typeof IGNORED_ENTRY_POINT|typeof INCOMPATIBLE_ENTRY_POINT|typeof NO_ENTRY_POINT;
/**
@ -109,11 +114,12 @@ export type GetEntryPointResult = EntryPoint|typeof INCOMPATIBLE_ENTRY_POINT|typ
* @param packagePath the absolute path to the containing npm package
* @param entryPointPath the absolute path to the potential entry-point.
* @returns
* - An entry-point if it is valid.
* - An entry-point if it is valid and not ignored.
* - `NO_ENTRY_POINT` when there is no package.json at the path and there is no config to force an
* entry-point or the entrypoint is `ignored`.
* - `INCOMPATIBLE_ENTRY_POINT` there is a package.json but it is not a valid Angular compiled
* entry-point.
* entry-point,
* - `IGNORED_ENTRY_POINT` when the entry-point is ignored by an ngcc config.
* - `INCOMPATIBLE_ENTRY_POINT` when there is a package.json but it is not a valid Angular compiled
* entry-point.
*/
export function getEntryPointInfo(
fs: FileSystem, config: NgccConfiguration, logger: Logger, packagePath: AbsoluteFsPath,
@ -131,7 +137,7 @@ export function getEntryPointInfo(
if (hasConfig && entryPointConfig.ignore === true) {
// Explicitly ignored
return NO_ENTRY_POINT;
return IGNORED_ENTRY_POINT;
}
const loadedEntryPointPackageJson = loadEntryPointPackage(fs, logger, packageJsonPath, hasConfig);
@ -174,6 +180,11 @@ export function getEntryPointInfo(
return entryPointInfo;
}
export function isEntryPoint(result: GetEntryPointResult): result is EntryPoint {
return result !== NO_ENTRY_POINT && result !== INCOMPATIBLE_ENTRY_POINT &&
result !== IGNORED_ENTRY_POINT;
}
/**
* Convert a package.json property into an entry-point format.
*

4
packages/compiler-cli/ngcc/src/packages/entry_point_manifest.ts
View File

@ -13,7 +13,7 @@ import {Logger} from '../logging/logger';
import {NGCC_VERSION} from './build_marker';
import {NgccConfiguration} from './configuration';
import {getEntryPointInfo, INCOMPATIBLE_ENTRY_POINT, NO_ENTRY_POINT} from './entry_point';
import {getEntryPointInfo, isEntryPoint} from './entry_point';
/**
* Manages reading and writing a manifest file that contains a list of all the entry-points that
@ -75,7 +75,7 @@ export class EntryPointManifest {
const result = getEntryPointInfo(
this.fs, this.config, this.logger, this.fs.resolve(basePath, packagePath),
this.fs.resolve(basePath, entryPointPath));
if (result === NO_ENTRY_POINT || result === INCOMPATIBLE_ENTRY_POINT) {
if (!isEntryPoint(result)) {
throw new Error(`The entry-point manifest at ${
manifestPath} contained an invalid pair of package paths: [${packagePath}, ${
entryPointPath}]`);

42
packages/compiler-cli/ngcc/test/entry_point_finder/directory_walker_entry_point_finder_spec.ts
View File

@ -103,6 +103,48 @@ runInEachFileSystem(() => {
expect(entryPoints).toEqual([]);
});
it('should not include ignored entry-points', () => {
const basePath = _Abs('/project/node_modules');
const finder = new DirectoryWalkerEntryPointFinder(
fs, config, logger, resolver, manifest, basePath, undefined);
loadTestFiles(createPackage(basePath, 'some-package'));
spyOn(config, 'getPackageConfig').and.returnValue({
versionRange: '*',
entryPoints: {
[_Abs('/project/node_modules/some-package')]: {ignore: true},
},
});
const {entryPoints} = finder.findEntryPoints();
expect(entryPoints).toEqual([]);
});
it('should look for sub-entry-points even if a containing entry-point is ignored', () => {
const basePath = _Abs('/project/node_modules');
const finder = new DirectoryWalkerEntryPointFinder(
fs, config, logger, resolver, manifest, basePath, undefined);
loadTestFiles([
...createPackage(basePath, 'some-package'),
...createPackage(fs.resolve(basePath, 'some-package'), 'sub-entry-point-1'),
...createPackage(
fs.resolve(basePath, 'some-package/sub-entry-point-1'), 'sub-entry-point-2'),
]);
spyOn(config, 'getPackageConfig').and.returnValue({
versionRange: '*',
entryPoints: {
[_Abs('/project/node_modules/some-package/sub-entry-point-1')]: {ignore: true},
},
});
const {entryPoints} = finder.findEntryPoints();
expect(dumpEntryPointPaths(basePath, entryPoints)).toEqual([
['some-package', 'some-package'],
['some-package', 'some-package/sub-entry-point-1/sub-entry-point-2'],
]);
});
it('should write an entry-point manifest file if none was found and basePath is `node_modules`',
() => {
const basePath = _Abs('/sub_entry_points/node_modules');

35
packages/compiler-cli/ngcc/test/entry_point_finder/targeted_entry_point_finder_spec.ts
View File

@ -109,6 +109,24 @@ runInEachFileSystem(() => {
expect(entryPoints).toEqual([]);
});
it('should return an empty array if the target path is an ignored entry-point', () => {
const basePath = _Abs('/project/node_modules');
const targetPath = _Abs('/project/node_modules/some-package');
const finder = new TargetedEntryPointFinder(
fs, config, logger, resolver, basePath, undefined, targetPath);
loadTestFiles(createPackage(basePath, 'some-package'));
spyOn(config, 'getPackageConfig').and.returnValue({
versionRange: '*',
entryPoints: {
[_Abs('/project/node_modules/some-package')]: {ignore: true},
},
});
const {entryPoints} = finder.findEntryPoints();
expect(entryPoints).toEqual([]);
});
it('should return an empty array if the target path is not an Angular entry-point', () => {
const targetPath = _Abs('/no_valid_entry_points/node_modules/some_package');
loadTestFiles([
@ -390,6 +408,23 @@ runInEachFileSystem(() => {
expect(finder.targetNeedsProcessingOrCleaning(['fesm2015'], true)).toBe(false);
});
it('should return false if the target path is ignored by the config', () => {
const basePath = _Abs('/project/node_modules');
const targetPath = _Abs('/project/node_modules/some-package');
const finder = new TargetedEntryPointFinder(
fs, config, logger, resolver, basePath, undefined, targetPath);
loadTestFiles(createPackage(basePath, 'some-package'));
spyOn(config, 'getPackageConfig').and.returnValue({
versionRange: '*',
entryPoints: {
[_Abs('/project/node_modules/some-package')]: {ignore: true},
},
});
expect(finder.targetNeedsProcessingOrCleaning(['fesm2015'], true)).toBe(false);
});
it('should false if the target path has no typings', () => {
const targetPath = _Abs('/no_valid_entry_points/node_modules/some_package');
loadTestFiles([

35
packages/compiler-cli/ngcc/test/integration/ngcc_spec.ts
View File

@ -1615,21 +1615,28 @@ runInEachFileSystem(() => {
loadTestFiles([
{
name: _('/ngcc.config.js'),
contents: `module.exports = { packages: {
'@angular/core': {
entryPoints: {
'./testing': {ignore: true}
},
},
'@angular/common': {
entryPoints: {
'.': {ignore: true}
},
}
}};`,
contents: `
module.exports = {
packages: {
'@angular/core': {
entryPoints: {
'./testing': {ignore: true},
},
},
'@angular/common': {
entryPoints: {
'.': {ignore: true},
'./http': {override: {fesm2015: undefined}},
},
},
},
};
`,
},
]);
mainNgcc({basePath: '/node_modules', propertiesToConsider: ['es2015']});
// We process core but not core/testing.
expect(loadPackage('@angular/core').__processed_by_ivy_ngcc__).toEqual({
module: '0.0.0-PLACEHOLDER',
@ -1638,12 +1645,14 @@ runInEachFileSystem(() => {
typings: '0.0.0-PLACEHOLDER',
});
expect(loadPackage('@angular/core/testing').__processed_by_ivy_ngcc__).toBeUndefined();
// We do not compile common but we do compile its sub-entry-points.
expect(loadPackage('@angular/common').__processed_by_ivy_ngcc__).toBeUndefined();
expect(loadPackage('@angular/common/http').__processed_by_ivy_ngcc__).toEqual({
// `fesm2015` is not processed, because the ngcc config removes it.
// fesm2015: '0.0.0-PLACEHOLDER',
module: '0.0.0-PLACEHOLDER',
es2015: '0.0.0-PLACEHOLDER',
fesm2015: '0.0.0-PLACEHOLDER',
typings: '0.0.0-PLACEHOLDER',
});
});

47
packages/compiler-cli/ngcc/test/packages/entry_point_manifest_spec.ts
View File

@ -185,6 +185,53 @@ runInEachFileSystem(() => {
const entryPoints = manifest.readEntryPointsUsingManifest(_Abs('/project/node_modules'));
expect(entryPoints).toEqual(null);
});
it('should return null if any of the entry-points are ignored by a config', () => {
fs.ensureDir(_Abs('/project/node_modules'));
fs.writeFile(_Abs('/project/yarn.lock'), 'LOCK FILE CONTENTS');
loadTestFiles([
{
name: _Abs('/project/node_modules/some_package/valid_entry_point/package.json'),
contents: createPackageJson('valid_entry_point'),
},
{
name: _Abs(
'/project/node_modules/some_package/valid_entry_point/valid_entry_point.metadata.json'),
contents: 'some meta data',
},
{
name: _Abs('/project/node_modules/some_package/ignored_entry_point/package.json'),
contents: createPackageJson('ignored_entry_point'),
},
{
name: _Abs(
'/project/node_modules/some_package/ignored_entry_point/ignored_entry_point.metadata.json'),
contents: 'some meta data',
},
]);
manifestFile.entryPointPaths.push(
[
_Abs('/project/node_modules/some_package'),
_Abs('/project/node_modules/some_package/valid_entry_point'), [], [], []
],
[
_Abs('/project/node_modules/some_package'),
_Abs('/project/node_modules/some_package/ignored_entry_point'), [], [], []
],
);
fs.writeFile(
_Abs('/project/node_modules/__ngcc_entry_points__.json'), JSON.stringify(manifestFile));
spyOn(config, 'getPackageConfig').and.returnValue({
versionRange: '*',
entryPoints: {
[_Abs('/project/node_modules/some_package/ignored_entry_point')]: {ignore: true},
},
});
const entryPoints = manifest.readEntryPointsUsingManifest(_Abs('/project/node_modules'));
expect(entryPoints).toEqual(null);
});
});
describe('writeEntryPointManifest()', () => {

12
packages/compiler-cli/ngcc/test/packages/entry_point_spec.ts
View File

@ -10,7 +10,7 @@ import {absoluteFrom, AbsoluteFsPath, FileSystem, getFileSystem} from '../../../
import {runInEachFileSystem} from '../../../src/ngtsc/file_system/testing';
import {loadTestFiles} from '../../../test/helpers';
import {NgccConfiguration} from '../../src/packages/configuration';
import {EntryPoint, EntryPointJsonProperty, getEntryPointFormat, getEntryPointInfo, INCOMPATIBLE_ENTRY_POINT, NO_ENTRY_POINT, SUPPORTED_FORMAT_PROPERTIES} from '../../src/packages/entry_point';
import {EntryPoint, EntryPointJsonProperty, getEntryPointFormat, getEntryPointInfo, IGNORED_ENTRY_POINT, INCOMPATIBLE_ENTRY_POINT, isEntryPoint, NO_ENTRY_POINT, SUPPORTED_FORMAT_PROPERTIES} from '../../src/packages/entry_point';
import {MockLogger} from '../helpers/mock_logger';
runInEachFileSystem(() => {
@ -55,7 +55,7 @@ runInEachFileSystem(() => {
});
});
it('should return `NO_ENTRY_POINT` if configured to ignore the specified entry-point', () => {
it('should return `IGNORED_ENTRY_POINT` if configured to ignore the specified entry-point', () => {
loadTestFiles([
{
name: _('/project/node_modules/some_package/valid_entry_point/package.json'),
@ -74,7 +74,7 @@ runInEachFileSystem(() => {
const entryPoint = getEntryPointInfo(
fs, config, new MockLogger(), SOME_PACKAGE,
_('/project/node_modules/some_package/valid_entry_point'));
expect(entryPoint).toBe(NO_ENTRY_POINT);
expect(entryPoint).toBe(IGNORED_ENTRY_POINT);
});
it('should override the properties on package.json if the entry-point is configured', () => {
@ -381,7 +381,7 @@ runInEachFileSystem(() => {
});
});
describe('getEntryPointFormat', () => {
describe('getEntryPointFormat()', () => {
let SOME_PACKAGE: AbsoluteFsPath;
let _: typeof absoluteFrom;
let fs: FileSystem;
@ -399,10 +399,10 @@ runInEachFileSystem(() => {
const result = getEntryPointInfo(
fs, config, new MockLogger(), SOME_PACKAGE,
_('/project/node_modules/some_package/valid_entry_point'));
if (result === NO_ENTRY_POINT || result === INCOMPATIBLE_ENTRY_POINT) {
if (!isEntryPoint(result)) {
return fail(`Expected an entry point but got ${result}`);
}
entryPoint = result as any;
entryPoint = result;
});
it('should return `esm2015` format for `fesm2015` property', () => {

10
packages/compiler-cli/ngcc/test/writing/new_entry_point_file_writer_spec.ts
View File

@ -9,7 +9,7 @@ import {absoluteFrom, FileSystem, getFileSystem, join} from '../../../src/ngtsc/
import {runInEachFileSystem} from '../../../src/ngtsc/file_system/testing';
import {loadTestFiles} from '../../../test/helpers';
import {NgccConfiguration} from '../../src/packages/configuration';
import {EntryPoint, EntryPointFormat, EntryPointJsonProperty, getEntryPointInfo, INCOMPATIBLE_ENTRY_POINT, NO_ENTRY_POINT} from '../../src/packages/entry_point';
import {EntryPoint, EntryPointFormat, EntryPointJsonProperty, getEntryPointInfo, isEntryPoint} from '../../src/packages/entry_point';
import {EntryPointBundle, makeEntryPointBundle} from '../../src/packages/entry_point_bundle';
import {FileWriter} from '../../src/writing/file_writer';
import {NewEntryPointFileWriter} from '../../src/writing/new_entry_point_file_writer';
@ -106,7 +106,7 @@ runInEachFileSystem(() => {
const config = new NgccConfiguration(fs, _('/'));
const result = getEntryPointInfo(
fs, config, logger, _('/node_modules/test'), _('/node_modules/test'))!;
if (result === NO_ENTRY_POINT || result === INCOMPATIBLE_ENTRY_POINT) {
if (!isEntryPoint(result)) {
return fail(`Expected an entry point but got ${result}`);
}
entryPoint = result;
@ -246,7 +246,7 @@ runInEachFileSystem(() => {
const config = new NgccConfiguration(fs, _('/'));
const result = getEntryPointInfo(
fs, config, logger, _('/node_modules/test'), _('/node_modules/test/a'))!;
if (result === NO_ENTRY_POINT || result === INCOMPATIBLE_ENTRY_POINT) {
if (!isEntryPoint(result)) {
return fail(`Expected an entry point but got ${result}`);
}
entryPoint = result;
@ -375,7 +375,7 @@ runInEachFileSystem(() => {
const config = new NgccConfiguration(fs, _('/'));
const result = getEntryPointInfo(
fs, config, new MockLogger(), _('/node_modules/test'), _('/node_modules/test/b'))!;
if (result === NO_ENTRY_POINT || result === INCOMPATIBLE_ENTRY_POINT) {
if (!isEntryPoint(result)) {
return fail(`Expected an entry point but got ${result}`);
}
entryPoint = result;
@ -501,7 +501,7 @@ runInEachFileSystem(() => {
const config = new NgccConfiguration(fs, _('/'));
const result = getEntryPointInfo(
fs, config, logger, _('/node_modules/test'), _('/node_modules/test'))!;
if (result === NO_ENTRY_POINT || result === INCOMPATIBLE_ENTRY_POINT) {
if (!isEntryPoint(result)) {
return fail(`Expected an entry point but got ${result}`);
}
entryPoint = result;

2
packages/compiler-cli/test/helpers/src/mock_file_loading.ts
View File

@ -37,7 +37,7 @@ export function loadStandardTestFiles(
loadFakeCore(tmpFs, basePath);
} else {
getAngularPackagesFromRunfiles().forEach(({name, pkgPath}) => {
loadTestDirectory(tmpFs, pkgPath, tmpFs.resolve('/node_modules/@angular', name));
loadTestDirectory(tmpFs, pkgPath, tmpFs.resolve(basePath, 'node_modules/@angular', name));
});
}