Skip to content
Open
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
31 changes: 31 additions & 0 deletions packages/angular/cli/src/commands/add/cli.ts
Original file line number Diff line number Diff line change
Expand Up @@ -591,6 +591,7 @@ export default class AddCommandModule
join(context.collectionName, 'package.json'),
);

await this.refreshInstalledPackageInfo(context, resolvedCollectionPath, false);
context.collectionName = dirname(resolvedCollectionPath);
} else {
await packageManager.add(
Expand All @@ -603,6 +604,8 @@ export default class AddCommandModule
registry,
},
);

await this.refreshInstalledPackageInfo(context);
}
} catch (e) {
if (e instanceof PackageManagerError) {
Expand All @@ -616,6 +619,34 @@ export default class AddCommandModule
}
}

private async refreshInstalledPackageInfo(
context: AddCommandTaskContext,
installedPackagePath?: string,
updateCollectionName = true,
): Promise<void> {
installedPackagePath ??= this.resolvePackageJson(context.collectionName ?? '');
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

Using context.collectionName ?? '' as an argument to resolvePackageJson can lead to unintended behavior. If collectionName is undefined or an empty string, resolvePackageJson('') will attempt to resolve the package.json file in the project root instead of a package within node_modules. It is safer to only attempt resolution when a valid collection name is available.

Suggested change
installedPackagePath ??= this.resolvePackageJson(context.collectionName ?? '');
installedPackagePath ??= context.collectionName ? this.resolvePackageJson(context.collectionName) : undefined;

if (!installedPackagePath) {
return;
}

try {
const installedManifest = JSON.parse(
await fs.readFile(installedPackagePath, 'utf-8'),
) as PackageManifest;

context.hasSchematics = !!installedManifest.schematics;
if (updateCollectionName) {
context.collectionName = installedManifest.name;
}
context.homepage = installedManifest.homepage ?? context.homepage;
} catch (e) {
assertIsError(e);
this.context.logger.debug(
`Unable to read installed package information from '${installedPackagePath}': ${e.message}`,
);
}
}

