Skip to content

Commit 213c25d

Browse files
FrozenPandazFrozenPandaz
committed
fix(js): properly validate if package.json dependencies are indeed pointing to workspace projects
1 parent 6600da2 commit 213c25d

File tree

6 files changed

+203
-95
lines changed

6 files changed

+203
-95
lines changed

‎packages/nx/src/config/workspace-json-project-json.ts‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -157,6 +157,7 @@ export interface ProjectMetadata {
157157
};
158158
js?: {
159159
packageName: string;
160+
packageVersion?: string;
160161
packageExports?: PackageJson['exports'];
161162
packageMain?: string;
162163
isInPackageManagerWorkspaces?: boolean;

‎packages/nx/src/plugins/js/project-graph/build-dependencies/explicit-package-json-dependencies.spec.ts‎

Lines changed: 136 additions & 78 deletions
Original file line numberDiff line numberDiff line change
@@ -34,6 +34,14 @@ describe('explicit package json dependencies', () => {
3434
root: 'libs/proj4',
3535
name: 'proj4',
3636
},
37+
proj5: {
38+
root: 'libs/proj5',
39+
name: 'proj5',
40+
},
41+
proj6: {
42+
root: 'libs/proj6',
43+
name: 'proj6',
44+
},
3745
},
3846
};
3947

