Skip to content

Commit

Permalink
fix(nx-plugin): deep merge executor options (#927)
Browse files Browse the repository at this point in the history
  • Loading branch information
hanna-skryl authored Jan 28, 2025
1 parent e68ce12 commit 3dcc4b1
Show file tree
Hide file tree
Showing 7 changed files with 114 additions and 6 deletions.
43 changes: 42 additions & 1 deletion e2e/nx-plugin-e2e/tests/executor-cli.e2e.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,10 @@ import { type Tree, updateProjectConfiguration } from '@nx/devkit';
import path from 'node:path';
import { readProjectConfiguration } from 'nx/src/generators/utils/project-configuration';
import { afterEach, expect } from 'vitest';
import { generateCodePushupConfig } from '@code-pushup/nx-plugin';
import {
type AutorunCommandExecutorOptions,
generateCodePushupConfig,
} from '@code-pushup/nx-plugin';
import {
generateWorkspaceAndProject,
materializeTree,
Expand All @@ -20,6 +23,7 @@ import { INLINE_PLUGIN } from './inline-plugin.js';
async function addTargetToWorkspace(
tree: Tree,
options: { cwd: string; project: string },
executorOptions?: AutorunCommandExecutorOptions,
) {
const { cwd, project } = options;
const projectCfg = readProjectConfiguration(tree, project);
Expand All @@ -29,6 +33,7 @@ async function addTargetToWorkspace(
...projectCfg.targets,
'code-pushup': {
executor: '@code-pushup/nx-plugin:cli',
...(executorOptions && { options: executorOptions }),
},
},
});
Expand Down Expand Up @@ -95,6 +100,42 @@ describe('executor command', () => {
).rejects.toThrow('');
});

it('should execute collect executor and merge target and command-line options', async () => {
const cwd = path.join(testFileDir, 'execute-collect-with-merged-options');
await addTargetToWorkspace(
tree,
{ cwd, project },
{
persist: {
outputDir: '.reports',
filename: 'report',
},
},
);

const { stdout, code } = await executeProcess({
command: 'npx',
args: [
'nx',
'run',
`${project}:code-pushup`,
'collect',
'--persist.filename=terminal-report',
],
cwd,
});

expect(code).toBe(0);
const cleanStdout = removeColorCodes(stdout);
expect(cleanStdout).toContain(
'nx run my-lib:code-pushup collect --persist.filename=terminal-report',
);

await expect(
readJsonFile(path.join(cwd, '.reports', 'terminal-report.json')),
).resolves.not.toThrow();
});

it('should execute collect executor and add report to sub folder named by project', async () => {
const cwd = path.join(testFileDir, 'execute-collect-command');
await addTargetToWorkspace(tree, { cwd, project });
Expand Down
2 changes: 1 addition & 1 deletion packages/nx-plugin/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,7 @@ Examples:
#### CLI

Install JS packages configure a target in your project json.
See [CLI executor docs](./src/executor/cli/README.md) for details
See [CLI executor docs](./src/executors/cli/README.md) for details

Examples:

Expand Down
2 changes: 1 addition & 1 deletion packages/nx-plugin/src/executors/cli/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -74,4 +74,4 @@ Show what will be executed without actually executing it:
| **dryRun** | `boolean` | To debug the executor, dry run the command without real execution. |
| **bin** | `string` | Path to Code PushUp CLI |

For all other options see the [CLI autorun documentation](../../cli/packages/cli/README.md#autorun-command).
For all other options see the [CLI autorun documentation](../../../../cli/README.md#autorun-command).
10 changes: 7 additions & 3 deletions packages/nx-plugin/src/executors/cli/executor.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ import { execSync } from 'node:child_process';
import { createCliCommand } from '../internal/cli.js';
import { normalizeContext } from '../internal/context.js';
import type { AutorunCommandExecutorOptions } from './schema.js';
import { parseAutorunExecutorOptions } from './utils.js';
import { mergeExecutorOptions, parseAutorunExecutorOptions } from './utils.js';

export type ExecutorOutput = {
success: boolean;
Expand All @@ -16,11 +16,15 @@ export default function runAutorunExecutor(
context: ExecutorContext,
): Promise<ExecutorOutput> {
const normalizedContext = normalizeContext(context);
const cliArgumentObject = parseAutorunExecutorOptions(
const mergedOptions = mergeExecutorOptions(
context.target?.options,
terminalAndExecutorOptions,
);
const cliArgumentObject = parseAutorunExecutorOptions(
mergedOptions,
normalizedContext,
);
const { dryRun, verbose, command } = terminalAndExecutorOptions;
const { dryRun, verbose, command } = mergedOptions;

const commandString = createCliCommand({ command, args: cliArgumentObject });
const commandStringOptions = context.cwd ? { cwd: context.cwd } : {};
Expand Down
38 changes: 38 additions & 0 deletions packages/nx-plugin/src/executors/cli/utils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -39,3 +39,41 @@ export function parseAutorunExecutorOptions(
: undefined,
};
}

/**
* Deeply merges executor options.
*
* @param targetOptions - The original options from the target configuration.
* @param cliOptions - The options from Nx, combining target options and CLI arguments.
* @returns A new object with deeply merged properties.
*
* Nx performs a shallow merge by default, where command-line arguments can override entire objects
* (e.g., `--persist.filename` replaces the entire `persist` object).
* This function ensures that nested properties are deeply merged,
* preserving the original target options where CLI arguments are not provided.
*/
export function mergeExecutorOptions(
targetOptions: Partial<AutorunCommandExecutorOptions>,
cliOptions: Partial<AutorunCommandExecutorOptions>,
): AutorunCommandExecutorOptions {
return {
...targetOptions,
...cliOptions,
...(targetOptions?.persist || cliOptions?.persist
? {
persist: {
...targetOptions?.persist,
...cliOptions?.persist,
},
}
: {}),
...(targetOptions?.upload || cliOptions?.upload
? {
upload: {
...targetOptions?.upload,
...cliOptions?.upload,
},
}
: {}),
};
}
24 changes: 24 additions & 0 deletions packages/nx-plugin/src/executors/cli/utils.unit.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ import { type MockInstance, expect, vi } from 'vitest';
import { osAgnosticPath } from '@code-pushup/test-utils';
import type { Command } from '../internal/types.js';
import {
mergeExecutorOptions,
parseAutorunExecutorOnlyOptions,
parseAutorunExecutorOptions,
} from './utils.js';
Expand Down Expand Up @@ -154,3 +155,26 @@ describe('parseAutorunExecutorOptions', () => {
},
);
});

describe('mergeExecutorOptions', () => {
it('should deeply merge target and CLI options', () => {
const targetOptions = {
persist: {
outputDir: '.reports',
filename: 'report',
},
};
const cliOptions = {
persist: {
filename: 'report-file',
},
};
const expected = {
persist: {
outputDir: '.reports',
filename: 'report-file',
},
};
expect(mergeExecutorOptions(targetOptions, cliOptions)).toEqual(expected);
});
});
1 change: 1 addition & 0 deletions packages/nx-plugin/src/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -15,3 +15,4 @@ export {
type ProcessConfig,
} from './internal/execute-process.js';
export { objectToCliArgs } from './executors/internal/cli.js';
export type { AutorunCommandExecutorOptions } from './executors/cli/schema.js';

0 comments on commit 3dcc4b1

Please sign in to comment.