Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

fix(core): prevent target defaults from being discarded during merge process #21624

Merged
merged 1 commit into from Feb 5, 2024
Merged

fix(core): prevent target defaults from being discarded during merge process #21624

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
108 changes: 107 additions & 1 deletion packages/nx/src/plugins/target-defaults/target-defaults-plugin.spec.ts
View file
Open in desktop
Expand Up @@ -2,7 +2,7 @@ import * as memfs from 'memfs';

import '../../../src/internal-testing-utils/mock-fs';

import { TargetDefaultsPlugin } from './target-defaults-plugin';
import { getTargetInfo, TargetDefaultsPlugin } from './target-defaults-plugin';
import { CreateNodesContext } from '../../utils/nx-plugin';
const {
createNodes: [, createNodesFn],
Expand Down Expand Up @@ -74,6 +74,7 @@ describe('target-defaults plugin', () => {
"dependsOn": [
"^build",
],
"executor": "nx:run-commands",
},
},
},
Expand Down Expand Up @@ -114,6 +115,10 @@ describe('target-defaults plugin', () => {
"targets": {
"test": {
"command": "jest",
"executor": "nx:run-script",
"options": {
"script": "test",
},
},
},
},
Expand Down Expand Up @@ -156,6 +161,10 @@ describe('target-defaults plugin', () => {
"targets": {
"test": {
"command": "jest",
"executor": "nx:run-script",
"options": {
"script": "test",
},
},
},
},
Expand Down Expand Up @@ -225,6 +234,10 @@ describe('target-defaults plugin', () => {
"targets": {
"test": {
"command": "jest",
"executor": "nx:run-script",
"options": {
"script": "test",
},
Symbol(ONLY_MODIFIES_EXISTING_TARGET): true,
},
},
Expand Down Expand Up @@ -274,11 +287,13 @@ describe('target-defaults plugin', () => {
".": {
"targets": {
"echo": {
"executor": "nx:run-commands",
"options": {
"cwd": "{projectRoot}",
},
},
"echo2": {
"executor": "nx:run-commands",
"options": {
"cwd": "{projectRoot}",
},
Expand Down Expand Up @@ -336,11 +351,13 @@ describe('target-defaults plugin', () => {
".": {
"targets": {
"echo": {
"executor": "nx:run-commands",
"options": {
"cwd": "{projectRoot}",
},
},
"echo2": {
"executor": "nx:run-commands",
"options": {
"cwd": "{projectRoot}",
},
Expand All @@ -358,4 +375,93 @@ describe('target-defaults plugin', () => {
`);
});
});

describe('get target info', () => {
it('should include command for single command', () => {
const result = getTargetInfo(
'echo',
{
targets: {
echo: {
command: 'echo hi',
},
},
},
null
);
expect(result).toMatchInlineSnapshot(`
{
"command": "echo hi",
}
`);
});

it('should include command for run-commands', () => {
const result = getTargetInfo(
'echo',
{
targets: {
echo: {
executor: 'nx:run-commands',
options: {
command: 'echo hi',
cwd: '{projectRoot}',
},
},
},
},
null
);
expect(result).toMatchInlineSnapshot(`
{
"executor": "nx:run-commands",
"options": {
"command": "echo hi",
},
}
`);
});

it('should include script for run-script', () => {
expect(
getTargetInfo('build', null, {
scripts: {
build: 'echo hi',
},
})
).toMatchInlineSnapshot(`
{
"executor": "nx:run-script",
"options": {
"script": "build",
},
}
`);

expect(
getTargetInfo('echo', null, {
scripts: {
build: 'echo hi',
},
nx: {
targets: {
echo: {
executor: 'nx:run-script',
options: {
script: 'build',
},
},
},
},
})
).toMatchInlineSnapshot(`
{
"executor": "nx:run-script",
"options": {
"script": "build",
},
}
`);
});
});
});
96 changes: 95 additions & 1 deletion packages/nx/src/plugins/target-defaults/target-defaults-plugin.ts
View file
Open in desktop
Expand Up @@ -88,9 +88,13 @@ export const TargetDefaultsPlugin: NxPluginV2 = {
// Prevents `build` from overwriting `@nx/js:tsc` if both are present
// and build is specified later in the ordering.
if (!modifiedTargets[targetName] || targetName !== defaultSpecifier) {
modifiedTargets[targetName] = JSON.parse(
const defaults = JSON.parse(
JSON.stringify(targetDefaults[defaultSpecifier])
);
modifiedTargets[targetName] = {
...getTargetInfo(targetName, projectJson, packageJson),
...defaults,
};
}
// TODO: Remove this after we figure out a way to define new targets
// in target defaults
Expand Down Expand Up @@ -140,3 +144,93 @@ function readJsonOrNull<T extends Object = any>(path: string) {
return null;
}
}

/**
* This fn gets target info that would make a target uniquely compatible
* with what is described by project.json or package.json. As the merge process
* for config happens, without this, the target defaults may be compatible
* with a config from a plugin and then that combined target be incompatible
* with the project json configuration resulting in the target default values
* being scrapped. By adding enough information from the project.json / package.json,
* we can make sure that the target after merging is compatible with the defined target.
*/
export function getTargetInfo(
target: string,
projectJson: Pick<ProjectConfiguration, 'targets'>,
packageJson: Pick<PackageJson, 'scripts' | 'nx'>
) {
const projectJsonTarget = projectJson?.targets?.[target];
const packageJsonTarget = packageJson?.nx?.targets?.[target];

const executor = getTargetExecutor(target, projectJson, packageJson);
const targetOptions = {
...packageJsonTarget?.options,
...projectJsonTarget?.options,
};

if (projectJsonTarget?.command) {
return {
command: projectJsonTarget?.command,
};
}

if (executor === 'nx:run-commands') {
if (targetOptions?.command) {
return {
executor: 'nx:run-commands',
options: {
command: projectJsonTarget.options?.command,
},
};
} else if (targetOptions?.commands) {
return {
executor: 'nx:run-commands',
options: {
commands: targetOptions.commands,
},
};
}
return {
executor: 'nx:run-commands',
};
}

if (executor === 'nx:run-script') {
return {
executor: 'nx:run-script',
options: {
script: targetOptions?.script ?? target,
},
};
}

if (executor) {
return { executor };
}

return {};
}

function getTargetExecutor(
target: string,
projectJson: Pick<ProjectConfiguration, 'targets'>,
packageJson: Pick<PackageJson, 'scripts' | 'nx'>
) {
const projectJsonTarget = projectJson?.targets?.[target];
const packageJsonTarget = packageJson?.nx?.targets?.[target];
const packageJsonScript = packageJson?.scripts?.[target];

if (projectJsonTarget?.command) {
return 'nx:run-commands';
}

if (
!projectJsonTarget?.executor &&
!packageJsonTarget?.executor &&
packageJsonScript
) {
return 'nx:run-script';
}

return projectJsonTarget?.executor ?? packageJsonTarget?.executor;
}