Skip to content

Commit 1600875

Browse files
AgentEnderJamesHenry
AgentEnder
and
JamesHenry
authored
fix(core): don't infer scripts as targets if sibling project json declares them (#26464)
<!-- Please make sure you have read the submission guidelines before posting an PR --> <!-- https://github.com/nrwl/nx/blob/master/CONTRIBUTING.md#-submitting-a-pr --> <!-- Please make sure that your commit message follows our format --> <!-- Example: `fix(nx): must begin with lowercase` --> ## Current Behavior Inferring some scripts during package-json-workspaces causes some thrashing ## Expected Behavior We only infer a script as a target if its not in project.json ## Related Issue(s) <!-- Please link the issue being fixed so it gets closed when this is merged. --> Fixes # --------- Co-authored-by: “JamesHenry” <[email protected]>
1 parent 3689c88 commit 1600875

File tree

4 files changed

+219
-16
lines changed

4 files changed

+219
-16
lines changed

‎packages/nx/src/generators/utils/project-configuration.ts

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -217,6 +217,7 @@ function readAndCombineAllProjectConfigurations(tree: Tree): {
217217
const packageJson = readJson<PackageJson>(tree, projectFile);
218218
const config = buildProjectConfigurationFromPackageJson(
219219
packageJson,
220+
tree.root,
220221
projectFile,
221222
readNxJson(tree)
222223
);

‎packages/nx/src/plugins/package-json-workspaces/create-nodes.spec.ts

Lines changed: 184 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,12 @@ import { vol } from 'memfs';
44
import { createNodeFromPackageJson, createNodes } from './create-nodes';
55

66
describe('nx package.json workspaces plugin', () => {
7+
const context = {
8+
workspaceRoot: '/root',
9+
configFiles: [],
10+
nxJsonConfiguration: {},
11+
};
12+
713
afterEach(() => {
814
vol.reset();
915
});
@@ -382,12 +388,6 @@ describe('nx package.json workspaces plugin', () => {
382388
'/root'
383389
);
384390

385-
const context = {
386-
workspaceRoot: '/root',
387-
configFiles: [],
388-
nxJsonConfiguration: {},
389-
};
390-
391391
// No matching project based on the lerna.json "packages" config
392392
expect(
393393
createNodes[1]('package.json', undefined, context)
@@ -446,4 +446,182 @@ describe('nx package.json workspaces plugin', () => {
446446
).toMatchInlineSnapshot(`{}`);
447447
});
448448
});
449+
450+
describe('sibling project.json files', () => {
451+
it('should add a script target if the sibling project.json file does not exist', () => {
452+
vol.fromJSON(
453+
{
454+
'package.json': JSON.stringify({
455+
name: 'root',
456+
workspaces: ['packages/*'],
457+
}),
458+
'packages/a/package.json': JSON.stringify({
459+
name: 'root',
460+
scripts: {
461+
build: 'echo build',
462+
},
463+
}),
464+
},
465+
'/root'
466+
);
467+
468+
expect(createNodes[1]('packages/a/package.json', undefined, context))
469+
.toMatchInlineSnapshot(`
470+
{
471+
"projects": {
472+
"packages/a": {
473+
"metadata": {
474+
"targetGroups": {
475+
"NPM Scripts": [
476+
"build",
477+
],
478+
},
479+
},
480+
"name": "root",
481+
"projectType": "library",
482+
"root": "packages/a",
483+
"sourceRoot": "packages/a",
484+
"targets": {
485+
"build": {
486+
"executor": "nx:run-script",
487+
"metadata": {
488+
"runCommand": "npm run build",
489+
"scriptContent": "echo build",
490+
},
491+
"options": {
492+
"script": "build",
493+
},
494+
},
495+
"nx-release-publish": {
496+
"dependsOn": [
497+
"^nx-release-publish",
498+
],
499+
"executor": "@nx/js:release-publish",
500+
"options": {},
501+
},
502+
},
503+
},
504+
},
505+
}
506+
`);
507+
});
508+
509+
it('should add a script target if the sibling project.json exists but does not have a conflicting target', () => {
510+
vol.fromJSON(
511+
{
512+
'package.json': JSON.stringify({
513+
name: 'root',
514+
workspaces: ['packages/*'],
515+
}),
516+
'packages/a/package.json': JSON.stringify({
517+
name: 'root',
518+
scripts: {
519+
build: 'echo build',
520+
},
521+
}),
522+
'packages/a/project.json': JSON.stringify({
523+
targets: {
524+
'something-other-than-build': {
525+
command: 'echo something-other-than-build',
526+
},
527+
},
528+
}),
529+
},
530+
'/root'
531+
);
532+
533+
expect(createNodes[1]('packages/a/package.json', undefined, context))
534+
.toMatchInlineSnapshot(`
535+
{
536+
"projects": {
537+
"packages/a": {
538+
"metadata": {
539+
"targetGroups": {
540+
"NPM Scripts": [
541+
"build",
542+
],
543+
},
544+
},
545+
"name": "root",
546+
"projectType": "library",
547+
"root": "packages/a",
548+
"sourceRoot": "packages/a",
549+
"targets": {
550+
"build": {
551+
"executor": "nx:run-script",
552+
"metadata": {
553+
"runCommand": "npm run build",
554+
"scriptContent": "echo build",
555+
},
556+
"options": {
557+
"script": "build",
558+
},
559+
},
560+
"nx-release-publish": {
561+
"dependsOn": [
562+
"^nx-release-publish",
563+
],
564+
"executor": "@nx/js:release-publish",
565+
"options": {},
566+
},
567+
},
568+
},
569+
},
570+
}
571+
`);
572+
});
573+
574+
it('should not add a script target if the sibling project.json exists and has a conflicting target', () => {
575+
vol.fromJSON(
576+
{
577+
'package.json': JSON.stringify({
578+
name: 'root',
579+
workspaces: ['packages/*'],
580+
}),
581+
'packages/a/package.json': JSON.stringify({
582+
name: 'root',
583+
scripts: {
584+
build: 'echo "build from package.json"',
585+
},
586+
}),
587+
'packages/a/project.json': JSON.stringify({
588+
targets: {
589+
build: {
590+
command: 'echo "build from project.json"',
591+
},
592+
},
593+
}),
594+
},
595+
'/root'
596+
);
597+
598+
expect(createNodes[1]('packages/a/package.json', undefined, context))
599+
.toMatchInlineSnapshot(`
600+
{
601+
"projects": {
602+
"packages/a": {
603+
"metadata": {
604+
"targetGroups": {
605+
"NPM Scripts": [],
606+
},
607+
},
608+
"name": "root",
609+
"projectType": "library",
610+
"root": "packages/a",
611+
"sourceRoot": "packages/a",
612+
"targets": {
613+
"nx-release-publish": {
614+
"dependsOn": [
615+
"^nx-release-publish",
616+
],
617+
"executor": "@nx/js:release-publish",
618+
"options": {},
619+
},
620+
},
621+
},
622+
},
623+
}
624+
`);
625+
});
626+
});
449627
});

