Skip to content

Commit

Permalink
fix(ngcc): correctly retrieve a package's version from its `package.j…
Browse files Browse the repository at this point in the history
…son` (#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
  • Loading branch information
gkalpak authored and subratpalhar92 committed Aug 15, 2020
1 parent cc18220 commit c027a5e
Show file tree
Hide file tree
Showing 2 changed files with 118 additions and 25 deletions.
27 changes: 15 additions & 12 deletions packages/compiler-cli/ngcc/src/packages/entry_point.ts
Expand Up @@ -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 {
Expand Down Expand Up @@ -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;
Expand Down
116 changes: 103 additions & 13 deletions packages/compiler-cli/ngcc/test/packages/entry_point_spec.ts
Expand Up @@ -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';
Expand Down Expand Up @@ -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([
{
Expand Down Expand Up @@ -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) {
Expand Down

0 comments on commit c027a5e

Please sign in to comment.