Skip to content

Commit

Permalink
fix(ngcc): cope with packages following APF v14+
Browse files Browse the repository at this point in the history
In PR #45405, the Angular Package Format (APF) was updated so that
secondary entry-points (such as `@angular/common/http`) do not have
their own `package.json` file, as they used to. Instead, the paths to
their various formats and types are exposed via the primary
`package.json` file's `exports` property. As an example, see the v13
[@angular/common/http/package.json][1] and compare it with the v14
[@angular/common/package.json > exports][2].

Previously, `ngcc` was not able to analyze such v14+ entry-points and
would instead error as it considered such entry-points missing.

This commit addresses the issue by detecting this situation and
synthesizing a `package.json` file for the secondary entry-points based
on the `exports` property of the primary `package.json` file. This data
is only used by `ngcc` in order to determine that the entry-point does
not need further processing, since it is already in Ivy format.

[1]: https://unpkg.com/browse/@angular/common@13.3.5/http/package.json
[2]: https://unpkg.com/browse/@angular/common@14.0.0-next.15/package.json
  • Loading branch information
gkalpak committed May 3, 2022
1 parent 5cf1459 commit d4c3a78
Show file tree
Hide file tree
Showing 9 changed files with 366 additions and 12 deletions.
24 changes: 20 additions & 4 deletions packages/compiler-cli/ngcc/src/dependencies/module_resolver.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@
*/
import {AbsoluteFsPath, ReadonlyFileSystem} from '../../../src/ngtsc/file_system';
import {PathMappings} from '../path_mappings';
import {isRelativePath, resolveFileWithPostfixes} from '../utils';
import {isRelativePath, loadJson, loadSecondaryEntryPointInfoForApfV14, resolveFileWithPostfixes} from '../utils';

