From 988cca7ef59df9c740b988eabb94fa084cfa5451 Mon Sep 17 00:00:00 2001 From: JoostK Date: Sat, 25 Sep 2021 16:13:48 +0200 Subject: [PATCH] fix(ngcc): do not fail for packages which correspond with `Object` members (#43589) Prior to this commit ngcc stored its package configuration in JavaScript objects, which caused the builtin `Object` members to be found as package configuration. This would subsequently crash as their shape was not as expected. This commit moves away from using raw JavaScript objects in favor of a Map. To code was refactored such that `PartiallyProcessedConfig` is now a class. Fixes #43570 PR Close #43589 --- .../ngcc/src/packages/configuration.ts | 176 +++++++++++------- .../ngcc/test/packages/configuration_spec.ts | 29 ++- 2 files changed, 131 insertions(+), 74 deletions(-) diff --git a/packages/compiler-cli/ngcc/src/packages/configuration.ts b/packages/compiler-cli/ngcc/src/packages/configuration.ts index e699515c9ea..26d15688d1e 100644 --- a/packages/compiler-cli/ngcc/src/packages/configuration.ts +++ b/packages/compiler-cli/ngcc/src/packages/configuration.ts @@ -16,11 +16,11 @@ import {PackageJsonFormatPropertiesMap} from './entry_point'; /** * The format of a project level configuration file. */ -export interface NgccProjectConfig { +export interface NgccProjectConfig { /** * The packages that are configured by this project config. */ - packages?: {[packagePath: string]: T|undefined}; + packages?: {[packagePath: string]: RawNgccPackageConfig|undefined}; /** * Options that control how locking the process is handled. */ @@ -102,7 +102,107 @@ interface VersionedPackageConfig extends RawNgccPackageConfig { versionRange: string; } -type PartiallyProcessedConfig = Required>; +/** + * The internal representation of a configuration file. Configured packages are transformed into + * `ProcessedNgccPackageConfig` when a certain version is requested. + */ +export class PartiallyProcessedConfig { + /** + * The packages that are configured by this project config, keyed by package name. + */ + packages = new Map(); + /** + * Options that control how locking the process is handled. + */ + locking: ProcessLockingConfiguration = {}; + /** + * Name of hash algorithm used to generate hashes of the configuration. + * + * Defaults to `sha256`. + */ + hashAlgorithm = 'sha256'; + + constructor(projectConfig: NgccProjectConfig) { + // locking configuration + if (projectConfig.locking !== undefined) { + this.locking = projectConfig.locking; + } + + // packages configuration + for (const packageNameAndVersion in projectConfig.packages) { + const packageConfig = projectConfig.packages[packageNameAndVersion]; + if (packageConfig) { + const [packageName, versionRange = '*'] = this.splitNameAndVersion(packageNameAndVersion); + this.addPackageConfig(packageName, {...packageConfig, versionRange}); + } + } + + // hash algorithm config + if (projectConfig.hashAlgorithm !== undefined) { + this.hashAlgorithm = projectConfig.hashAlgorithm; + } + } + + private splitNameAndVersion(packageNameAndVersion: string): [string, string|undefined] { + const versionIndex = packageNameAndVersion.lastIndexOf('@'); + // Note that > 0 is because we don't want to match @ at the start of the line + // which is what you would have with a namespaced package, e.g. `@angular/common`. + return versionIndex > 0 ? + [ + packageNameAndVersion.substring(0, versionIndex), + packageNameAndVersion.substring(versionIndex + 1), + ] : + [packageNameAndVersion, undefined]; + } + + /** + * Registers the configuration for a particular version of the provided package. + */ + private addPackageConfig(packageName: string, config: VersionedPackageConfig): void { + if (!this.packages.has(packageName)) { + this.packages.set(packageName, []); + } + this.packages.get(packageName)!.push(config); + } + + /** + * Finds the configuration for a particular version of the provided package. + */ + findPackageConfig(packageName: string, version: string|null): VersionedPackageConfig|null { + if (!this.packages.has(packageName)) { + return null; + } + + const configs = this.packages.get(packageName)!; + if (version === null) { + // The package has no version (!) - perhaps the entry-point was from a deep import, which made + // it impossible to find the package.json. + // So just return the first config that matches the package name. + return configs[0]; + } + return configs.find( + config => satisfies(version, config.versionRange, {includePrerelease: true})) ?? + null; + } + + /** + * Converts the configuration into a JSON representation that is used to compute a hash of the + * configuration. + */ + toJson(): string { + return JSON.stringify(this, (key: string, value: unknown) => { + if (value instanceof Map) { + const res: Record = {}; + for (const [k, v] of value) { + res[k] = v; + } + return res; + } else { + return value; + } + }); + } +} /** * The default configuration for ngcc. @@ -239,8 +339,8 @@ export class NgccConfiguration { readonly hashAlgorithm: string; constructor(private fs: ReadonlyFileSystem, baseDir: AbsoluteFsPath) { - this.defaultConfig = this.processProjectConfig(DEFAULT_NGCC_CONFIG); - this.projectConfig = this.processProjectConfig(this.loadProjectConfig(baseDir)); + this.defaultConfig = new PartiallyProcessedConfig(DEFAULT_NGCC_CONFIG); + this.projectConfig = new PartiallyProcessedConfig(this.loadProjectConfig(baseDir)); this.hashAlgorithm = this.projectConfig.hashAlgorithm; this.hash = this.computeHash(); } @@ -281,9 +381,7 @@ export class NgccConfiguration { return this.cache.get(cacheKey)!; } - const projectLevelConfig = this.projectConfig.packages ? - findSatisfactoryVersion(this.projectConfig.packages[packageName], version) : - null; + const projectLevelConfig = this.projectConfig.findPackageConfig(packageName, version); if (projectLevelConfig !== null) { this.cache.set(cacheKey, projectLevelConfig); return projectLevelConfig; @@ -295,9 +393,7 @@ export class NgccConfiguration { return packageLevelConfig; } - const defaultLevelConfig = this.defaultConfig.packages ? - findSatisfactoryVersion(this.defaultConfig.packages[packageName], version) : - null; + const defaultLevelConfig = this.defaultConfig.findPackageConfig(packageName, version); if (defaultLevelConfig !== null) { this.cache.set(cacheKey, defaultLevelConfig); return defaultLevelConfig; @@ -306,34 +402,6 @@ export class NgccConfiguration { return {versionRange: '*'}; } - private processProjectConfig(projectConfig: NgccProjectConfig): PartiallyProcessedConfig { - const processedConfig: - PartiallyProcessedConfig = {packages: {}, locking: {}, hashAlgorithm: 'sha256'}; - - // locking configuration - if (projectConfig.locking !== undefined) { - processedConfig.locking = projectConfig.locking; - } - - // packages configuration - for (const packageNameAndVersion in projectConfig.packages) { - const packageConfig = projectConfig.packages[packageNameAndVersion]; - if (packageConfig) { - const [packageName, versionRange = '*'] = this.splitNameAndVersion(packageNameAndVersion); - const packageConfigs = - processedConfig.packages[packageName] || (processedConfig.packages[packageName] = []); - packageConfigs!.push({...packageConfig, versionRange}); - } - } - - // hash algorithm config - if (projectConfig.hashAlgorithm !== undefined) { - processedConfig.hashAlgorithm = projectConfig.hashAlgorithm; - } - - return processedConfig; - } - private loadProjectConfig(baseDir: AbsoluteFsPath): NgccProjectConfig { const configFilePath = this.fs.join(baseDir, NGCC_CONFIG_FILENAME); if (this.fs.exists(configFilePath)) { @@ -379,35 +447,7 @@ export class NgccConfiguration { return sandbox.module.exports; } - private splitNameAndVersion(packageNameAndVersion: string): [string, string|undefined] { - const versionIndex = packageNameAndVersion.lastIndexOf('@'); - // Note that > 0 is because we don't want to match @ at the start of the line - // which is what you would have with a namespaced package, e.g. `@angular/common`. - return versionIndex > 0 ? - [ - packageNameAndVersion.substring(0, versionIndex), - packageNameAndVersion.substring(versionIndex + 1), - ] : - [packageNameAndVersion, undefined]; - } - private computeHash(): string { - return createHash(this.hashAlgorithm).update(JSON.stringify(this.projectConfig)).digest('hex'); + return createHash(this.hashAlgorithm).update(this.projectConfig.toJson()).digest('hex'); } } - -function findSatisfactoryVersion(configs: VersionedPackageConfig[]|undefined, version: string|null): - VersionedPackageConfig|null { - if (configs === undefined) { - return null; - } - if (version === null) { - // The package has no version (!) - perhaps the entry-point was from a deep import, which made - // it impossible to find the package.json. - // So just return the first config that matches the package name. - return configs[0]; - } - return configs.find( - config => satisfies(version, config.versionRange, {includePrerelease: true})) || - null; -} diff --git a/packages/compiler-cli/ngcc/test/packages/configuration_spec.ts b/packages/compiler-cli/ngcc/test/packages/configuration_spec.ts index 7003d00ba9d..cf63df31674 100644 --- a/packages/compiler-cli/ngcc/test/packages/configuration_spec.ts +++ b/packages/compiler-cli/ngcc/test/packages/configuration_spec.ts @@ -10,7 +10,7 @@ import {createHash} from 'crypto'; import {absoluteFrom, getFileSystem, ReadonlyFileSystem} from '../../../src/ngtsc/file_system'; import {runInEachFileSystem} from '../../../src/ngtsc/file_system/testing'; import {loadTestFiles} from '../../../src/ngtsc/testing'; -import {DEFAULT_NGCC_CONFIG, NgccConfiguration, NgccProjectConfig, ProcessLockingConfiguration, RawNgccPackageConfig} from '../../src/packages/configuration'; +import {DEFAULT_NGCC_CONFIG, NgccConfiguration, NgccProjectConfig, PartiallyProcessedConfig, ProcessLockingConfiguration} from '../../src/packages/configuration'; runInEachFileSystem(() => { @@ -75,10 +75,9 @@ runInEachFileSystem(() => { loadTestFiles([{name: _Abs('/project-1/empty.js'), contents: ``}]); const configuration = new NgccConfiguration(fs, _Abs('/project-1')); expect(configuration.hash) - .toEqual( - createHash('sha256') - .update(JSON.stringify({packages: {}, locking: {}, hashAlgorithm: 'sha256'})) - .digest('hex')); + .toEqual(createHash('sha256') + .update(new PartiallyProcessedConfig({}).toJson()) + .digest('hex')); }); it('should use a custom hash algorithm if specified in the config', () => { @@ -98,7 +97,7 @@ runInEachFileSystem(() => { const project1Conf = new NgccConfiguration(fs, project1); const expectedProject1Config = `{"packages":{"package-1":[{"entryPoints":{"./entry-point-1":{}},"versionRange":"*"}]},"locking":{},"hashAlgorithm":"md5"}`; - expect(JSON.stringify((project1Conf as any).projectConfig)).toEqual(expectedProject1Config); + expect((project1Conf as any).projectConfig.toJson()).toEqual(expectedProject1Config); expect(project1Conf.hash) .toEqual(createHash('md5').update(expectedProject1Config).digest('hex')); }); @@ -150,6 +149,24 @@ runInEachFileSystem(() => { })); }); + it('should handle packages with an Object member name when no config is present', () => { + loadTestFiles([ + { + name: _Abs('/project-1/node_modules/constructor/package.json'), + contents: '{"version": "1.0.0"}', + }, + ]); + + const configuration = new NgccConfiguration(fs, _Abs('/project-1')); + const config = configuration.getPackageConfig( + 'constructor', _Abs('/project-1/node_modules/constructor'), '1.0.0'); + + expect(config).toEqual(jasmine.objectContaining({ + ignorableDeepImportMatchers: [], + entryPoints: new Map(), + })); + }); + it('should read extra package config from package level file', () => { loadTestFiles(packageWithConfigFiles( 'package-1', 'entry-point-1', '1.0.0', 'ignorableDeepImportMatchers: [ /xxx/ ]'));