From d6084b8a050ce59e4f066550842c27df5b5e3aa5 Mon Sep 17 00:00:00 2001 From: eipc16 Date: Wed, 25 Jun 2025 13:28:06 +0200 Subject: [PATCH] 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 --- .changeset/fuzzy-cooks-dress.md | 5 + .../afterWorkspaceDependencyAddition.test.ts | 89 ++++++++++++------ .../afterWorkspaceDependencyAddition.ts | 6 +- ...fterWorkspaceDependencyReplacement.test.ts | 93 ++++++++----------- .../afterWorkspaceDependencyReplacement.ts | 16 +--- 5 files changed, 117 insertions(+), 92 deletions(-) create mode 100644 .changeset/fuzzy-cooks-dress.md diff --git a/.changeset/fuzzy-cooks-dress.md b/.changeset/fuzzy-cooks-dress.md new file mode 100644 index 0000000000..77fab62647 --- /dev/null +++ b/.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. diff --git a/packages/yarn-plugin/src/handlers/afterWorkspaceDependencyAddition.test.ts b/packages/yarn-plugin/src/handlers/afterWorkspaceDependencyAddition.test.ts index ae575a8b02..171f8ad50c 100644 --- a/packages/yarn-plugin/src/handlers/afterWorkspaceDependencyAddition.test.ts +++ b/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 = []; - 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:^`, + ); }); }); diff --git a/packages/yarn-plugin/src/handlers/afterWorkspaceDependencyAddition.ts b/packages/yarn-plugin/src/handlers/afterWorkspaceDependencyAddition.ts index 219830a8c9..7fec01b3c7 100644 --- a/packages/yarn-plugin/src/handlers/afterWorkspaceDependencyAddition.ts +++ b/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; } } }; diff --git a/packages/yarn-plugin/src/handlers/afterWorkspaceDependencyReplacement.test.ts b/packages/yarn-plugin/src/handlers/afterWorkspaceDependencyReplacement.test.ts index 7bf9c77554..137d91d680 100644 --- a/packages/yarn-plugin/src/handlers/afterWorkspaceDependencyReplacement.test.ts +++ b/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(); }); }); diff --git a/packages/yarn-plugin/src/handlers/afterWorkspaceDependencyReplacement.ts b/packages/yarn-plugin/src/handlers/afterWorkspaceDependencyReplacement.ts index 55a11086fd..382a19c48b 100644 --- a/packages/yarn-plugin/src/handlers/afterWorkspaceDependencyReplacement.ts +++ b/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.`, + ); } };