/**
* This is a very cut-down implementation of the TypeScript module resolution strategy.
Expand Down Expand Up @@ -110,8 +110,8 @@ export class ModuleResolver {
* Try to resolve the `moduleName` as an external entry-point by searching the `node_modules`
* folders up the tree for a matching `.../node_modules/${moduleName}`.
*
* If a folder is found but the path does not contain a `package.json` then it is marked as a
* "deep-import".
* If a folder is found but the path is not considered an entry-point (see `isEntryPoint()`) then
* it is marked as a "deep-import".
*/
private resolveAsEntryPoint(moduleName: string, fromPath: AbsoluteFsPath): ResolvedModule|null {
let folder = fromPath;
Expand All @@ -136,9 +136,25 @@ export class ModuleResolver {
* Can we consider the given path as an entry-point to a package?
*
* This is achieved by checking for the existence of `${modulePath}/package.json`.
* If there is no `package.json`, we check whether this is an APF v14+ secondary entry-point,
* which does not have each own `package.json` but has an `exports` entry in the package's primary
* `package.json`.
*/
private isEntryPoint(modulePath: AbsoluteFsPath): boolean {
return this.fs.exists(this.fs.join(modulePath, 'package.json'));
if (this.fs.exists(this.fs.join(modulePath, 'package.json'))) {
return true;
}

const packagePath = this.findPackagePath(modulePath);
if (packagePath === null) {
return false;
}

const packagePackageJson = loadJson(this.fs, this.fs.join(packagePath, 'package.json'));
const entryPointInfoForApfV14 =
loadSecondaryEntryPointInfoForApfV14(this.fs, packagePackageJson, packagePath, modulePath);

return entryPointInfoForApfV14 !== null;
}

/**
Expand Down
78 changes: 71 additions & 7 deletions packages/compiler-cli/ngcc/src/packages/entry_point.ts
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ import ts from 'typescript';
import {AbsoluteFsPath, PathManipulation, ReadonlyFileSystem} from '../../../src/ngtsc/file_system';
import {Logger} from '../../../src/ngtsc/logging';
import {parseStatementForUmdModule} from '../host/umd_host';
import {JsonObject, loadJson, resolveFileWithPostfixes} from '../utils';
import {JsonObject, loadJson, loadSecondaryEntryPointInfoForApfV14, resolveFileWithPostfixes} from '../utils';

import {NgccConfiguration, NgccEntryPointConfig} from './configuration';

Expand Down Expand Up @@ -131,7 +131,8 @@ export function getEntryPointInfo(
const loadedPackagePackageJson = loadJson<EntryPointPackageJson>(fs, packagePackageJsonPath);
const loadedEntryPointPackageJson = (packagePackageJsonPath === entryPointPackageJsonPath) ?
loadedPackagePackageJson :
loadJson<EntryPointPackageJson>(fs, entryPointPackageJsonPath);
loadOrSynthesizeSecondaryPackageJson(
fs, packagePath, entryPointPath, entryPointPackageJsonPath, loadedPackagePackageJson);
const {packageName, packageVersion} = getPackageNameAndVersion(
fs, packagePath, loadedPackagePackageJson, loadedEntryPointPackageJson);
const repositoryUrl = getRepositoryUrl(loadedPackagePackageJson);
Expand All @@ -141,17 +142,17 @@ export function getEntryPointInfo(
let entryPointPackageJson: EntryPointPackageJson;

if (entryPointConfig === undefined) {
if (!fs.exists(entryPointPackageJsonPath)) {
// No `package.json` and no config.
if (loadedEntryPointPackageJson !== null) {
entryPointPackageJson = loadedEntryPointPackageJson;
} else if (!fs.exists(entryPointPackageJsonPath)) {
// No entry-point `package.json` or package `package.json` with exports and no config.
return NO_ENTRY_POINT;
} else if (loadedEntryPointPackageJson === null) {
} else {
// `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: ${
entryPointPackageJsonPath}`);

return INCOMPATIBLE_ENTRY_POINT;
} else {
entryPointPackageJson = loadedEntryPointPackageJson;
}
} else if (entryPointConfig.ignore === true) {
// Explicitly ignored entry-point.
Expand Down Expand Up @@ -246,6 +247,69 @@ export function getEntryPointFormat(
}
}

/**
* Parse the JSON from a secondary `package.json` file. If no such file exists, look for a
* corresponding entry in the primary `package.json` file's `exports` property (if any) and
* synthesize the JSON from that.
*
* @param packagePath The absolute path to the containing npm package.
* @param entryPointPath The absolute path to the secondary entry-point.
* @param secondaryPackageJsonPath The absolute path to the secondary `package.json` file.
* @param primaryPackageJson The parsed JSON of the primary `package.json` (or `null` if it failed
* to be loaded).
* @returns Parsed JSON (either loaded from a secondary `package.json` file or synthesized from a
* primary one) if it is valid, `null` otherwise.
*/
function loadOrSynthesizeSecondaryPackageJson(
fs: ReadonlyFileSystem, packagePath: AbsoluteFsPath, entryPointPath: AbsoluteFsPath,
secondaryPackageJsonPath: AbsoluteFsPath,
primaryPackageJson: EntryPointPackageJson|null): EntryPointPackageJson|null {
// If a secondary `package.json` exists and is valid, load and return that.
const loadedPackageJson = loadJson<EntryPointPackageJson>(fs, secondaryPackageJsonPath);
if (loadedPackageJson !== null) {
return loadedPackageJson;
}

// Try to load the entry-point info from the primary `package.json` data.
const entryPointInfo =
loadSecondaryEntryPointInfoForApfV14(fs, primaryPackageJson, packagePath, entryPointPath);
if (entryPointInfo === null) {
return null;
}

// Create a synthesized `package.json`.
//
// NOTE:
// We do not care about being able to update the synthesized `package.json` (for example, updating
// its `__processed_by_ivy_ngcc__` property), because these packages are generated with Angular
// v14+ (following the Angular Package Format v14+) and thus are already in Ivy format and do not
// require processing by `ngcc`.
const synthesizedPackageJson: EntryPointPackageJson = {
synthesized: true,
name: `${primaryPackageJson!.name}/${fs.relative(packagePath, entryPointPath)}`,
};

// Update the synthesized `package.json` with any of the supported format and types properties,
// changing paths to make them relative to the entry-point directory. This makes the synthesized
// `package.json` similar to how a `package.json` inside the entry-point directory would look
// like.
for (const prop of [...SUPPORTED_FORMAT_PROPERTIES, 'types', 'typings']) {
const packageRelativePath = entryPointInfo[prop];

if (typeof packageRelativePath === 'string') {
const absolutePath = fs.resolve(packagePath, packageRelativePath);
const entryPointRelativePath = fs.relative(entryPointPath, absolutePath);
synthesizedPackageJson[prop] =
(fs.isRooted(entryPointRelativePath) || entryPointRelativePath.startsWith('.')) ?
entryPointRelativePath :
`./${entryPointRelativePath}`;
}
}

// Return the synthesized JSON.
return synthesizedPackageJson;
}

function sniffModuleFormat(
fs: ReadonlyFileSystem, sourceFilePath: AbsoluteFsPath): EntryPointFormat|undefined {
const resolvedPath = resolveFileWithPostfixes(fs, sourceFilePath, ['', '.js', '/index.js']);
Expand Down
30 changes: 30 additions & 0 deletions packages/compiler-cli/ngcc/src/utils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -186,3 +186,33 @@ export function loadJson<T extends JsonObject = JsonObject>(
return null;
}
}

/**
* Given the parsed JSON of a `package.json` file, try to extract info for a secondary entry-point
* from the `exports` property. Such info will only be present for packages following Angular
* Package Format v14+.
*
* @param primaryPackageJson The parsed JSON of the primary `package.json` (or `null` if it failed
* to be loaded).
* @param packagePath The absolute path to the containing npm package.
* @param entryPointPath The absolute path to the secondary entry-point.
* @returns The `exports` info for the specified entry-point if it exists, `null` otherwise.
*/
export function loadSecondaryEntryPointInfoForApfV14(
fs: ReadonlyFileSystem, primaryPackageJson: JsonObject|null, packagePath: AbsoluteFsPath,
entryPointPath: AbsoluteFsPath): JsonObject|null {
// Check if primary `package.json` has been loaded and has an `exports` property.
if (primaryPackageJson?.exports === undefined) {
return null;
}

// Find the `exports` key for the secondary entry-point.
const relativeEntryPointPath = fs.relative(packagePath, entryPointPath);
const exportsKey = `./${relativeEntryPointPath}`;

// Read the data (if it exists).
const exportsData =
(primaryPackageJson.exports as JsonObject)[exportsKey] as JsonObject | undefined;

return exportsData ?? null;
}
10 changes: 10 additions & 0 deletions packages/compiler-cli/ngcc/src/writing/package_json_updater.ts
Original file line number Diff line number Diff line change
Expand Up @@ -101,6 +101,7 @@ export class PackageJsonUpdate {
*/
writeChanges(packageJsonPath: AbsoluteFsPath, parsedJson?: JsonObject): void {
this.ensureNotApplied();
this.ensureNotSynthesized(parsedJson);
this.writeChangesImpl(this.changes, packageJsonPath, parsedJson);
this.applied = true;
}
Expand All @@ -110,6 +111,15 @@ export class PackageJsonUpdate {
throw new Error('Trying to apply a `PackageJsonUpdate` that has already been applied.');
}
}

private ensureNotSynthesized(parsedJson?: JsonObject) {
if (parsedJson?.synthesized) {
// Theoretically, this should never happen, because synthesized `package.json` files should
// only be created for libraries following the Angular Package Format v14+, which means they
// should already be in Ivy format and not require processing by `ngcc`.
throw new Error('Trying to update a non-existent (synthesized) `package.json` file.');
}
}
}

/** A `PackageJsonUpdater` that writes directly to the file-system. */
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -64,6 +64,25 @@ runInEachFileSystem(() => {
name: _('/node_modules/top-package/package.json'),
contents: 'PACKAGE.JSON for top-package'
},
{
name: _('/node_modules/apf-v14-package/package.json'),
contents: `{
"name": "apf-v14-package",
"exports": {
"./second/ary": {
"main": "./second/ary/index.js"
}
}
}`,
},
{
name: _('/node_modules/apf-v14-package/index.js'),
contents: `export const type = 'primary';`,
},
{
name: _('/node_modules/apf-v14-package/second/ary/index.js'),
contents: `export const type = 'secondary';`,
},
]);
});

