Skip to content

refactor(@angular/cli): improve update package discovery #18610

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -115,6 +115,7 @@
"@types/pidusage": "^2.0.1",
"@types/progress": "^2.0.3",
"@types/request": "^2.47.1",
"@types/resolve": "^1.17.1",
"@types/rimraf": "^3.0.0",
"@types/semver": "^7.0.0",
"@types/universal-analytics": "^0.4.2",
Expand Down
1 change: 1 addition & 0 deletions packages/angular/cli/BUILD.bazel
Original file line number Diff line number Diff line change
Expand Up @@ -72,6 +72,7 @@ ts_library(
"@npm//@types/debug",
"@npm//@types/inquirer",
"@npm//@types/node",
"@npm//@types/resolve",
"@npm//@types/rimraf",
"@npm//@types/semver",
"@npm//@types/universal-analytics",
Expand Down
48 changes: 27 additions & 21 deletions packages/angular/cli/commands/update-impl.ts
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,12 @@ import {
fetchPackageManifest,
fetchPackageMetadata,
} from '../utilities/package-metadata';
import { PackageTreeNode, findNodeDependencies, readPackageTree } from '../utilities/package-tree';
import {
PackageTreeNode,
findPackageJson,
getProjectDependencies,
readPackageJson,
} from '../utilities/package-tree';
import { Schema as UpdateCommandSchema } from './update';

const npa = require('npm-package-arg') as (selector: string) => PackageIdentifier;
Expand Down Expand Up @@ -377,15 +382,14 @@ export class UpdateCommand extends Command<UpdateCommandSchema> {

this.logger.info('Collecting installed dependencies...');

const packageTree = await readPackageTree(this.workspace.root);
const rootDependencies = findNodeDependencies(packageTree);
const rootDependencies = await getProjectDependencies(this.workspace.root);

this.logger.info(`Found ${Object.keys(rootDependencies).length} dependencies.`);
this.logger.info(`Found ${rootDependencies.size} dependencies.`);

if (options.all) {
// 'all' option and a zero length packages have already been checked.
// Add all direct dependencies to be updated
for (const dep of Object.keys(rootDependencies)) {
for (const dep of rootDependencies.keys()) {
const packageIdentifier = npa(dep);
if (options.next) {
packageIdentifier.fetchSpec = 'next';
Expand All @@ -400,7 +404,7 @@ export class UpdateCommand extends Command<UpdateCommandSchema> {
next: options.next || false,
verbose: options.verbose || false,
packageManager: this.packageManager,
packages: options.all ? Object.keys(rootDependencies) : [],
packages: options.all ? rootDependencies.keys() : [],
});

return success ? 0 : 1;
Expand All @@ -424,8 +428,9 @@ export class UpdateCommand extends Command<UpdateCommandSchema> {
}

const packageName = packages[0].name;
const packageDependency = rootDependencies[packageName];
let packageNode = packageDependency && packageDependency.node;
const packageDependency = rootDependencies.get(packageName);
let packagePath = packageDependency?.path;
let packageNode = packageDependency?.package;
if (packageDependency && !packageNode) {
this.logger.error('Package found in package.json but is not installed.');

Expand All @@ -434,20 +439,21 @@ export class UpdateCommand extends Command<UpdateCommandSchema> {
// Allow running migrations on transitively installed dependencies
// There can technically be nested multiple versions
// TODO: If multiple, this should find all versions and ask which one to use
const child = packageTree.children.find(c => c.name === packageName);
if (child) {
packageNode = child;
const packageJson = findPackageJson(this.workspace.root, packageName);
if (packageJson) {
packagePath = path.dirname(packageJson);
packageNode = await readPackageJson(packagePath);
}
}

if (!packageNode) {
if (!packageNode || !packagePath) {
this.logger.error('Package is not installed.');

return 1;
}

const updateMetadata = packageNode.package['ng-update'];
let migrations = updateMetadata && updateMetadata.migrations;
const updateMetadata = packageNode['ng-update'];
let migrations = updateMetadata?.migrations;
if (migrations === undefined) {
this.logger.error('Package does not provide migrations.');

Expand Down Expand Up @@ -477,14 +483,14 @@ export class UpdateCommand extends Command<UpdateCommandSchema> {
}

// Check if it is a package-local location
const localMigrations = path.join(packageNode.path, migrations);
const localMigrations = path.join(packagePath, migrations);
if (fs.existsSync(localMigrations)) {
migrations = localMigrations;
} else {
// Try to resolve from package location.
// This avoids issues with package hoisting.
try {
migrations = require.resolve(migrations, { paths: [packageNode.path] });
migrations = require.resolve(migrations, { paths: [packagePath] });
} catch (e) {
if (e.code === 'MODULE_NOT_FOUND') {
this.logger.error('Migrations for package were not found.');
Expand Down Expand Up @@ -513,7 +519,7 @@ export class UpdateCommand extends Command<UpdateCommandSchema> {
}

const migrationRange = new semver.Range(
'>' + from + ' <=' + (options.to || packageNode.package.version),
'>' + from + ' <=' + (options.to || packageNode.version),
);

success = await this.executeMigrations(
Expand All @@ -529,7 +535,7 @@ export class UpdateCommand extends Command<UpdateCommandSchema> {
packageName === '@angular/core'
&& options.from
&& +options.from.split('.')[0] < 9
&& (options.to || packageNode.package.version).split('.')[0] === '9'
&& (options.to || packageNode.version).split('.')[0] === '9'
) {
this.logger.info(NG_VERSION_9_POST_MSG);
}
Expand All @@ -547,8 +553,8 @@ export class UpdateCommand extends Command<UpdateCommandSchema> {

// Validate packages actually are part of the workspace
for (const pkg of packages) {
const node = rootDependencies[pkg.name] && rootDependencies[pkg.name].node;
if (!node) {
const node = rootDependencies.get(pkg.name);
if (!node?.package) {
this.logger.error(`Package '${pkg.name}' is not a dependency.`);

return 1;
Expand Down Expand Up @@ -627,7 +633,7 @@ export class UpdateCommand extends Command<UpdateCommandSchema> {
return 1;
}

if (manifest.version === node.package.version) {
if (manifest.version === node.package?.version) {
this.logger.info(`Package '${packageName}' is already up to date.`);
continue;
}
Expand Down
4 changes: 2 additions & 2 deletions packages/angular/cli/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,7 @@
"npm-pick-manifest": "6.1.0",
"open": "7.2.0",
"pacote": "9.5.12",
"read-package-tree": "5.3.1",
"resolve": "1.17.0",
"rimraf": "3.0.2",
"semver": "7.3.2",
"symbol-observable": "1.2.0",
Expand All @@ -50,7 +50,7 @@
"migrations": "@schematics/angular/migrations/migration-collection.json",
"packageGroup": {
"@angular/cli": "0.0.0",
"@angular-devkit/build-angular": "0.0.0",
"@angular-devkit/build-angular": "0.0.0",
"@angular-devkit/build-ng-packagr": "0.0.0",
"@angular-devkit/build-webpack": "0.0.0",
"@angular-devkit/core": "0.0.0",
Expand Down
136 changes: 71 additions & 65 deletions packages/angular/cli/utilities/package-tree.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,88 +6,94 @@
* found in the LICENSE file at https://angular.io/license
*/


import * as fs from 'fs';
import { dirname, join } from 'path';
import * as resolve from 'resolve';
import { promisify } from 'util';
import { NgAddSaveDepedency } from './package-metadata';

export interface PackageTreeNodeBase {
const readFile = promisify(fs.readFile);

interface PackageJson {
name: string;
path: string;
realpath: string;
error?: Error;
id: number;
isLink: boolean;
package: {
name: string;
version: string;
dependencies?: Record<string, string>;
devDependencies?: Record<string, string>;
peerDependencies?: Record<string, string>;
optionalDependencies?: Record<string, string>;
'ng-update'?: {
migrations?: string;
};
'ng-add'?: {
save?: NgAddSaveDepedency;
};
version: string;
dependencies?: Record<string, string>;
devDependencies?: Record<string, string>;
peerDependencies?: Record<string, string>;
optionalDependencies?: Record<string, string>;
'ng-update'?: {
migrations?: string;
};
'ng-add'?: {
save?: NgAddSaveDepedency;
};
parent?: PackageTreeNode;
children: PackageTreeNode[];
}

export interface PackageTreeActual extends PackageTreeNodeBase {
isLink: false;
}
async function readJSON(file: string) {
const buffer = await readFile(file);

export interface PackageTreeLink extends PackageTreeNodeBase {
isLink: true;
target: PackageTreeActual;
return JSON.parse(buffer.toString());
}

export type PackageTreeNode = PackageTreeActual | PackageTreeLink;
function getAllDependencies(pkg: PackageJson) {
return new Set([
...Object.entries(pkg.dependencies || []),
...Object.entries(pkg.devDependencies || []),
...Object.entries(pkg.peerDependencies || []),
...Object.entries(pkg.optionalDependencies || []),
]);
}

export function readPackageTree(path: string): Promise<PackageTreeNode> {
const rpt = require('read-package-tree');
export interface PackageTreeNode {
name: string;
version: string;
path: string;
package: PackageJson | undefined;
}

return new Promise((resolve, reject) => {
rpt(path, (e: Error | undefined, data: PackageTreeNode) => {
if (e) {
reject(e);
} else {
resolve(data);
}
});
});
export async function readPackageJson(packageJsonPath: string): Promise<PackageJson | undefined> {
try {
return await readJSON(packageJsonPath);
} catch (err) {
return undefined;
}
}

export interface NodeDependency {
version: string;
node?: PackageTreeNode;
export function findPackageJson(workspaceDir: string, packageName: string) {
try {
// avoid require.resolve here, see: https://github.com/angular/angular-cli/pull/18610#issuecomment-681980185
const packageJsonPath = resolve.sync(`${packageName}/package.json`, { paths: [workspaceDir] });

return packageJsonPath;
} catch (err) {
return undefined;
}
}

export function findNodeDependencies(node: PackageTreeNode) {
const rawDeps: Record<string, string> = {
...node.package.dependencies,
...node.package.devDependencies,
...node.package.peerDependencies,
...node.package.optionalDependencies,
};
export async function getProjectDependencies(dir: string) {
const pkgJsonPath = resolve.sync(join(dir, `package.json`));
if (!pkgJsonPath) {
throw new Error('Could not find package.json');
}

return Object.entries(rawDeps).reduce(
(deps, [name, version]) => {
let dependencyNode;
let parent: PackageTreeNode | undefined | null = node;
while (!dependencyNode && parent) {
dependencyNode = parent.children.find(child => child.name === name);
parent = parent.parent;
}
const pkg: PackageJson = await readJSON(pkgJsonPath);

deps[name] = {
node: dependencyNode,
version,
};
const results = new Map<string, PackageTreeNode>();
await Promise.all(
Array.from(getAllDependencies(pkg)).map(async ([name, version]) => {
const packageJsonPath = findPackageJson(dir, name);
if (packageJsonPath) {
const currentDependency = {
name,
version,
path: dirname(packageJsonPath),
package: await readPackageJson(packageJsonPath),
};

return deps;
},
Object.create(null) as Record<string, NodeDependency>,
results.set(currentDependency.name, currentDependency);
}
}),
);

return results;
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
{
"private": true,
"workspaces": {
"packages": [
"packages/*"
],
"nohoist": [
"**/codelyzer",
"**/codelyzer/**",
"**/@angular*",
"**/@angular*/**"
]
}
}
Loading