From 916762440c8a0933b714d73714983f2787a6b519 Mon Sep 17 00:00:00 2001 From: Pete Bacon Darwin Date: Fri, 4 Oct 2019 11:54:33 +0100 Subject: [PATCH] feat(ngcc): support fallback to a default configuration (#33008) It is now possible to include a set of default ngcc configurations that ship with ngcc out of the box. This allows ngcc to handle a set of common packages, which are unlikely to be fixed, without requiring the application developer to write their own configuration for them. Any packages that are configured at the package or project level will override these default configurations. This allows a reasonable level of control at the package and user level. PR Close #33008 --- .../ngcc/src/packages/configuration.ts | 65 ++++- .../ngcc/test/packages/configuration_spec.ts | 275 ++++++++++++------ 2 files changed, 239 insertions(+), 101 deletions(-) diff --git a/packages/compiler-cli/ngcc/src/packages/configuration.ts b/packages/compiler-cli/ngcc/src/packages/configuration.ts index cf8fd8e1c6..612f33a539 100644 --- a/packages/compiler-cli/ngcc/src/packages/configuration.ts +++ b/packages/compiler-cli/ngcc/src/packages/configuration.ts @@ -44,13 +44,50 @@ export interface NgccEntryPointConfig { override?: PackageJsonFormatPropertiesMap; } +/** + * The default configuration for ngcc. + * + * This is the ultimate fallback configuration that ngcc will use if there is no configuration + * for a package at the package level or project level. + * + * This configuration is for packages that are "dead" - i.e. no longer maintained and so are + * unlikely to be fixed to work with ngcc, nor provide a package level config of their own. + * + * The fallback process for looking up configuration is: + * + * Project -> Package -> Default + * + * If a package provides its own configuration then that would override this default one. + * + * Also application developers can always provide configuration at their project level which + * will override everything else. + * + * Note that the fallback is package based not entry-point based. + * For example, if a there is configuration for a package at the project level this will replace all + * entry-point configurations that may have been provided in the package level or default level + * configurations, even if the project level configuration does not provide for a given entry-point. + */ +export const DEFAULT_NGCC_CONFIG: NgccProjectConfig = { + packages: { + // Add default package configuration here. For example: + // '@angular/fire@^5.2.0': { + // entryPoints: { + // './database-deprecated': { + // ignore: true, + // }, + // }, + // }, + } +}; + const NGCC_CONFIG_FILENAME = 'ngcc.config.js'; export class NgccConfiguration { - // TODO: change string => ModuleSpecifier when we tighten the path types in #30556 + private defaultConfig: NgccProjectConfig; private cache = new Map(); constructor(private fs: FileSystem, baseDir: AbsoluteFsPath) { + this.defaultConfig = this.processDefaultConfig(baseDir); const projectConfig = this.loadProjectConfig(baseDir); for (const packagePath in projectConfig.packages) { const absPackagePath = resolve(baseDir, 'node_modules', packagePath); @@ -66,12 +103,26 @@ export class NgccConfiguration { return this.cache.get(packagePath) !; } - const packageConfig = this.loadPackageConfig(packagePath); - packageConfig.entryPoints = this.processEntryPoints(packagePath, packageConfig.entryPoints); + const packageConfig = this.loadPackageConfig(packagePath) || + this.defaultConfig.packages[packagePath] || {entryPoints: {}}; this.cache.set(packagePath, packageConfig); return packageConfig; } + private processDefaultConfig(baseDir: AbsoluteFsPath): NgccProjectConfig { + const defaultConfig: NgccProjectConfig = {packages: {}}; + for (const packagePath in DEFAULT_NGCC_CONFIG.packages) { + const absPackagePath = resolve(baseDir, 'node_modules', packagePath); + const packageConfig = DEFAULT_NGCC_CONFIG.packages[packagePath]; + if (packageConfig) { + packageConfig.entryPoints = + this.processEntryPoints(absPackagePath, packageConfig.entryPoints); + defaultConfig.packages[absPackagePath] = packageConfig; + } + } + return defaultConfig; + } + private loadProjectConfig(baseDir: AbsoluteFsPath): NgccProjectConfig { const configFilePath = join(baseDir, NGCC_CONFIG_FILENAME); if (this.fs.exists(configFilePath)) { @@ -85,16 +136,18 @@ export class NgccConfiguration { } } - private loadPackageConfig(packagePath: AbsoluteFsPath): NgccPackageConfig { + private loadPackageConfig(packagePath: AbsoluteFsPath): NgccPackageConfig|null { const configFilePath = join(packagePath, NGCC_CONFIG_FILENAME); if (this.fs.exists(configFilePath)) { try { - return this.evalSrcFile(configFilePath); + const packageConfig = this.evalSrcFile(configFilePath); + packageConfig.entryPoints = this.processEntryPoints(packagePath, packageConfig.entryPoints); + return packageConfig; } catch (e) { throw new Error(`Invalid package configuration file at "${configFilePath}": ` + e.message); } } else { - return {entryPoints: {}}; + return null; } } diff --git a/packages/compiler-cli/ngcc/test/packages/configuration_spec.ts b/packages/compiler-cli/ngcc/test/packages/configuration_spec.ts index 7c1fe97852..8fb42c1618 100644 --- a/packages/compiler-cli/ngcc/test/packages/configuration_spec.ts +++ b/packages/compiler-cli/ngcc/test/packages/configuration_spec.ts @@ -8,7 +8,7 @@ import {FileSystem, absoluteFrom, getFileSystem} from '../../../src/ngtsc/file_system'; import {runInEachFileSystem} from '../../../src/ngtsc/file_system/testing'; import {loadTestFiles} from '../../../test/helpers'; -import {NgccConfiguration} from '../../src/packages/configuration'; +import {DEFAULT_NGCC_CONFIG, NgccConfiguration} from '../../src/packages/configuration'; runInEachFileSystem(() => { @@ -31,65 +31,69 @@ runInEachFileSystem(() => { }); describe('getConfig()', () => { - it('should return configuration for a package found in a package level file', () => { - loadTestFiles([{ - name: _Abs('/project-1/node_modules/package-1/ngcc.config.js'), - contents: `module.exports = {entryPoints: { './entry-point-1': {}}}` - }]); - 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')); + describe('at the package level', () => { + it('should return configuration for a package found in a package level file', () => { + loadTestFiles([{ + name: _Abs('/project-1/node_modules/package-1/ngcc.config.js'), + contents: `module.exports = {entryPoints: { './entry-point-1': {}}}` + }]); + 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')); - expect(config).toEqual( - {entryPoints: {[_Abs('/project-1/node_modules/package-1/entry-point-1')]: {}}}); - expect(readFileSpy) - .toHaveBeenCalledWith(_Abs('/project-1/node_modules/package-1/ngcc.config.js')); - }); + expect(config).toEqual( + {entryPoints: {[_Abs('/project-1/node_modules/package-1/entry-point-1')]: {}}}); + expect(readFileSpy) + .toHaveBeenCalledWith(_Abs('/project-1/node_modules/package-1/ngcc.config.js')); + }); - it('should cache configuration for a package found in a package level file', () => { - loadTestFiles([{ - name: _Abs('/project-1/node_modules/package-1/ngcc.config.js'), - contents: ` + it('should used cached configuration for a package if available', () => { + loadTestFiles([{ + name: _Abs('/project-1/node_modules/package-1/ngcc.config.js'), + contents: ` module.exports = { entryPoints: { './entry-point-1': {} }, };` - }]); - const configuration = new NgccConfiguration(fs, _Abs('/project-1')); + }]); + const configuration = new NgccConfiguration(fs, _Abs('/project-1')); - // Populate the cache - configuration.getConfig(_Abs('/project-1/node_modules/package-1')); + // Populate the cache + configuration.getConfig(_Abs('/project-1/node_modules/package-1')); - const readFileSpy = spyOn(fs, 'readFile').and.callThrough(); - const config = configuration.getConfig(_Abs('/project-1/node_modules/package-1')); + const readFileSpy = spyOn(fs, 'readFile').and.callThrough(); + const config = configuration.getConfig(_Abs('/project-1/node_modules/package-1')); - expect(config).toEqual( - {entryPoints: {[_Abs('/project-1/node_modules/package-1/entry-point-1')]: {}}}); - expect(readFileSpy).not.toHaveBeenCalled(); + expect(config).toEqual( + {entryPoints: {[_Abs('/project-1/node_modules/package-1/entry-point-1')]: {}}}); + expect(readFileSpy).not.toHaveBeenCalled(); + }); + + it('should return an empty configuration object if there is no matching configuration for the package', + () => { + const configuration = new NgccConfiguration(fs, _Abs('/project-1')); + const config = configuration.getConfig(_Abs('/project-1/node_modules/package-1')); + expect(config).toEqual({entryPoints: {}}); + }); + + it('should error if a package level config file is badly formatted', () => { + loadTestFiles([{ + name: _Abs('/project-1/node_modules/package-1/ngcc.config.js'), + contents: `bad js code` + }]); + const configuration = new NgccConfiguration(fs, _Abs('/project-1')); + expect(() => configuration.getConfig(_Abs('/project-1/node_modules/package-1'))) + .toThrowError( + `Invalid package configuration file at "${_Abs('/project-1/node_modules/package-1/ngcc.config.js')}": Unexpected identifier`); + }); }); - it('should return an empty configuration object if there is no matching config file', () => { - const configuration = new NgccConfiguration(fs, _Abs('/project-1')); - const config = configuration.getConfig(_Abs('/project-1/node_modules/package-1')); - expect(config).toEqual({entryPoints: {}}); - }); - - it('should error if a package level config file is badly formatted', () => { - loadTestFiles([{ - name: _Abs('/project-1/node_modules/package-1/ngcc.config.js'), - contents: `bad js code` - }]); - const configuration = new NgccConfiguration(fs, _Abs('/project-1')); - expect(() => configuration.getConfig(_Abs('/project-1/node_modules/package-1'))) - .toThrowError( - `Invalid package configuration file at "${_Abs('/project-1/node_modules/package-1/ngcc.config.js')}": Unexpected identifier`); - }); - - it('should return configuration for a package found in a project level file', () => { - loadTestFiles([{ - name: _Abs('/project-1/ngcc.config.js'), - contents: ` + describe('at the project level', () => { + it('should return configuration for a package found in a project level file', () => { + loadTestFiles([{ + name: _Abs('/project-1/ngcc.config.js'), + contents: ` module.exports = { packages: { 'package-1': { @@ -99,21 +103,21 @@ runInEachFileSystem(() => { }, }, };` - }]); - const readFileSpy = spyOn(fs, 'readFile').and.callThrough(); - const configuration = new NgccConfiguration(fs, _Abs('/project-1')); - expect(readFileSpy).toHaveBeenCalledWith(_Abs('/project-1/ngcc.config.js')); + }]); + const readFileSpy = spyOn(fs, 'readFile').and.callThrough(); + const configuration = new NgccConfiguration(fs, _Abs('/project-1')); + expect(readFileSpy).toHaveBeenCalledWith(_Abs('/project-1/ngcc.config.js')); - const config = configuration.getConfig(_Abs('/project-1/node_modules/package-1')); - expect(config).toEqual( - {entryPoints: {[_Abs('/project-1/node_modules/package-1/entry-point-1')]: {}}}); - }); + const config = configuration.getConfig(_Abs('/project-1/node_modules/package-1')); + expect(config).toEqual( + {entryPoints: {[_Abs('/project-1/node_modules/package-1/entry-point-1')]: {}}}); + }); - it('should override package level config with project level config per package', () => { - loadTestFiles([ - { - name: _Abs('/project-1/ngcc.config.js'), - contents: ` + it('should override package level config with project level config per package', () => { + loadTestFiles([ + { + name: _Abs('/project-1/ngcc.config.js'), + contents: ` module.exports = { packages: { 'package-2': { @@ -123,45 +127,126 @@ runInEachFileSystem(() => { }, }, };`, - }, - { + }, + { + name: _Abs('/project-1/node_modules/package-1/ngcc.config.js'), + contents: ` + module.exports = { + entryPoints: { + './package-setting-entry-point': {} + }, + };`, + }, + { + name: _Abs('/project-1/node_modules/package-2/ngcc.config.js'), + contents: ` + module.exports = { + entryPoints: { + './package-setting-entry-point': {} + }, + };`, + } + ]); + const readFileSpy = spyOn(fs, 'readFile').and.callThrough(); + const configuration = new NgccConfiguration(fs, _Abs('/project-1')); + expect(readFileSpy).toHaveBeenCalledWith(_Abs('/project-1/ngcc.config.js')); + + const package1Config = configuration.getConfig(_Abs('/project-1/node_modules/package-1')); + expect(package1Config).toEqual({ + entryPoints: + {[_Abs('/project-1/node_modules/package-1/package-setting-entry-point')]: {}} + }); + expect(readFileSpy) + .toHaveBeenCalledWith(_Abs('/project-1/node_modules/package-1/ngcc.config.js')); + + // Note that for `package-2` 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. + const package2Config = configuration.getConfig(_Abs('/project-1/node_modules/package-2')); + expect(package2Config).toEqual({ + entryPoints: + {[_Abs('/project-1/node_modules/package-2/project-setting-entry-point')]: {}} + }); + expect(readFileSpy) + .not.toHaveBeenCalledWith(_Abs('/project-1/node_modules/package-2/ngcc.config.js')); + }); + }); + + describe('at the default level', () => { + const originalDefaultConfig = DEFAULT_NGCC_CONFIG.packages['package-1']; + beforeEach(() => { + DEFAULT_NGCC_CONFIG.packages['package-1'] = { + entryPoints: {'./default-level-entry-point': {}}, + }; + }); + afterEach(() => { DEFAULT_NGCC_CONFIG.packages['package-1'] = originalDefaultConfig; }); + + it('should return configuration for a package found in the default config', () => { + + const readFileSpy = spyOn(fs, 'readFile').and.callThrough(); + const configuration = new NgccConfiguration(fs, _Abs('/project-1')); + expect(readFileSpy).not.toHaveBeenCalled(); + + const config = configuration.getConfig(_Abs('/project-1/node_modules/package-1')); + expect(config).toEqual({ + entryPoints: + {[_Abs('/project-1/node_modules/package-1/default-level-entry-point')]: {}} + }); + }); + + it('should override default level config with package level config, if provided', () => { + loadTestFiles([{ name: _Abs('/project-1/node_modules/package-1/ngcc.config.js'), contents: ` - module.exports = { - entryPoints: { - './package-setting-entry-point': {} - }, - };`, - }, - { - name: _Abs('/project-1/node_modules/package-2/ngcc.config.js'), - contents: ` - module.exports = { - entryPoints: { - './package-setting-entry-point': {} - }, - };`, - } - ]); - const readFileSpy = spyOn(fs, 'readFile').and.callThrough(); - const configuration = new NgccConfiguration(fs, _Abs('/project-1')); - expect(readFileSpy).toHaveBeenCalledWith(_Abs('/project-1/ngcc.config.js')); - - const package1Config = configuration.getConfig(_Abs('/project-1/node_modules/package-1')); - expect(package1Config).toEqual({ - entryPoints: - {[_Abs('/project-1/node_modules/package-1/package-setting-entry-point')]: {}} + module.exports = { + entryPoints: {'./package-level-entry-point': {}}, + };`, + }]); + const configuration = new NgccConfiguration(fs, _Abs('/project-1')); + const config = configuration.getConfig(_Abs('/project-1/node_modules/package-1')); + // 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. + expect(config).toEqual({ + entryPoints: + {[_Abs('/project-1/node_modules/package-1/package-level-entry-point')]: {}} + }); }); - expect(readFileSpy) - .toHaveBeenCalledWith(_Abs('/project-1/node_modules/package-1/ngcc.config.js')); - const package2Config = configuration.getConfig(_Abs('/project-1/node_modules/package-2')); - expect(package2Config).toEqual({ - entryPoints: - {[_Abs('/project-1/node_modules/package-2/project-setting-entry-point')]: {}} + it('should override default level config with project level config, if provided', () => { + loadTestFiles([ + { + name: _Abs('/project-1/node_modules/package-1/ngcc.config.js'), + contents: ` + module.exports = { + entryPoints: {'./package-level-entry-point': {}}, + };`, + }, + { + name: _Abs('/project-1/ngcc.config.js'), + contents: ` + module.exports = { + packages: { + 'package-1': { + entryPoints: { + './project-level-entry-point': {} + }, + }, + }, + };`, + }, + ]); + + const configuration = new NgccConfiguration(fs, _Abs('/project-1')); + const config = configuration.getConfig(_Abs('/project-1/node_modules/package-1')); + // 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. + expect(config).toEqual({ + entryPoints: + {[_Abs('/project-1/node_modules/package-1/project-level-entry-point')]: {}} + }); }); - expect(readFileSpy) - .not.toHaveBeenCalledWith(_Abs('/project-1/node_modules/package-2/ngcc.config.js')); }); }); });