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
Original file line number Diff line number Diff line change
Expand Up @@ -155,6 +155,7 @@ export const SUPPORTED_PACKAGE_MANAGERS = {
ignoreScriptsFlag: '--ignore-scripts',
ignorePeerDependenciesFlag: '--force',
configFiles: ['.npmrc'],
copyConfigFromProject: true,
getRegistryOptions: (registry: string) => ({ args: ['--registry', registry] }),
versionCommand: ['--version'],
listDependenciesCommand: ['list', '--depth=0', '--json=true', '--all=true'],
Expand All @@ -180,6 +181,7 @@ export const SUPPORTED_PACKAGE_MANAGERS = {
noLockfileFlag: '',
ignoreScriptsFlag: '--mode=skip-build',
configFiles: ['.yarnrc.yml', '.yarnrc.yaml'],
copyConfigFromProject: true,
getRegistryOptions: (registry: string) => ({ env: { YARN_NPM_REGISTRY_SERVER: registry } }),
versionCommand: ['--version'],
listDependenciesCommand: ['info', '--name-only', '--json'],
Expand Down Expand Up @@ -208,6 +210,7 @@ export const SUPPORTED_PACKAGE_MANAGERS = {
noLockfileFlag: '--no-lockfile',
ignoreScriptsFlag: '--ignore-scripts',
configFiles: ['.yarnrc', '.npmrc'],
copyConfigFromProject: true,
getRegistryOptions: (registry: string) => ({ args: ['--registry', registry] }),
versionCommand: ['--version'],
listDependenciesCommand: ['list', '--depth=0', '--json'],
Expand All @@ -233,7 +236,10 @@ export const SUPPORTED_PACKAGE_MANAGERS = {
noLockfileFlag: '--no-lockfile',
ignoreScriptsFlag: '--ignore-scripts',
ignorePeerDependenciesFlag: '--strict-peer-dependencies=false',
configFiles: ['.npmrc', 'pnpm-workspace.yaml'],
// Note: pnpm-workspace.yaml is intentionally excluded. Copying it to the
// temporary directory could cause pnpm to treat it as a workspace root.
configFiles: ['.npmrc'],
copyConfigFromProject: true,
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

Enabling copyConfigFromProject for pnpm will cause pnpm-workspace.yaml to be copied into the temporary directory (as it is listed in configFiles on line 238). This might cause pnpm to attempt workspace-related operations in an isolated temporary directory, which could lead to unexpected behavior or errors if the workspace structure is incomplete. Consider if pnpm-workspace.yaml should be removed from the configFiles list if only registry settings from .npmrc are required for temporary package acquisition.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

Only .npmrc is needed for registry settings. Added a comment explaining the exclusion.
Worth noting that pnpm-workspace.yaml was never actually copied before this change since copyConfigFromProject was disabled for pnpm, so this is not a regression.

getRegistryOptions: (registry: string) => ({ args: ['--registry', registry] }),
versionCommand: ['--version'],
listDependenciesCommand: ['list', '--depth=0', '--json'],
Expand Down
3 changes: 2 additions & 1 deletion packages/angular/cli/src/package-managers/package-manager.ts
Original file line number Diff line number Diff line change
Expand Up @@ -621,7 +621,8 @@ export class PackageManager {
// Writing an empty package.json file beforehand prevents this.
await this.host.writeFile(join(workingDirectory, 'package.json'), '{}');

// Copy configuration files if the package manager requires it (e.g., bun).
// Copy configuration files from the project to the temp directory so that
// registry settings and other package manager config are respected.
if (this.descriptor.copyConfigFromProject) {
for (const configFile of this.descriptor.configFiles) {
try {
Expand Down
47 changes: 47 additions & 0 deletions packages/angular/cli/src/package-managers/package-manager_spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -113,4 +113,51 @@ describe('PackageManager', () => {
expect(manifest).toEqual({ name: 'foo', version: '1.0.0' });
});
});

describe('acquireTempPackage', () => {
it('should copy config files from the project when copyConfigFromProject is true', async () => {
// eslint-disable-next-line @typescript-eslint/no-explicit-any
const copyFileSpy = spyOn(host as any, 'copyFile').and.resolveTo();
spyOn(host, 'createTempDirectory').and.resolveTo('/tmp/angular-cli-packages-test');
spyOn(host, 'writeFile').and.resolveTo();
spyOn(host, 'deleteDirectory').and.resolveTo();

const pm = new PackageManager(host, '/project', descriptor);

await pm.acquireTempPackage('foo');

expect(copyFileSpy).toHaveBeenCalledWith(
'/project/.npmrc',
'/tmp/angular-cli-packages-test/.npmrc',
);
});

it('should not copy config files when copyConfigFromProject is not set', async () => {
// eslint-disable-next-line @typescript-eslint/no-explicit-any
const copyFileSpy = spyOn(host as any, 'copyFile').and.resolveTo();
spyOn(host, 'createTempDirectory').and.resolveTo('/tmp/angular-cli-packages-test');
spyOn(host, 'writeFile').and.resolveTo();
spyOn(host, 'deleteDirectory').and.resolveTo();

const noCopyDescriptor = { ...descriptor, copyConfigFromProject: undefined };
const pm = new PackageManager(host, '/project', noCopyDescriptor);

await pm.acquireTempPackage('foo');

expect(copyFileSpy).not.toHaveBeenCalled();
});

it('should ignore missing config files gracefully', async () => {
// eslint-disable-next-line @typescript-eslint/no-explicit-any
const copyFileSpy = spyOn(host as any, 'copyFile').and.rejectWith(new Error('ENOENT'));
spyOn(host, 'createTempDirectory').and.resolveTo('/tmp/angular-cli-packages-test');
spyOn(host, 'writeFile').and.resolveTo();
spyOn(host, 'deleteDirectory').and.resolveTo();

const pm = new PackageManager(host, '/project', descriptor);

await expectAsync(pm.acquireTempPackage('foo')).toBeResolved();
expect(copyFileSpy).toHaveBeenCalled();
});
});
});
Loading