‎packages/nx/src/plugins/package-json-workspaces/create-nodes.ts

Lines changed: 33 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -68,12 +68,16 @@ export function buildPackageJsonWorkspacesMatcher(
6868
negativePatterns.every((negative) => minimatch(p, negative));
6969
}
7070

71-
export function createNodeFromPackageJson(pkgJsonPath: string, root: string) {
72-
const json: PackageJson = readJsonFile(join(root, pkgJsonPath));
71+
export function createNodeFromPackageJson(
72+
pkgJsonPath: string,
73+
workspaceRoot: string
74+
) {
75+
const json: PackageJson = readJsonFile(join(workspaceRoot, pkgJsonPath));
7376
const project = buildProjectConfigurationFromPackageJson(
7477
json,
78+
workspaceRoot,
7579
pkgJsonPath,
76-
readNxJson(root)
80+
readNxJson(workspaceRoot)
7781
);
7882
return {
7983
projects: {
@@ -84,13 +88,24 @@ export function createNodeFromPackageJson(pkgJsonPath: string, root: string) {
8488

8589
export function buildProjectConfigurationFromPackageJson(
8690
packageJson: PackageJson,
87-
path: string,
91+
workspaceRoot: string,
92+
packageJsonPath: string,
8893
nxJson: NxJsonConfiguration
8994
): ProjectConfiguration & { name: string } {
90-
const normalizedPath = path.split('\\').join('/');
91-
const directory = dirname(normalizedPath);
95+
const normalizedPath = packageJsonPath.split('\\').join('/');
96+
const projectRoot = dirname(normalizedPath);
9297

93-
if (!packageJson.name && directory === '.') {
98+
const siblingProjectJson = tryReadJson<ProjectConfiguration>(
99+
join(workspaceRoot, projectRoot, 'project.json')
100+
);
101+
102+
if (siblingProjectJson) {
103+
for (const target of Object.keys(siblingProjectJson?.targets ?? {})) {
104+
delete packageJson.scripts?.[target];
105+
}
106+
}
107+
108+
if (!packageJson.name && projectRoot === '.') {
94109
throw new Error(
95110
'Nx requires the root package.json to specify a name if it is being used as an Nx project.'
96111
);
@@ -100,13 +115,13 @@ export function buildProjectConfigurationFromPackageJson(
100115
const projectType =
101116
nxJson?.workspaceLayout?.appsDir != nxJson?.workspaceLayout?.libsDir &&
102117
nxJson?.workspaceLayout?.appsDir &&
103-
directory.startsWith(nxJson.workspaceLayout.appsDir)
118+
projectRoot.startsWith(nxJson.workspaceLayout.appsDir)
104119
? 'application'
105120
: 'library';
106121

107122
return {
108-
root: directory,
109-
sourceRoot: directory,
123+
root: projectRoot,
124+
sourceRoot: projectRoot,
110125
name,
111126
projectType,
112127
...packageJson.nx,
@@ -185,3 +200,11 @@ function normalizePatterns(patterns: string[]): string[] {
185200
function removeRelativePath(pattern: string): string {
186201
return pattern.startsWith('./') ? pattern.substring(2) : pattern;
187202
}
203+
204+
function tryReadJson<T extends Object = any>(path: string): T | null {
205+
try {
206+
return readJsonFile<T>(path);
207+
} catch {
208+
return null;
209+
}
210+
}

‎packages/nx/src/project-graph/file-utils.ts

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -215,6 +215,7 @@ function getProjectsSync(
215215
const packageJson = readJsonFile<PackageJson>(projectFile);
216216
const config = buildProjectConfigurationFromPackageJson(
217217
packageJson,
218+
root,
218219
projectFile,
219220
nxJson
220221
);

0 commit comments

Comments
 (0)