From 98931bf9b586712eac64ef11a61587c2ab18d373 Mon Sep 17 00:00:00 2001 From: Pete Bacon Darwin Date: Tue, 28 Apr 2020 14:39:27 +0100 Subject: [PATCH] refactor(ngcc): rename `Configuration.getConfig()` (#36838) Strictly this method only returns config for packages. So this commit renames it to `getPackageConfig()`, which frees us up to add other "getXxxxConfig()` methods later. PR Close #36838 --- .../src/dependencies/dependency_resolver.ts | 2 +- .../ngcc/src/packages/configuration.ts | 2 +- .../ngcc/src/packages/entry_point.ts | 2 +- .../ngcc/test/packages/configuration_spec.ts | 51 +++++++++++-------- .../ngcc/test/packages/entry_point_spec.ts | 8 +-- 5 files changed, 37 insertions(+), 28 deletions(-) diff --git a/packages/compiler-cli/ngcc/src/dependencies/dependency_resolver.ts b/packages/compiler-cli/ngcc/src/dependencies/dependency_resolver.ts index 06e544791d..b0b2021255 100644 --- a/packages/compiler-cli/ngcc/src/dependencies/dependency_resolver.ts +++ b/packages/compiler-cli/ngcc/src/dependencies/dependency_resolver.ts @@ -225,7 +225,7 @@ export class DependencyResolver { private filterIgnorableDeepImports(entryPoint: EntryPoint, deepImports: Set): AbsoluteFsPath[] { const version = (entryPoint.packageJson.version || null) as string | null; - const packageConfig = this.config.getConfig(entryPoint.package, version); + const packageConfig = this.config.getPackageConfig(entryPoint.package, version); const matchers = packageConfig.ignorableDeepImportMatchers || []; return Array.from(deepImports) .filter(deepImport => !matchers.some(matcher => matcher.test(deepImport))); diff --git a/packages/compiler-cli/ngcc/src/packages/configuration.ts b/packages/compiler-cli/ngcc/src/packages/configuration.ts index 5b718f7ca2..850fb5e834 100644 --- a/packages/compiler-cli/ngcc/src/packages/configuration.ts +++ b/packages/compiler-cli/ngcc/src/packages/configuration.ts @@ -177,7 +177,7 @@ export class NgccConfiguration { * @param version The version of the package whose config we want, or `null` if the package's * package.json did not exist or was invalid. */ - getConfig(packagePath: AbsoluteFsPath, version: string|null): VersionedPackageConfig { + getPackageConfig(packagePath: AbsoluteFsPath, version: string|null): VersionedPackageConfig { const cacheKey = packagePath + (version !== null ? `@${version}` : ''); if (this.cache.has(cacheKey)) { return this.cache.get(cacheKey)!; diff --git a/packages/compiler-cli/ngcc/src/packages/entry_point.ts b/packages/compiler-cli/ngcc/src/packages/entry_point.ts index 1c6e56fa4f..6ff336a2e0 100644 --- a/packages/compiler-cli/ngcc/src/packages/entry_point.ts +++ b/packages/compiler-cli/ngcc/src/packages/entry_point.ts @@ -121,7 +121,7 @@ export function getEntryPointInfo( const packageJsonPath = resolve(entryPointPath, 'package.json'); const packageVersion = getPackageVersion(fs, packageJsonPath); const entryPointConfig = - config.getConfig(packagePath, packageVersion).entryPoints[entryPointPath]; + config.getPackageConfig(packagePath, packageVersion).entryPoints[entryPointPath]; const hasConfig = entryPointConfig !== undefined; if (!hasConfig && !fs.exists(packageJsonPath)) { diff --git a/packages/compiler-cli/ngcc/test/packages/configuration_spec.ts b/packages/compiler-cli/ngcc/test/packages/configuration_spec.ts index 3be81f0fd8..eebdf6c06b 100644 --- a/packages/compiler-cli/ngcc/test/packages/configuration_spec.ts +++ b/packages/compiler-cli/ngcc/test/packages/configuration_spec.ts @@ -84,7 +84,7 @@ runInEachFileSystem(() => { }); }); - describe('getConfig()', () => { + describe('getPackageConfig()', () => { describe('at the package level', () => { it('should return configuration for a package found in a package level file, with a matching version', () => { @@ -92,7 +92,7 @@ runInEachFileSystem(() => { const readFileSpy = spyOn(fs, 'readFile').and.callThrough(); const configuration = new NgccConfiguration(fs, _Abs('/project-1')); const config = - configuration.getConfig(_Abs('/project-1/node_modules/package-1'), '1.0.0'); + configuration.getPackageConfig(_Abs('/project-1/node_modules/package-1'), '1.0.0'); expect(config).toEqual({ versionRange: '1.0.0', @@ -107,7 +107,7 @@ runInEachFileSystem(() => { 'package-1', 'entry-point-1', '1.0.0', 'ignorableDeepImportMatchers: [ /xxx/ ]')); const configuration = new NgccConfiguration(fs, _Abs('/project-1')); const config = - configuration.getConfig(_Abs('/project-1/node_modules/package-1'), '1.0.0'); + configuration.getPackageConfig(_Abs('/project-1/node_modules/package-1'), '1.0.0'); expect(config).toEqual({ versionRange: '1.0.0', @@ -121,11 +121,11 @@ runInEachFileSystem(() => { const configuration = new NgccConfiguration(fs, _Abs('/project-1')); // Populate the cache - configuration.getConfig(_Abs('/project-1/node_modules/package-1'), '1.0.0'); + configuration.getPackageConfig(_Abs('/project-1/node_modules/package-1'), '1.0.0'); const readFileSpy = spyOn(fs, 'readFile').and.callThrough(); const config = - configuration.getConfig(_Abs('/project-1/node_modules/package-1'), '1.0.0'); + configuration.getPackageConfig(_Abs('/project-1/node_modules/package-1'), '1.0.0'); expect(config).toEqual({ versionRange: '1.0.0', @@ -139,7 +139,7 @@ runInEachFileSystem(() => { loadTestFiles(packageWithConfigFiles('package-2', 'entry-point-1', '1.0.0')); const configuration = new NgccConfiguration(fs, _Abs('/project-1')); const config = - configuration.getConfig(_Abs('/project-1/node_modules/package-1'), '1.0.0'); + configuration.getPackageConfig(_Abs('/project-1/node_modules/package-1'), '1.0.0'); expect(config).toEqual({versionRange: '*', entryPoints: {}}); }); @@ -149,7 +149,9 @@ runInEachFileSystem(() => { contents: `bad js code` }]); const configuration = new NgccConfiguration(fs, _Abs('/project-1')); - expect(() => configuration.getConfig(_Abs('/project-1/node_modules/package-1'), '1.0.0')) + expect( + () => configuration.getPackageConfig( + _Abs('/project-1/node_modules/package-1'), '1.0.0')) .toThrowError(`Invalid package configuration file at "${ _Abs( '/project-1/node_modules/package-1/ngcc.config.js')}": Unexpected identifier`); @@ -174,7 +176,7 @@ runInEachFileSystem(() => { }]); const configuration = new NgccConfiguration(fs, _Abs('/project-1')); const config = - configuration.getConfig(_Abs('/project-1/node_modules/package-1'), '1.0.0'); + configuration.getPackageConfig(_Abs('/project-1/node_modules/package-1'), '1.0.0'); expect(config).toEqual({ versionRange: '*', entryPoints: {[_Abs('/project-1/node_modules/package-1/entry-point-1')]: {}}, @@ -209,22 +211,26 @@ runInEachFileSystem(() => { }]); const configuration = new NgccConfiguration(fs, _Abs('/project-1')); - expect(configuration.getConfig(_Abs('/project-1/node_modules/package-1'), '1.0.0')) + expect( + configuration.getPackageConfig(_Abs('/project-1/node_modules/package-1'), '1.0.0')) .toEqual({ versionRange: '1.0.0', entryPoints: {[_Abs('/project-1/node_modules/package-1/entry-point-1')]: {}} }); - expect(configuration.getConfig(_Abs('/project-1/node_modules/package-1'), '2.5.0')) + expect( + configuration.getPackageConfig(_Abs('/project-1/node_modules/package-1'), '2.5.0')) .toEqual({ versionRange: '2.*', entryPoints: {[_Abs('/project-1/node_modules/package-1/entry-point-2')]: {}} }); - expect(configuration.getConfig(_Abs('/project-1/node_modules/package-1'), '3.2.5')) + expect( + configuration.getPackageConfig(_Abs('/project-1/node_modules/package-1'), '3.2.5')) .toEqual({ versionRange: '^3.2.0', entryPoints: {[_Abs('/project-1/node_modules/package-1/entry-point-3')]: {}} }); - expect(configuration.getConfig(_Abs('/project-1/node_modules/package-1'), '4.0.0')) + expect( + configuration.getPackageConfig(_Abs('/project-1/node_modules/package-1'), '4.0.0')) .toEqual({versionRange: '*', entryPoints: {}}); }); @@ -258,7 +264,8 @@ runInEachFileSystem(() => { const configuration = new NgccConfiguration(fs, _Abs('/project-1')); const getConfig = (packageName: string, version: string|null) => - configuration.getConfig(_Abs(`/project-1/node_modules/${packageName}`), version); + configuration.getPackageConfig( + _Abs(`/project-1/node_modules/${packageName}`), version); // Default version range: * expect(getConfig('package-1', '1.0.0-beta.2')) @@ -352,7 +359,8 @@ runInEachFileSystem(() => { }]); const configuration = new NgccConfiguration(fs, _Abs('/project-1')); - expect(configuration.getConfig(_Abs('/project-1/node_modules/@angular/common'), '1.0.0')) + expect(configuration.getPackageConfig( + _Abs('/project-1/node_modules/@angular/common'), '1.0.0')) .toEqual({ versionRange: '*', entryPoints: {[_Abs('/project-1/node_modules/@angular/common')]: {}} @@ -383,7 +391,7 @@ runInEachFileSystem(() => { expect(readFileSpy).toHaveBeenCalledWith(_Abs('/project-1/ngcc.config.js')); const package1Config = - configuration.getConfig(_Abs('/project-1/node_modules/package-1'), '1.0.0'); + configuration.getPackageConfig(_Abs('/project-1/node_modules/package-1'), '1.0.0'); expect(package1Config).toEqual({ versionRange: '1.0.0', entryPoints: @@ -396,7 +404,7 @@ runInEachFileSystem(() => { // This is because overriding happens for packages as a whole and there is no attempt to // merge entry-points. const package2Config = - configuration.getConfig(_Abs('/project-1/node_modules/package-2'), '1.0.0'); + configuration.getPackageConfig(_Abs('/project-1/node_modules/package-2'), '1.0.0'); expect(package2Config).toEqual({ versionRange: '*', entryPoints: @@ -410,7 +418,7 @@ runInEachFileSystem(() => { describe('at the default level', () => { const originalDefaultConfig = JSON.stringify(DEFAULT_NGCC_CONFIG.packages); beforeEach(() => { - DEFAULT_NGCC_CONFIG.packages['package-1'] = { + DEFAULT_NGCC_CONFIG.packages!['package-1'] = { entryPoints: {'./default-level-entry-point': {}}, }; }); @@ -422,7 +430,7 @@ runInEachFileSystem(() => { expect(readFileSpy).not.toHaveBeenCalled(); const config = - configuration.getConfig(_Abs('/project-1/node_modules/package-1'), '1.0.0'); + configuration.getPackageConfig(_Abs('/project-1/node_modules/package-1'), '1.0.0'); expect(config).toEqual({ versionRange: '*', entryPoints: {[_Abs('/project-1/node_modules/package-1/default-level-entry-point')]: {}} @@ -433,7 +441,7 @@ runInEachFileSystem(() => { loadTestFiles(packageWithConfigFiles('package-1', 'package-level-entry-point', '1.0.0')); const configuration = new NgccConfiguration(fs, _Abs('/project-1')); const config = - configuration.getConfig(_Abs('/project-1/node_modules/package-1'), '1.0.0'); + configuration.getPackageConfig(_Abs('/project-1/node_modules/package-1'), '1.0.0'); // Note that only the package-level-entry-point is left. // This is because overriding happens for packages as a whole and there is no attempt to // merge entry-points. @@ -470,7 +478,7 @@ runInEachFileSystem(() => { const configuration = new NgccConfiguration(fs, _Abs('/project-1')); const config = - configuration.getConfig(_Abs('/project-1/node_modules/package-1'), '1.0.0'); + configuration.getPackageConfig(_Abs('/project-1/node_modules/package-1'), '1.0.0'); // Note that only the project-level-entry-point is left. // This is because overriding happens for packages as a whole and there is no attempt to // merge entry-points. @@ -501,7 +509,8 @@ runInEachFileSystem(() => { const configuration = new NgccConfiguration(fs, _Abs('/project-1')); const getConfig = (packageName: string, version: string|null) => - configuration.getConfig(_Abs(`/project-1/node_modules/${packageName}`), version); + configuration.getPackageConfig( + _Abs(`/project-1/node_modules/${packageName}`), version); // Default version range: * expect(getConfig('package-1', '1.0.0-beta.2')) diff --git a/packages/compiler-cli/ngcc/test/packages/entry_point_spec.ts b/packages/compiler-cli/ngcc/test/packages/entry_point_spec.ts index 4fed2fcd40..aef13dbe65 100644 --- a/packages/compiler-cli/ngcc/test/packages/entry_point_spec.ts +++ b/packages/compiler-cli/ngcc/test/packages/entry_point_spec.ts @@ -68,7 +68,7 @@ runInEachFileSystem(() => { }, ]); const config = new NgccConfiguration(fs, _('/project')); - spyOn(config, 'getConfig').and.returnValue({ + spyOn(config, 'getPackageConfig').and.returnValue({ entryPoints: {[_('/project/node_modules/some_package/valid_entry_point')]: {ignore: true}} } as any); const entryPoint = getEntryPointInfo( @@ -94,7 +94,7 @@ runInEachFileSystem(() => { typings: './some_other.d.ts', esm2015: './some_other.js', }; - spyOn(config, 'getConfig').and.returnValue({ + spyOn(config, 'getPackageConfig').and.returnValue({ entryPoints: {[_('/project/node_modules/some_package/valid_entry_point')]: {override}}, versionRange: '*' }); @@ -145,7 +145,7 @@ runInEachFileSystem(() => { const config = new NgccConfiguration(fs, _('/project')); const override = JSON.parse(createPackageJson('missing_package_json', {excludes: ['name']})); - spyOn(config, 'getConfig').and.returnValue({ + spyOn(config, 'getPackageConfig').and.returnValue({ entryPoints: {[_('/project/node_modules/some_package/missing_package_json')]: {override}}, versionRange: '*' @@ -281,7 +281,7 @@ runInEachFileSystem(() => { // no metadata.json! ]); const config = new NgccConfiguration(fs, _('/project')); - spyOn(config, 'getConfig').and.returnValue({ + spyOn(config, 'getPackageConfig').and.returnValue({ entryPoints: {[_('/project/node_modules/some_package/missing_metadata')]: {}}, versionRange: '*' });