From f3bef24d4f34a0ec21f6c38fee34f445d91f3036 Mon Sep 17 00:00:00 2001 From: Jerico Pingul Date: Tue, 6 Dec 2022 22:07:43 +0000 Subject: [PATCH 1/4] fix(core): handle undefined when package.json changes are not in npm packages ISSUES CLOSED: #13668 --- .../affected/locators/npm-packages.spec.ts | 28 +++++++++++++++++++ .../affected/locators/npm-packages.ts | 1 + 2 files changed, 29 insertions(+) diff --git a/packages/nx/src/project-graph/affected/locators/npm-packages.spec.ts b/packages/nx/src/project-graph/affected/locators/npm-packages.spec.ts index 97dba48a8befd..2c25d62745933 100644 --- a/packages/nx/src/project-graph/affected/locators/npm-packages.spec.ts +++ b/packages/nx/src/project-graph/affected/locators/npm-packages.spec.ts @@ -265,4 +265,32 @@ describe('getTouchedNpmPackages', () => { 'npm:awesome-nrwl', ]); }); + it('should handle workspace package.json changes when the changes are not in npmPackages (projectGraph.externalNodes)', () => { + expect(() => { + getTouchedNpmPackages( + [ + { + file: 'package.json', + hash: 'some-hash', + getChanges: () => [ + { + type: 'JsonPropertyAdded', + path: ['devDependencies', 'changed-test-pkg-name'], + value: { rhs: 'workspace:*' }, + }, + ], + }, + ], + workspaceJson, + nxJson, + { + dependencies: { + 'happy-nrwl': '0.0.1', + 'awesome-nrwl': '0.0.1', + }, + }, + projectGraph + ); + }).not.toThrowError(); + }); }); diff --git a/packages/nx/src/project-graph/affected/locators/npm-packages.ts b/packages/nx/src/project-graph/affected/locators/npm-packages.ts index cfec247231cca..991f2a5bcfc43 100644 --- a/packages/nx/src/project-graph/affected/locators/npm-packages.ts +++ b/packages/nx/src/project-graph/affected/locators/npm-packages.ts @@ -31,6 +31,7 @@ export const getTouchedNpmPackages: TouchedProjectLocator< const npmPackage = npmPackages.find( (pkg) => pkg.data.packageName === c.path[1] ); + if (!npmPackage) break; touched.push(npmPackage.name); // If it was a type declarations package then also mark its corresponding implementation package as affected if (npmPackage.name.startsWith('npm:@types/')) { From 20e1544390694f19418b848c2aa9a2a5d7c62cc2 Mon Sep 17 00:00:00 2001 From: Jerico Pingul Date: Tue, 6 Dec 2022 23:52:45 +0000 Subject: [PATCH 2/4] fix(core): update test text --- .../nx/src/project-graph/affected/locators/npm-packages.spec.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/nx/src/project-graph/affected/locators/npm-packages.spec.ts b/packages/nx/src/project-graph/affected/locators/npm-packages.spec.ts index 2c25d62745933..33f2d8189abc7 100644 --- a/packages/nx/src/project-graph/affected/locators/npm-packages.spec.ts +++ b/packages/nx/src/project-graph/affected/locators/npm-packages.spec.ts @@ -265,7 +265,7 @@ describe('getTouchedNpmPackages', () => { 'npm:awesome-nrwl', ]); }); - it('should handle workspace package.json changes when the changes are not in npmPackages (projectGraph.externalNodes)', () => { + it('should handle workspace package.json changes when the changes are not in `npmPackages` (projectGraph.externalNodes)', () => { expect(() => { getTouchedNpmPackages( [ From c11564a127715eae15286d96dfc3dfa68be46d17 Mon Sep 17 00:00:00 2001 From: Jerico Pingul Date: Wed, 7 Dec 2022 00:23:24 +0000 Subject: [PATCH 3/4] fix(core): change to continue --- packages/nx/src/project-graph/affected/locators/npm-packages.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/nx/src/project-graph/affected/locators/npm-packages.ts b/packages/nx/src/project-graph/affected/locators/npm-packages.ts index 991f2a5bcfc43..8f13a204ac73f 100644 --- a/packages/nx/src/project-graph/affected/locators/npm-packages.ts +++ b/packages/nx/src/project-graph/affected/locators/npm-packages.ts @@ -31,7 +31,7 @@ export const getTouchedNpmPackages: TouchedProjectLocator< const npmPackage = npmPackages.find( (pkg) => pkg.data.packageName === c.path[1] ); - if (!npmPackage) break; + if (!npmPackage) continue; touched.push(npmPackage.name); // If it was a type declarations package then also mark its corresponding implementation package as affected if (npmPackage.name.startsWith('npm:@types/')) { From 221fb6635623587f81e7ded34b7d631ca0ec057d Mon Sep 17 00:00:00 2001 From: Jerico Pingul Date: Thu, 8 Dec 2022 22:33:02 +0000 Subject: [PATCH 4/4] fix(core): add warning to user indicating that affected projects may not have been identified --- .../affected/locators/npm-packages.spec.ts | 14 ++++++++++++-- .../affected/locators/npm-packages.ts | 15 ++++++++++++++- 2 files changed, 26 insertions(+), 3 deletions(-) diff --git a/packages/nx/src/project-graph/affected/locators/npm-packages.spec.ts b/packages/nx/src/project-graph/affected/locators/npm-packages.spec.ts index 33f2d8189abc7..58a9d12612226 100644 --- a/packages/nx/src/project-graph/affected/locators/npm-packages.spec.ts +++ b/packages/nx/src/project-graph/affected/locators/npm-packages.spec.ts @@ -1,6 +1,7 @@ import { NxJsonConfiguration } from '../../../config/nx-json'; import { ProjectGraph } from '../../../config/project-graph'; import { JsonDiffType } from '../../../utils/json-diff'; +import { logger } from '../../../utils/logger'; import { WholeFileChange } from '../../file-utils'; import { getTouchedNpmPackages } from './npm-packages'; @@ -265,7 +266,8 @@ describe('getTouchedNpmPackages', () => { 'npm:awesome-nrwl', ]); }); - it('should handle workspace package.json changes when the changes are not in `npmPackages` (projectGraph.externalNodes)', () => { + it('should handle and log workspace package.json changes when the changes are not in `npmPackages` (projectGraph.externalNodes)', () => { + jest.spyOn(logger, 'warn'); expect(() => { getTouchedNpmPackages( [ @@ -275,7 +277,12 @@ describe('getTouchedNpmPackages', () => { getChanges: () => [ { type: 'JsonPropertyAdded', - path: ['devDependencies', 'changed-test-pkg-name'], + path: ['devDependencies', 'changed-test-pkg-name-1'], + value: { rhs: 'workspace:*' }, + }, + { + type: 'JsonPropertyAdded', + path: ['devDependencies', 'changed-test-pkg-name-2'], value: { rhs: 'workspace:*' }, }, ], @@ -292,5 +299,8 @@ describe('getTouchedNpmPackages', () => { projectGraph ); }).not.toThrowError(); + expect(logger.warn).toHaveBeenCalledWith( + 'The affected projects might have not been identified properly. The package(s) changed-test-pkg-name-1, changed-test-pkg-name-2 were not found. Please open an issue in Github including the package.json file.' + ); }); }); diff --git a/packages/nx/src/project-graph/affected/locators/npm-packages.ts b/packages/nx/src/project-graph/affected/locators/npm-packages.ts index 8f13a204ac73f..3f795ee688793 100644 --- a/packages/nx/src/project-graph/affected/locators/npm-packages.ts +++ b/packages/nx/src/project-graph/affected/locators/npm-packages.ts @@ -4,6 +4,7 @@ import { isJsonChange, JsonChange, } from '../../../utils/json-diff'; +import { logger } from '../../../utils/logger'; import { TouchedProjectLocator } from '../affected-project-graph-models'; export const getTouchedNpmPackages: TouchedProjectLocator< @@ -17,6 +18,8 @@ export const getTouchedNpmPackages: TouchedProjectLocator< const npmPackages = Object.values(projectGraph.externalNodes); + const missingTouchedNpmPackages: string[] = []; + for (const c of changes) { if ( isJsonChange(c) && @@ -31,7 +34,10 @@ export const getTouchedNpmPackages: TouchedProjectLocator< const npmPackage = npmPackages.find( (pkg) => pkg.data.packageName === c.path[1] ); - if (!npmPackage) continue; + if (!npmPackage) { + missingTouchedNpmPackages.push(c.path[1]); + continue; + } touched.push(npmPackage.name); // If it was a type declarations package then also mark its corresponding implementation package as affected if (npmPackage.name.startsWith('npm:@types/')) { @@ -50,5 +56,12 @@ export const getTouchedNpmPackages: TouchedProjectLocator< } } + if (missingTouchedNpmPackages.length) { + logger.warn( + `The affected projects might have not been identified properly. The package(s) ${missingTouchedNpmPackages.join( + ', ' + )} were not found. Please open an issue in Github including the package.json file.` + ); + } return touched; };