Skip to content

Commit

Permalink
fix(ngcc): do not fail for packages which correspond with Object me…
Browse files Browse the repository at this point in the history 
…mbers (#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
  • Loading branch information
@JoostK @alxhub
JoostK authored and alxhub committed Sep 27, 2021
1 parent 66fb311 commit 988cca7
Show file tree
Hide file tree
Showing 2 changed files with 131 additions and 74 deletions.
176 changes: 108 additions & 68 deletions packages/compiler-cli/ngcc/src/packages/configuration.ts
View file
Expand Up @@ -16,11 +16,11 @@ import {PackageJsonFormatPropertiesMap} from './entry_point';
/**
* The format of a project level configuration file.
*/
export interface NgccProjectConfig<T = RawNgccPackageConfig> {
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.
*/
Expand Down Expand Up @@ -102,7 +102,107 @@ interface VersionedPackageConfig extends RawNgccPackageConfig {
versionRange: string;
}

type PartiallyProcessedConfig = Required<NgccProjectConfig<VersionedPackageConfig[]>>;
/**
* 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<string, VersionedPackageConfig[]>();
/**
* 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<string, unknown> = {};
for (const [k, v] of value) {
res[k] = v;
}
return res;
} else {
return value;
}
});
}
}

/**
* The default configuration for ngcc.
Expand Down Expand Up @@ -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();
}
Expand Down Expand Up @@ -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;
Expand All @@ -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;
Expand All @@ -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)) {
Expand Down Expand Up @@ -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');
}
}

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 createHash(this.hashAlgorithm).update(this.projectConfig.toJson()).digest('hex');
}
return configs.find(
config => satisfies(version, config.versionRange, {includePrerelease: true})) ||
null;
}
29 changes: 23 additions & 6 deletions packages/compiler-cli/ngcc/test/packages/configuration_spec.ts
View file
Expand Up @@ -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(() => {
Expand Down Expand Up @@ -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', () => {
Expand All @@ -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'));
});
Expand Down Expand Up @@ -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/ ]'));
Expand Down

0 comments on commit 988cca7

Please sign in to comment.