@@ -65,6 +73,16 @@ describe('explicit package json dependencies', () => {
6573
name: 'proj4',
6674
version: '2.0.0', // the source version in the workspace
6775
}),
76+
'./libs/proj5/index.js': '',
77+
'./libs/proj5/package.json': JSON.stringify({
78+
name: 'proj5',
79+
version: '1.1.0', // the source version in the workspace
80+
}),
81+
'./libs/proj6/index.js': '',
82+
'./libs/proj6/package.json': JSON.stringify({
83+
name: 'proj6',
84+
version: '1.1.0', // the source version in the workspace
85+
}),
6886
'./libs/proj3/node_modules/lodash/index.js': '',
6987
'./libs/proj3/node_modules/lodash/package.json': JSON.stringify({
7088
name: 'lodash',
@@ -79,6 +97,8 @@ describe('explicit package json dependencies', () => {
7997
dependencies: {
8098
proj2: '*',
8199
proj4: '1.0.0', // references an installed older version from package manager
100+
proj5: '^1.0.0',
101+
proj6: 'file:../proj6',
82102
lodash: '4.0.0',
83103
},
84104
devDependencies: { proj3: '*' },
@@ -136,6 +156,37 @@ describe('explicit package json dependencies', () => {
136156
metadata: {
137157
js: {
138158
packageName: 'proj4',
159+
packageVersion: '2.0.0',
160+
packageExports: undefined,
161+
isInPackageManagerWorkspaces: true,
162+
},
163+
},
164+
},
165+
},
166+
proj5: {
167+
name: 'proj5',
168+
type: 'lib',
169+
data: {
170+
root: 'libs/proj5',
171+
metadata: {
172+
js: {
173+
packageName: 'proj5',
174+
packageVersion: '1.1.0',
175+
packageExports: undefined,
176+
isInPackageManagerWorkspaces: true,
177+
},
178+
},
179+
},
180+
},
181+
proj6: {
182+
name: 'proj6',
183+
type: 'lib',
184+
data: {
185+
root: 'libs/proj6',
186+
metadata: {
187+
js: {
188+
packageName: 'proj6',
189+
packageVersion: '1.1.0',
139190
packageExports: undefined,
140191
isInPackageManagerWorkspaces: true,
141192
},
@@ -201,46 +252,37 @@ describe('explicit package json dependencies', () => {
201252
);
202253

203254
const res = buildExplicitPackageJsonDependencies(ctx, targetProjectLocator);
204-
expect(res).toEqual([
205-
{
206-
source: 'proj',
207-
target: 'proj3',
208-
sourceFile: 'libs/proj/package.json',
209-
type: 'static',
210-
},
211-
{
212-
source: 'proj',
213-
target: 'proj2',
214-
sourceFile: 'libs/proj/package.json',
215-
type: 'static',
216-
},
217-
{
218-
source: 'proj',
219-
sourceFile: 'libs/proj/package.json',
220-
target: 'npm:proj4', // correctly resolves to the npm package of the project rather than the workspace project
221-
type: 'static',
222-
},
223-
{
224-
sourceFile: 'libs/proj/package.json',
225-
source: 'proj',
226-
target: 'npm:lodash',
227-
type: 'static',
228-
},
229-
{
230-
sourceFile: 'libs/proj3/package.json',
231-
source: 'proj3',
232-
target: 'npm:[email protected]',
233-
type: 'static',
234-
},
235-
{
236-
source: 'proj3',
237-
sourceFile: 'libs/proj3/package.json',
238-
target: 'proj4', // correctly resolves to the workspace dependency of the project in the workspace
239-
type: 'static',
240-
},
241-
]);
255+
expect(res).toEqual(
256+
expect.arrayContaining([
257+
{
258+
source: 'proj',
259+
target: 'proj3',
260+
sourceFile: 'libs/proj/package.json',
261+
type: 'static',
262+
},
263+
{
264+
source: 'proj',
265+
target: 'proj2',
266+
sourceFile: 'libs/proj/package.json',
267+
type: 'static',
268+
},
269+
{
270+
sourceFile: 'libs/proj/package.json',
271+
source: 'proj',
272+
target: 'npm:lodash',
273+
type: 'static',
274+
},
275+
{
276+
sourceFile: 'libs/proj3/package.json',
277+
source: 'proj3',
278+
target: 'npm:[email protected]',
279+
type: 'static',
280+
},
281+
])
282+
);
242283
expect(npmResolutionCache).toMatchInlineSnapshot(`
243284
Map {
285+
"proj4__libs/proj" => "npm:proj4",
244286
"lodash__libs/proj" => "npm:lodash",
245287
"lodash__libs/proj3" => "npm:[email protected]",
246288
}
@@ -260,46 +302,20 @@ describe('explicit package json dependencies', () => {
260302
npmResolutionCache.set('lodash__libs/proj3', 'npm:[email protected]');
261303

262304
const res = buildExplicitPackageJsonDependencies(ctx, targetProjectLocator);
263-
expect(res).toEqual([
264-
{
265-
source: 'proj',
266-
target: 'proj3',
267-
sourceFile: 'libs/proj/package.json',
268-
type: 'static',
269-
},
270-
{
271-
source: 'proj',
272-
target: 'proj2',
273-
sourceFile: 'libs/proj/package.json',
274-
type: 'static',
275-
},
276-
{
277-
source: 'proj',
278-
sourceFile: 'libs/proj/package.json',
279-
target: 'npm:proj4', // correctly resolves to the npm package of the project rather than the workspace project
280-
type: 'static',
281-
},
282-
{
283-
sourceFile: 'libs/proj/package.json',
284-
source: 'proj',
285-
// Preferred the cached version of lodash, instead of 4.0.0 resolved from tempFs node_modules
286-
target: 'npm:[email protected]',
287-
type: 'static',
288-
},
289-
{
290-
sourceFile: 'libs/proj3/package.json',
291-
source: 'proj3',
292-
// Preferred the cached version of lodash, instead of 3.0.0 resolved from tempFs node_modules
293-
target: 'npm:[email protected]',
294-
type: 'static',
295-
},
296-
{
297-
source: 'proj3',
298-
sourceFile: 'libs/proj3/package.json',
299-
target: 'proj4', // correctly resolves to the workspace dependency of the project in the workspace
300-
type: 'static',
301-
},
302-
]);
305+
expect(res).toContainEqual({
306+
sourceFile: 'libs/proj/package.json',
307+
source: 'proj',
308+
// Preferred the cached version of lodash, instead of 4.0.0 resolved from tempFs node_modules
309+
target: 'npm:[email protected]',
310+
type: 'static',
311+
});
312+
expect(res).toContainEqual({
313+
sourceFile: 'libs/proj3/package.json',
314+
source: 'proj3',
315+
// Preferred the cached version of lodash, instead of 3.0.0 resolved from tempFs node_modules
316+
target: 'npm:[email protected]',
317+
type: 'static',
318+
});
303319
});
304320

305321
it('should add correct npm dependencies for projects that use an older installed version of a package that exists in the workspace', async () => {
@@ -322,4 +338,46 @@ describe('explicit package json dependencies', () => {
322338
])
323339
);
324340
});
341+
342+
it('should add correct workspace dependencies for projects that use a range for packages which exist in the workspace', async () => {
343+
const npmResolutionCache = new Map();
344+
const targetProjectLocator = new TargetProjectLocator(
345+
projects,
346+
ctx.externalNodes,
347+
npmResolutionCache
348+
);
349+
350+
const res = buildExplicitPackageJsonDependencies(ctx, targetProjectLocator);
351+
expect(res).toEqual(
352+
expect.arrayContaining([
353+
{
354+
source: 'proj',
355+
sourceFile: 'libs/proj/package.json',
356+
target: 'proj5', // correctly resolves to the npm package of the project rather than the workspace project
357+
type: 'static',
358+
},
359+
])
360+
);
361+
});
362+
363+
it('should add correct workspace dependencies for projects that use a file reference of a package that exists in the workspace', async () => {
364+
const npmResolutionCache = new Map();
365+
const targetProjectLocator = new TargetProjectLocator(
366+
projects,
367+
ctx.externalNodes,
368+
npmResolutionCache
369+
);
370+
371+
const res = buildExplicitPackageJsonDependencies(ctx, targetProjectLocator);
372+
expect(res).toEqual(
373+
expect.arrayContaining([
374+
{
375+
source: 'proj',
376+
sourceFile: 'libs/proj/package.json',
377+
target: 'proj6', // correctly resolves to the npm package of the project rather than the workspace project
378+
type: 'static',
379+
},
380+
])
381+
);
382+
});
325383
});

‎packages/nx/src/plugins/js/project-graph/build-dependencies/explicit-package-json-dependencies.ts‎

Lines changed: 12 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -42,23 +42,27 @@ function isPackageJsonAtProjectRoot(
4242

4343
function processPackageJson(
4444
sourceProject: string,
45-
fileName: string,
45+
packageJsonPath: string,
4646
ctx: CreateDependenciesContext,
4747
targetProjectLocator: TargetProjectLocator,
4848
collectedDeps: RawProjectGraphDependency[]
4949
) {
5050
try {
51-
const deps = readDeps(parseJson(defaultFileRead(fileName)));
51+
const deps = readDeps(parseJson(defaultFileRead(packageJsonPath)));
5252

53-
for (const d of Object.keys(deps)) {
53+
for (const [packageName, packageVersion] of Object.entries(deps)) {
5454
const localProject =
55-
targetProjectLocator.findDependencyInWorkspaceProjects(d);
55+
targetProjectLocator.findDependencyInWorkspaceProjects(
56+
packageJsonPath,
57+
packageName,
58+
packageVersion
59+
);
5660
if (localProject) {
5761
// package.json refers to another project in the monorepo
5862
const dependency: RawProjectGraphDependency = {
5963
source: sourceProject,
6064
target: localProject,
61-
sourceFile: fileName,
65+
sourceFile: packageJsonPath,
6266
type: DependencyType.static,
6367
};
6468
validateDependency(dependency, ctx);
@@ -67,8 +71,8 @@ function processPackageJson(
6771
}
6872

6973
const externalNodeName = targetProjectLocator.findNpmProjectFromImport(
70-
d,
71-
fileName
74+
packageName,
75+
packageJsonPath
7276
);
7377
if (!externalNodeName) {
7478
continue;
@@ -77,7 +81,7 @@ function processPackageJson(
7781
const dependency: RawProjectGraphDependency = {
7882
source: sourceProject,
7983
target: externalNodeName,
80-
sourceFile: fileName,
84+
sourceFile: packageJsonPath,
8185
type: DependencyType.static,
8286
};
8387
validateDependency(dependency, ctx);

‎packages/nx/src/plugins/js/project-graph/build-dependencies/target-project-locator.spec.ts‎

Lines changed: 10 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1116,8 +1116,11 @@ describe('TargetProjectLocator', () => {
11161116
{},
11171117
new Map()
11181118
);
1119-
const result =
1120-
targetProjectLocator.findDependencyInWorkspaceProjects('@org/pkg1');
1119+
const result = targetProjectLocator.findDependencyInWorkspaceProjects(
1120+
'',
1121+
'@org/pkg1',
1122+
'*'
1123+
);
11211124

11221125
expect(result).toEqual('pkg1');
11231126
}
@@ -1145,8 +1148,11 @@ describe('TargetProjectLocator', () => {
11451148
{},
11461149
new Map()
11471150
);
1148-
const result =
1149-
targetProjectLocator.findDependencyInWorkspaceProjects('@org/pkg2');
1151+
const result = targetProjectLocator.findDependencyInWorkspaceProjects(
1152+
'',
1153+
'@org/pkg2',
1154+
'*'
1155+
);
11501156

11511157
expect(result).toBeFalsy();
11521158
});

0 commit comments

Comments
 (0)