Expand Down Expand Up @@ -144,6 +163,17 @@ runInEachFileSystem(() => {
.toEqual(new ResolvedDeepImport(
_('/libs/local-package/node_modules/package-1/sub-folder')));
});

it('should resolve to APF v14+ secondary entry-points', () => {
const resolver = new ModuleResolver(getFileSystem());

expect(resolver.resolveModuleImport(
'apf-v14-package/second/ary', _('/libs/local-package/index.js')))
.toEqual(new ResolvedExternalModule(_('/node_modules/apf-v14-package/second/ary')));
expect(resolver.resolveModuleImport(
'apf-v14-package/second/ary', _('/libs/local-package/sub-folder/index.js')))
.toEqual(new ResolvedExternalModule(_('/node_modules/apf-v14-package/second/ary')));
});
});

describe('with mapped path external modules', () => {
Expand Down Expand Up @@ -256,6 +286,20 @@ runInEachFileSystem(() => {
'package-4/secondary-entry-point', _('/dist/package-4/index.js')))
.toEqual(new ResolvedExternalModule(_('/dist/package-4/secondary-entry-point')));
});

it('should resolve APF v14+ secondary entry-points', () => {
const resolver = new ModuleResolver(getFileSystem(), {
baseUrl: '/node_modules',
paths: {'package-42': ['apf-v14-package'], 'package-42/*': ['apf-v14-package/*']},
});

expect(resolver.resolveModuleImport(
'package-42/second/ary', _('/libs/local-package/index.js')))
.toEqual(new ResolvedExternalModule(_('/node_modules/apf-v14-package/second/ary')));
expect(resolver.resolveModuleImport(
'package-42/second/ary', _('/libs/local-package/sub-folder/index.js')))
.toEqual(new ResolvedExternalModule(_('/node_modules/apf-v14-package/second/ary')));
});
});

describe('with mapped path relative paths', () => {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -197,6 +197,14 @@ runInEachFileSystem(() => {
expect(() => update.writeChanges(_('/bar/package.json')))
.toThrowError('Trying to apply a `PackageJsonUpdate` that has already been applied.');
});

it('should throw, if trying to update a synthesized `package.json` file', () => {
const update = updater.createUpdate().addChange(['foo'], 'updated');

expect(() => update.writeChanges(_('/foo/package.json'), {
synthesized: true
})).toThrowError('Trying to update a non-existent (synthesized) `package.json` file.');
});
});
});

Expand Down

0 comments on commit d4c3a78

Please sign in to comment.