fix: resolved issue that caused add & replace dependency hook to always fall silently

+ existing implementation did not do anything, because executing `getPackageVersion` always resulted in an error that was then silently suppressed

Signed-off-by: eipc16 <pprzemek.312@gmail.com>

.changeset/fuzzy-cooks-dress.md

@@ -0,0 +1,5 @@
+---
+'yarn-plugin-backstage': patch
+---
+
+Fixed a bug that would prevent the yarn plugin from installing new dependencies with the `backstage:^` protocol.

packages/yarn-plugin/src/handlers/afterWorkspaceDependencyAddition.test.ts

@@ -16,18 +16,17 @@
 import {
   Descriptor,
   DescriptorHash,
+  httpUtils,
   IdentHash,
   Workspace,
 } from '@yarnpkg/core';
+import { npath, ppath } from '@yarnpkg/fslib';
 import { suggestUtils } from '@yarnpkg/plugin-essentials';
-import { getPackageVersion } from '../util';
 import { afterWorkspaceDependencyAddition } from './afterWorkspaceDependencyAddition';
-
-jest.mock('../util', () => ({
-  getPackageVersion: jest.fn(),
-}));
+import { createMockDirectory } from '@backstage/backend-test-utils';
 
 describe('afterWorkspaceDependencyAddition', () => {
+  const mockDir = createMockDirectory();
   const workspace = {
     project: {
       configuration: {},
@@ -35,12 +34,42 @@ describe('afterWorkspaceDependencyAddition', () => {
   } as Workspace;
   const target = {} as suggestUtils.Target;
   const strategies: Array<suggestUtils.Strategy> = [];
-  const mockGetPackageVersion = getPackageVersion as jest.MockedFunction<
-    typeof getPackageVersion
-  >;
+
+  const consoleInfoSpy = jest
+    .spyOn(console, 'info')
+    .mockImplementation(() => {});
 
   beforeEach(() => {
-    mockGetPackageVersion.mockReset();
+    jest.resetAllMocks();
+
+    jest.spyOn(httpUtils, 'get').mockResolvedValue({
+      releaseVersion: '1.23.45',
+      packages: [
+        {
+          name: '@backstage/test-package',
+          version: '6.7.8',
+        },
+      ],
+    });
+
+    jest
+      .spyOn(ppath, 'cwd')
+      .mockReturnValue(npath.toPortablePath(mockDir.path));
+
+    jest
+      .spyOn(process, 'cwd')
+      .mockReturnValue(npath.toPortablePath(mockDir.path));
+
+    mockDir.setContent({
+      'backstage.json': JSON.stringify({
+        version: '1.23.45',
+      }),
+      'package.json': JSON.stringify({
+        workspaces: {
+          packages: ['packages/*'],
+        },
+      }),
+    });
   });
 
   it('should replace the range for a backstage scoped dependency', async () => {
@@ -52,8 +81,6 @@ describe('afterWorkspaceDependencyAddition', () => {
       identHash: {} as IdentHash,
     };
 
-    mockGetPackageVersion.mockImplementation(() => Promise.resolve('success'));
-
     await afterWorkspaceDependencyAddition(
       workspace,
       target,
@@ -62,26 +89,20 @@ describe('afterWorkspaceDependencyAddition', () => {
     );
 
     expect(input.range).toBe('backstage:^');
-    expect(mockGetPackageVersion).toHaveBeenCalledTimes(1);
-    expect(mockGetPackageVersion).toHaveBeenCalledWith(
-      input,
-      workspace.project.configuration,
+    expect(consoleInfoSpy).toHaveBeenCalledWith(
+      `Setting ${input.scope}/${input.name} to backstage:^`,
     );
   });
 
   it('should not replace the range for a backstage scoped dependency where it cant find a version from remote', async () => {
     const input: Descriptor = {
       scope: 'backstage',
-      name: 'test-package',
+      name: 'missing-package',
       range: '^1.0.0',
       descriptorHash: {} as DescriptorHash,
       identHash: {} as IdentHash,
     };
 
-    mockGetPackageVersion.mockImplementation(() =>
-      Promise.reject(new Error('test error')),
-    );
-
     await afterWorkspaceDependencyAddition(
       workspace,
       target,
@@ -90,11 +111,6 @@ describe('afterWorkspaceDependencyAddition', () => {
     );
 
     expect(input.range).toBe('^1.0.0');
-    expect(mockGetPackageVersion).toHaveBeenCalledTimes(1);
-    expect(mockGetPackageVersion).toHaveBeenCalledWith(
-      input,
-      workspace.project.configuration,
-    );
   });
 
   it('should not replace the range for a non-backstage scoped dependency', async () => {
@@ -114,6 +130,27 @@ describe('afterWorkspaceDependencyAddition', () => {
     );
 
     expect(input.range).toBe('^1.0.0');
-    expect(mockGetPackageVersion).not.toHaveBeenCalled();
+  });
+
+  it('should not replace the range for a dependency with backstage:^ version', async () => {
+    const input: Descriptor = {
+      scope: 'backstage',
+      name: 'another-test-package',
+      range: 'backstage:^',
+      descriptorHash: {} as DescriptorHash,
+      identHash: {} as IdentHash,
+    };
+
+    await afterWorkspaceDependencyAddition(
+      workspace,
+      target,
+      input,
+      strategies,
+    );
+
+    expect(input.range).toBe('backstage:^');
+    expect(consoleInfoSpy).not.toHaveBeenCalledWith(
+      `Setting ${input.scope}/${input.name} to backstage:^`,
+    );
   });
 });

packages/yarn-plugin/src/handlers/afterWorkspaceDependencyAddition.ts

@@ -31,16 +31,18 @@ export const afterWorkspaceDependencyAddition = async (
     descriptor.scope === 'backstage' &&
     descriptorRange.protocol !== PROTOCOL
   ) {
+    const originalRange = descriptor.range;
     try {
-      await getPackageVersion(descriptor, workspace.project.configuration);
+      descriptor.range = `${PROTOCOL}^`;
+      await getPackageVersion(descriptor, workspace.project.configuration); // Verify that the actual version can be resolved
       console.info(
         `Setting ${descriptor.scope}/${descriptor.name} to ${PROTOCOL}^`,
       );
-      descriptor.range = `${PROTOCOL}^`;
     } catch (_error: any) {
       // if there's no found version then this is likely a deprecated package
       // or otherwise the plugin won't be able to resolve the real version
       // and we should leave the desired range as is
+      descriptor.range = originalRange;
     }
   }
 };

packages/yarn-plugin/src/handlers/afterWorkspaceDependencyReplacement.test.ts

@@ -16,30 +16,59 @@
 import {
   Descriptor,
   DescriptorHash,
+  httpUtils,
   IdentHash,
   Workspace,
 } from '@yarnpkg/core';
+import { npath, ppath } from '@yarnpkg/fslib';
 import { suggestUtils } from '@yarnpkg/plugin-essentials';
-import { getPackageVersion } from '../util';
 import { afterWorkspaceDependencyReplacement } from './afterWorkspaceDependencyReplacement';
-
-jest.mock('../util', () => ({
-  getPackageVersion: jest.fn(),
-}));
+import { createMockDirectory } from '@backstage/backend-test-utils';
 
 describe('afterWorkspaceDependencyReplacement.test', () => {
+  const mockDir = createMockDirectory();
   const workspace = {
     project: {
       configuration: {},
     },
   } as Workspace;
   const target = {} as suggestUtils.Target;
-  const mockGetPackageVersion = getPackageVersion as jest.MockedFunction<
-    typeof getPackageVersion
-  >;
+
+  const consoleWarnSpy = jest
+    .spyOn(console, 'warn')
+    .mockImplementation(() => {});
 
   beforeEach(() => {
-    mockGetPackageVersion.mockReset();
+    jest.resetAllMocks();
+
+    jest.spyOn(httpUtils, 'get').mockResolvedValue({
+      releaseVersion: '1.23.45',
+      packages: [
+        {
+          name: '@backstage/test-package',
+          version: '6.7.8',
+        },
+      ],
+    });
+
+    jest
+      .spyOn(ppath, 'cwd')
+      .mockReturnValue(npath.toPortablePath(mockDir.path));
+
+    jest
+      .spyOn(process, 'cwd')
+      .mockReturnValue(npath.toPortablePath(mockDir.path));
+
+    mockDir.setContent({
+      'backstage.json': JSON.stringify({
+        version: '1.23.45',
+      }),
+      'package.json': JSON.stringify({
+        workspaces: {
+          packages: ['packages/*'],
+        },
+      }),
+    });
   });
 
   it('should warn that the range is being changed for a backstage scoped dependency', async () => {
@@ -58,8 +87,6 @@ describe('afterWorkspaceDependencyReplacement.test', () => {
       identHash: {} as IdentHash,
     };
 
-    mockGetPackageVersion.mockImplementation(() => Promise.resolve('success'));
-
     await afterWorkspaceDependencyReplacement(
       workspace,
       target,
@@ -67,46 +94,8 @@ describe('afterWorkspaceDependencyReplacement.test', () => {
       toDescriptor,
     );
 
-    expect(toDescriptor.range).toBe('^1.0.0');
-    expect(mockGetPackageVersion).toHaveBeenCalledTimes(1);
-    expect(mockGetPackageVersion).toHaveBeenCalledWith(
-      toDescriptor,
-      workspace.project.configuration,
-    );
-  });
-
-  it('should not warn that the range is being changed for a backstage scoped dependency where it cant find a version from remote', async () => {
-    const fromDescriptor: Descriptor = {
-      scope: 'backstage',
-      name: 'test-package',
-      range: 'backstage:^',
-      descriptorHash: {} as DescriptorHash,
-      identHash: {} as IdentHash,
-    };
-    const toDescriptor: Descriptor = {
-      scope: 'backstage',
-      name: 'test-package',
-      range: '^1.0.0',
-      descriptorHash: {} as DescriptorHash,
-      identHash: {} as IdentHash,
-    };
-
-    mockGetPackageVersion.mockImplementation(() =>
-      Promise.reject(new Error('test error')),
-    );
-
-    await afterWorkspaceDependencyReplacement(
-      workspace,
-      target,
-      fromDescriptor,
-      toDescriptor,
-    );
-
-    expect(toDescriptor.range).toBe('^1.0.0');
-    expect(mockGetPackageVersion).toHaveBeenCalledTimes(1);
-    expect(mockGetPackageVersion).toHaveBeenCalledWith(
-      toDescriptor,
-      workspace.project.configuration,
+    expect(consoleWarnSpy).toHaveBeenCalledWith(
+      'test-package should be set to "backstage:^" instead of "^1.0.0". Make sure this change is intentional and not a mistake.',
     );
   });
 
@@ -132,7 +121,7 @@ describe('afterWorkspaceDependencyReplacement.test', () => {
       toDescriptor,
     );
 
+    expect(consoleWarnSpy).not.toHaveBeenCalled();
     expect(toDescriptor.range).toBe('^1.0.0');
-    expect(mockGetPackageVersion).not.toHaveBeenCalled();
   });
 });

packages/yarn-plugin/src/handlers/afterWorkspaceDependencyReplacement.ts

@@ -16,11 +16,10 @@
 
 import { Descriptor, structUtils, Workspace } from '@yarnpkg/core';
 import { suggestUtils } from '@yarnpkg/plugin-essentials';
-import { getPackageVersion } from '../util';
 import { PROTOCOL } from '../constants';
 
 export const afterWorkspaceDependencyReplacement = async (
-  workspace: Workspace,
+  _workspace: Workspace,
   _target: suggestUtils.Target,
   _fromDescriptor: Descriptor,
   toDescriptor: Descriptor,
@@ -31,15 +30,8 @@ export const afterWorkspaceDependencyReplacement = async (
     toDescriptor.scope === 'backstage' &&
     toDescriptorRange.protocol !== PROTOCOL
   ) {
-    try {
-      await getPackageVersion(toDescriptor, workspace.project.configuration);
-      console.warn(
-        `${toDescriptor.name} should be set to "${PROTOCOL}^" instead of "${toDescriptor.range}". Make sure this change is intentional and not a mistake.`,
-      );
-    } catch (_error: any) {
-      // if there's no found version then this is likely a deprecated package
-      // or otherwise the plugin won't be able to resolve the real version
-      // and we should not warn them.
-    }
+    console.warn(
+      `${toDescriptor.name} should be set to "${PROTOCOL}^" instead of "${toDescriptor.range}". Make sure this change is intentional and not a mistake.`,
+    );
   }
 };