Skip to content

Commit

Permalink
fix(core): address some wonkiness when merging command and run-comman…
Browse files Browse the repository at this point in the history 
…ds (#21315)
  • Loading branch information
@AgentEnder
AgentEnder authored Jan 29, 2024
1 parent 8274f34 commit c811be5
Show file tree
Hide file tree
Showing 5 changed files with 201 additions and 128 deletions.
13 changes: 11 additions & 2 deletions packages/nx/src/generators/utils/project-configuration.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -211,7 +211,13 @@ function readAndCombineAllProjectConfigurations(tree: Tree): {
if (basename(projectFile) === 'project.json') {
const json = readJson(tree, projectFile);
const config = buildProjectFromProjectJson(json, projectFile);
mergeProjectConfigurationIntoRootMap(rootMap, config);
mergeProjectConfigurationIntoRootMap(
rootMap,
config,
undefined,
undefined,
true
);
} else if (basename(projectFile) === 'package.json') {
const packageJson = readJson<PackageJson>(tree, projectFile);
const config = buildProjectConfigurationFromPackageJson(
Expand All @@ -227,7 +233,10 @@ function readAndCombineAllProjectConfigurations(tree: Tree): {
{
name: config.name,
root: config.root,
}
},
undefined,
undefined,
true
);
}
}
Expand Down
22 changes: 0 additions & 22 deletions packages/nx/src/project-graph/utils/normalize-project-nodes.spec.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -80,28 +80,6 @@ describe('workspace-projects', () => {
});

describe('normalizeTargets', () => {
it('should convert command property to run-commands executor', () => {
expect(
normalizeProjectTargets(
{
root: 'my/project',
targets: {
build: {
command: 'echo',
},
},
},
'build'
).build
).toEqual({
executor: 'nx:run-commands',
configurations: {},
options: {
command: 'echo',
},
});
});

it('should support {projectRoot}, {workspaceRoot}, and {projectName} tokens', () => {
expect(
normalizeProjectTargets(
Expand Down
31 changes: 0 additions & 31 deletions packages/nx/src/project-graph/utils/normalize-project-nodes.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -94,11 +94,6 @@ export function normalizeProjectTargets(
continue;
}

targets[target] = resolveCommandSyntacticSugar(
targets[target],
`${projectName}:${target}`
);

targets[target].options = resolveNxTokensInOptions(
targets[target].options,
project,
Expand Down Expand Up @@ -134,29 +129,3 @@ export function normalizeImplicitDependencies(
.concat(implicitDependencies.filter((x) => x.startsWith('!')))
);
}

function resolveCommandSyntacticSugar(
target: TargetConfiguration,
key: string
): TargetConfiguration {
const { command, ...config } = target ?? {};

if (!command) {
return target;
}

if (config.executor) {
throw new Error(
`${NX_PREFIX} ${key} should not have executor and command both configured.`
);
} else {
return {
...config,
executor: 'nx:run-commands',
options: {
...config.options,
command: command,
},
};
}
}
150 changes: 112 additions & 38 deletions packages/nx/src/project-graph/utils/project-configuration-utils.spec.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -303,6 +303,106 @@ describe('project-configuration-utils', () => {
).toEqual(projectDefaultConfiguration);
});
});

describe('run-commands', () => {
it('should merge two run-commands targets appropriately', () => {
const merged = mergeTargetConfigurations(
{
outputs: ['{projectRoot}/outputfile.json'],
options: {
command: 'eslint . -o outputfile.json',
},
},
{
cache: true,
inputs: [
'default',
'{workspaceRoot}/.eslintrc.json',
'{workspaceRoot}/apps/third-app/.eslintrc.json',
'{workspaceRoot}/tools/eslint-rules/**/*',
{ externalDependencies: ['eslint'] },
],
options: { cwd: 'apps/third-app', command: 'eslint .' },
executor: 'nx:run-commands',
configurations: {},
}
);
expect(merged).toMatchInlineSnapshot(`
{
"cache": true,
"configurations": {},
"executor": "nx:run-commands",
"inputs": [
"default",
"{workspaceRoot}/.eslintrc.json",
"{workspaceRoot}/apps/third-app/.eslintrc.json",
"{workspaceRoot}/tools/eslint-rules/**/*",
{
"externalDependencies": [
"eslint",
],
},
],
"options": {
"command": "eslint . -o outputfile.json",
"cwd": "apps/third-app",
},
"outputs": [
"{projectRoot}/outputfile.json",
],
}
`);
});

it('should merge targets when the base uses command syntactic sugar', () => {
const merged = mergeTargetConfigurations(
{
outputs: ['{projectRoot}/outputfile.json'],
options: {
command: 'eslint . -o outputfile.json',
},
},
{
cache: true,
inputs: [
'default',
'{workspaceRoot}/.eslintrc.json',
'{workspaceRoot}/apps/third-app/.eslintrc.json',
'{workspaceRoot}/tools/eslint-rules/**/*',
{ externalDependencies: ['eslint'] },
],
options: { cwd: 'apps/third-app' },
configurations: {},
command: 'eslint .',
}
);
expect(merged).toMatchInlineSnapshot(`
{
"cache": true,
"command": "eslint .",
"configurations": {},
"inputs": [
"default",
"{workspaceRoot}/.eslintrc.json",
"{workspaceRoot}/apps/third-app/.eslintrc.json",
"{workspaceRoot}/tools/eslint-rules/**/*",
{
"externalDependencies": [
"eslint",
],
},
],
"options": {
"command": "eslint . -o outputfile.json",
"cwd": "apps/third-app",
},
"outputs": [
"{projectRoot}/outputfile.json",
],
}
`);
});
});
});

describe('mergeProjectConfigurationIntoRootMap', () => {
Expand Down Expand Up @@ -333,7 +433,10 @@ describe('project-configuration-utils', () => {
"root": "libs/lib-a",
"targets": {
"build": {
"command": "tsc",
"executor": "nx:run-commands",
"options": {
"command": "tsc",
},
},
"echo": {
"command": "echo lib-a",
Expand Down Expand Up @@ -1081,18 +1184,10 @@ describe('project-configuration-utils', () => {
expect(
isCompatibleTarget(
{
command: 'echo',
},
{
command: 'echo',
}
)
).toBe(true);

expect(
isCompatibleTarget(
{
command: 'echo',
executor: 'nx:run-commands',
options: {
command: 'echo',
},
},
{
executor: 'nx:run-commands',
Expand All @@ -1108,18 +1203,10 @@ describe('project-configuration-utils', () => {
expect(
isCompatibleTarget(
{
command: 'echo',
},
{
command: 'echo2',
}
)
).toBe(false);

expect(
isCompatibleTarget(
{
command: 'echo',
executor: 'nx:run-commands',
options: {
command: 'echo',
},
},
{
executor: 'nx:run-commands',
Expand All @@ -1130,19 +1217,6 @@ describe('project-configuration-utils', () => {
)
).toBe(false);
});

it('should return false if one target specifies an executor and the other a command', () => {
expect(
isCompatibleTarget(
{
executor: 'nx:noop',
},
{
command: 'echo',
}
)
).toBe(false);
});
});
});

Expand Down
Loading

1 comment on commit c811be5

@vercel
Copy link

@vercel vercel bot commented on c811be5 Jan 29, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Successfully deployed to the following URLs:

nx-dev – ./

nx-five.vercel.app
nx-dev-git-master-nrwl.vercel.app
nx-dev-nrwl.vercel.app
nx.dev

Please sign in to comment.