private async isProjectVersionValid(packageIdentifier: npa.Result): Promise<boolean> {
if (!packageIdentifier.name) {
return false;
Expand Down
194 changes: 194 additions & 0 deletions packages/angular/cli/src/commands/add/cli_spec.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,194 @@
/**
* @license
* Copyright Google LLC All Rights Reserved.
*
* Use of this source code is governed by an MIT-style license that can be
* found in the LICENSE file at https://angular.dev/license
*/

import { logging } from '@angular-devkit/core';
import { mkdir, mkdtemp, rm, writeFile } from 'node:fs/promises';
import { tmpdir } from 'node:os';
import { join } from 'node:path';
import type { Argv } from 'yargs';
import type { CommandContext } from '../../command-builder/definitions';
import type { PackageManager, PackageManifest } from '../../package-managers';
import AddCommandModule from './cli';

describe('AddCommandModule', () => {
let root: string;
let logger: logging.Logger;

beforeEach(async () => {
root = await mkdtemp(join(tmpdir(), 'angular-cli-add-'));
logger = {
info: jasmine.createSpy('info'),
error: jasmine.createSpy('error'),
warn: jasmine.createSpy('warn'),
debug: jasmine.createSpy('debug'),
fatal: jasmine.createSpy('fatal'),
} as unknown as logging.Logger;

await writeFile(join(root, 'package.json'), '{}');
});

afterEach(async () => {
await rm(root, { recursive: true, force: true });
});

it('uses the installed package manifest to detect ng-add schematics', async () => {
const packageName = '@private/package';
const packageManager = createPackageManager({
async add() {
await writeInstalledPackageManifest(packageName, {
name: packageName,
version: '1.0.0',
schematics: './collection.json',
});
},
getManifest: jasmine
.createSpy('getManifest')
.and.resolveTo({ name: packageName, version: '1.0.0' }),
});
const command = createCommand(packageManager);
const { createSchematic } = mockSchematicWorkflow(command);

const result = await command.run({
collection: `${packageName}@1.0.0`,
defaults: false,
dryRun: false,
force: false,
interactive: false,
skipConfirmation: true,
});

expect(result).toBe(0);
expect(packageManager.add).toHaveBeenCalled();
expect(createSchematic).toHaveBeenCalledWith('ng-add', true);
expect(command.executeSchematic).toHaveBeenCalledWith(
jasmine.objectContaining({ collection: packageName }),
);
});

it('uses the temporary package manifest to detect ng-add schematics', async () => {
const packageName = '@private/package';
const workingDirectory = join(root, 'temp-install');
const packageManager = createPackageManager({
async acquireTempPackage() {
await writeInstalledPackageManifest(
packageName,
{
name: packageName,
version: '1.0.0',
schematics: './collection.json',
},
workingDirectory,
);

return { workingDirectory, cleanup: jasmine.createSpy('cleanup') };
},
getManifest: jasmine.createSpy('getManifest').and.resolveTo({
name: packageName,
version: '1.0.0',
'ng-add': { save: false },
}),
});
const command = createCommand(packageManager);
const { createSchematic } = mockSchematicWorkflow(command);

const result = await command.run({
collection: `${packageName}@1.0.0`,
defaults: false,
dryRun: false,
force: false,
interactive: false,
skipConfirmation: true,
});

expect(result).toBe(0);
expect(packageManager.add).not.toHaveBeenCalled();
expect(packageManager.acquireTempPackage).toHaveBeenCalled();
expect(createSchematic).toHaveBeenCalledWith('ng-add', true);
expect(command.executeSchematic).toHaveBeenCalledWith(
jasmine.objectContaining({
collection: join(workingDirectory, 'node_modules', ...packageName.split('/')),
}),
);
});

function createCommand(packageManager: PackageManager): AddCommandModuleInternals {
const context = {
args: {
positional: [],
options: {
getYargsCompletions: false,
help: false,
jsonHelp: false,
},
},
currentDirectory: root,
globalConfiguration: {},
logger,
packageManager,
root,
yargsInstance: {} as Argv,
} as unknown as CommandContext;

const command = new AddCommandModule(context) as unknown as AddCommandModuleInternals;
command.executeSchematic = jasmine.createSpy('executeSchematic').and.resolveTo(0);

return command;
}

function createPackageManager(options: {
acquireTempPackage?: PackageManager['acquireTempPackage'];
add?: PackageManager['add'];
getManifest: jasmine.Spy;
}): PackageManager {
const packageManager = {
acquireTempPackage: jasmine
.createSpy('acquireTempPackage')
.and.callFake(options.acquireTempPackage ?? fail),
add: jasmine.createSpy('add').and.callFake(options.add ?? fail),
getManifest: options.getManifest,
name: 'npm',
} as unknown as PackageManager;

return packageManager;
}

function mockSchematicWorkflow(command: AddCommandModuleInternals): {
createSchematic: jasmine.Spy;
} {
const createSchematic = jasmine.createSpy('createSchematic');

command.getOrCreateWorkflowForBuilder = jasmine
.createSpy('getOrCreateWorkflowForBuilder')
.and.returnValue({
engine: {
createCollection: jasmine.createSpy('createCollection').and.returnValue({
createSchematic,
}),
},
});

return { createSchematic };
}

async function writeInstalledPackageManifest(
packageName: string,
manifest: PackageManifest,
basePath = root,
): Promise<void> {
const packagePath = join(basePath, 'node_modules', ...packageName.split('/'));

await mkdir(packagePath, { recursive: true });
await writeFile(join(packagePath, 'package.json'), JSON.stringify(manifest));
}
});

type AddCommandModuleInternals = {
executeSchematic: jasmine.Spy;
getOrCreateWorkflowForBuilder: jasmine.Spy;
run: AddCommandModule['run'];
};
Loading