From 829ddf95e59aa507ffeaeb65101321407356a67f Mon Sep 17 00:00:00 2001 From: George Kalpakas Date: Mon, 8 Jun 2020 22:04:33 +0300 Subject: [PATCH] fix(ngcc): correctly retrieve a package's version from its `package.json` (#37040) In order to retrieve the ngcc configuration (if any) for an entry-point, ngcc has to detect the containing package's version. Previously, ngcc would try to read the version from the entry-point's `package.json` file, which was different than the package's top-level `package.json` for secondary entry-points. For example, it would try to read it from `node_modules/@angular/common/http/package.json` for entry-point `@angular/common/http`. However, the `package.json` files for secondary entry-points are not guaranteed to include a `version` property. This commit fixes this by first trying to read the version from the _package's_ `package.json` (falling back to the entry-point's `package.json`). For example, it will first try to read it from `@angular/common/package.json` for entry-point `@angular/common/http`. PR Close #37040 --- .../ngcc/src/packages/entry_point.ts | 27 ++-- .../ngcc/test/packages/entry_point_spec.ts | 116 ++++++++++++++++-- 2 files changed, 118 insertions(+), 25 deletions(-) diff --git a/packages/compiler-cli/ngcc/src/packages/entry_point.ts b/packages/compiler-cli/ngcc/src/packages/entry_point.ts index 1384d2ee1a..31d59cc97d 100644 --- a/packages/compiler-cli/ngcc/src/packages/entry_point.ts +++ b/packages/compiler-cli/ngcc/src/packages/entry_point.ts @@ -125,21 +125,25 @@ export type GetEntryPointResult = export function getEntryPointInfo( fs: FileSystem, config: NgccConfiguration, logger: Logger, packagePath: AbsoluteFsPath, entryPointPath: AbsoluteFsPath): GetEntryPointResult { - const packageJsonPath = resolve(entryPointPath, 'package.json'); - const loadedEntryPointPackageJson = loadPackageJson(fs, packageJsonPath); - const packageVersion = getPackageVersion(loadedEntryPointPackageJson); + const packagePackageJsonPath = resolve(packagePath, 'package.json'); + const entryPointPackageJsonPath = resolve(entryPointPath, 'package.json'); + const loadedPackagePackageJson = loadPackageJson(fs, packagePackageJsonPath); + const loadedEntryPointPackageJson = (packagePackageJsonPath === entryPointPackageJsonPath) ? + loadedPackagePackageJson : + loadPackageJson(fs, entryPointPackageJsonPath); + const packageVersion = getPackageVersion(loadedPackagePackageJson); const entryPointConfig = config.getPackageConfig(packagePath, packageVersion).entryPoints[entryPointPath]; let entryPointPackageJson: EntryPointPackageJson; if (entryPointConfig === undefined) { - if (!fs.exists(packageJsonPath)) { + if (!fs.exists(entryPointPackageJsonPath)) { // No `package.json` and no config. return NO_ENTRY_POINT; } else if (loadedEntryPointPackageJson === null) { // `package.json` exists but could not be parsed and there is no redeeming config. - logger.warn( - `Failed to read entry point info from invalid 'package.json' file: ${packageJsonPath}`); + logger.warn(`Failed to read entry point info from invalid 'package.json' file: ${ + entryPointPackageJsonPath}`); return INCOMPATIBLE_ENTRY_POINT; } else { @@ -302,13 +306,12 @@ function guessTypingsFromPackageJson( /** * Find the version of the package at `packageJsonPath`. * - * The version is read off of the `version` property of `packageJson`. It is assumed that even if - * `packageJson` corresponds to a secondary entry-point (e.g. `@angular/common/http`) it will still - * contain the same version as the main package (e.g. `@angular/common`). + * The version is read off of the `version` property of the package's `package.json` file (if + * available). * - * @param packageJson the parsed `package.json` of the package or one of its entry-points (if - * available). - * @returns the version string or `null` if the `package.json` is not available. + * @param packageJson the parsed `package.json` of the package (if available). + * @returns the version string or `null` if the `pckage.json` file is missing or doesn't contain a + * version. */ function getPackageVersion(packageJson: EntryPointPackageJson|null): string|null { return packageJson?.version ?? null; 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 3e0ad554b3..7c53a437a5 100644 --- a/packages/compiler-cli/ngcc/test/packages/entry_point_spec.ts +++ b/packages/compiler-cli/ngcc/test/packages/entry_point_spec.ts @@ -6,7 +6,7 @@ * found in the LICENSE file at https://angular.io/license */ -import {absoluteFrom, AbsoluteFsPath, FileSystem, getFileSystem} from '../../../src/ngtsc/file_system'; +import {absoluteFrom, AbsoluteFsPath, FileSystem, getFileSystem, join} from '../../../src/ngtsc/file_system'; import {runInEachFileSystem} from '../../../src/ngtsc/file_system/testing'; import {loadTestFiles} from '../../../test/helpers'; import {NgccConfiguration} from '../../src/packages/configuration'; @@ -77,6 +77,92 @@ runInEachFileSystem(() => { expect(entryPoint).toBe(IGNORED_ENTRY_POINT); }); + it('should retrieve the entry-point\'s version from the package\'s `package.json`', () => { + const entryPointPath = join(SOME_PACKAGE, 'valid_entry_point'); + + loadTestFiles([ + { + name: _('/project/ngcc.config.js'), + contents: ` + module.exports = { + packages: { + 'some_package@3': { + entryPoints: {valid_entry_point: {override: {packageVersion: '3'}}}, + }, + 'some_package@2': { + entryPoints: {valid_entry_point: {override: {packageVersion: '2'}}}, + }, + 'some_package@1': { + entryPoints: {valid_entry_point: {override: {packageVersion: '1'}}}, + }, + }, + }; + `, + }, + { + name: join(SOME_PACKAGE, 'package.json'), + contents: createPackageJson('', {version: '1.0.0'}), + }, + { + name: join(entryPointPath, 'package.json'), + contents: createPackageJson('valid_entry_point', {version: '2.0.0'}), + }, + { + name: join(entryPointPath, 'valid_entry_point.metadata.json'), + contents: 'some meta data', + }, + ]); + + const config = new NgccConfiguration(fs, _('/project')); + const info: EntryPoint = + getEntryPointInfo(fs, config, new MockLogger(), SOME_PACKAGE, entryPointPath) as any; + + expect(info.packageJson).toEqual(jasmine.objectContaining({packageVersion: '1'})); + }); + + it('should use `null` for version if it cannot be retrieved from a `package.json`', () => { + const entryPointPath = join(SOME_PACKAGE, 'valid_entry_point'); + + loadTestFiles([ + { + name: _('/project/ngcc.config.js'), + contents: ` + module.exports = { + packages: { + 'some_package@3': { + entryPoints: {valid_entry_point: {override: {packageVersion: '3'}}}, + }, + 'some_package@2': { + entryPoints: {valid_entry_point: {override: {packageVersion: '2'}}}, + }, + 'some_package@1': { + entryPoints: {valid_entry_point: {override: {packageVersion: '1'}}}, + }, + }, + }; + `, + }, + { + name: join(SOME_PACKAGE, 'package.json'), + contents: createPackageJson(''), + }, + { + name: join(entryPointPath, 'package.json'), + contents: createPackageJson('valid_entry_point'), + }, + { + name: join(entryPointPath, 'valid_entry_point.metadata.json'), + contents: 'some meta data', + }, + ]); + + const config = new NgccConfiguration(fs, _('/project')); + const info: EntryPoint = + getEntryPointInfo(fs, config, new MockLogger(), SOME_PACKAGE, entryPointPath) as any; + + expect(info.packageJson).toEqual(jasmine.objectContaining({packageVersion: '3'})); + }); + it('should override the properties on package.json if the entry-point is configured', () => { loadTestFiles([ { @@ -514,19 +600,23 @@ runInEachFileSystem(() => { }); export function createPackageJson( - packageName: string, - {excludes, typingsProp = 'typings', typingsIsArray}: - {excludes?: string[], typingsProp?: string, typingsIsArray?: boolean} = {}): string { + entryPointName: string, {excludes, typingsProp = 'typings', typingsIsArray, version}: { + excludes?: string[], + typingsProp?: string, + typingsIsArray?: boolean, + version?: string + } = {}): string { const packageJson: any = { - name: `some_package/${packageName}`, - [typingsProp]: typingsIsArray ? [`./${packageName}.d.ts`] : `./${packageName}.d.ts`, - fesm2015: `./fesm2015/${packageName}.js`, - esm2015: `./esm2015/${packageName}.js`, - es2015: `./es2015/${packageName}.js`, - fesm5: `./fesm5/${packageName}.js`, - esm5: `./esm5/${packageName}.js`, - main: `./bundles/${packageName}/index.js`, - browser: `./bundles/${packageName}/index.js`, + name: (entryPointName === '') ? 'some_package' : `some_package/${entryPointName}`, + version, + [typingsProp]: typingsIsArray ? [`./${entryPointName}.d.ts`] : `./${entryPointName}.d.ts`, + fesm2015: `./fesm2015/${entryPointName}.js`, + esm2015: `./esm2015/${entryPointName}.js`, + es2015: `./es2015/${entryPointName}.js`, + fesm5: `./fesm5/${entryPointName}.js`, + esm5: `./esm5/${entryPointName}.js`, + main: `./bundles/${entryPointName}/index.js`, + browser: `./bundles/${entryPointName}/index.js`, module: './index.js', }; if (excludes) {