Skip to content

Commit

Permalink
fix(core): merge args and options in nx:run-commands executor (#26573)
Browse files Browse the repository at this point in the history
<!-- 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` -->

<!-- If this is a particularly complex change or feature addition, you
can request a dedicated Nx release for this pull request branch. Mention
someone from the Nx team or the `@nrwl/nx-pipelines-reviewers` and they
will confirm if the PR warrants its own release for testing purposes,
and generate it for you if appropriate. -->

## Current Behavior
<!-- This is the behavior we have today -->

When a target using `nx:run-commands` has the `args` option set, the
rest of the options are completely ignored and not forwarded to the
command.

## Expected Behavior
<!-- This is the behavior we should expect with the changes in this PR
-->

When a target using `nx:run-commands` has the `args` option set, the
rest of the options that don't match any of the options set in `args`
and are not specific to the executor should be forwarded to the command.

## Related Issue(s)
<!-- Please link the issue being fixed so it gets closed when this is
merged. -->
<!-- Fixes NXP-811 -->

Fixes #
  • Loading branch information
leosvelperez authored Jun 17, 2024
1 parent 4456526 commit 59ab43a
Show file tree
Hide file tree
Showing 2 changed files with 27 additions and 7 deletions.
17 changes: 16 additions & 1 deletion packages/nx/src/executors/run-commands/run-commands.impl.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -64,7 +64,7 @@ describe('Run Commands', () => {
}
);

it('should overwrite options with args', async () => {
it('should overwrite matching options with args', async () => {
let result = (
await runCommands(
{
Expand Down Expand Up @@ -115,6 +115,21 @@ describe('Run Commands', () => {
).terminalOutput.trim();
expect(result).not.toContain('--key=123');
expect(result).toContain('--key=789'); // should take args over unknown options

result = (
await runCommands(
{
command: 'echo',
__unparsed__: [],
key1: 'from options',
key2: 'from options',
args: '--key1="from args"',
},
context
)
).terminalOutput.trim();
expect(result).not.toContain('--key1="from options"');
expect(result).toContain('echo --key2="from options" --key1="from args"'); // take args over options with the same name while keeping the rest
});

it('should not foward any args to underlying command if forwardAllArgs is false', async () => {
Expand Down
17 changes: 11 additions & 6 deletions packages/nx/src/executors/run-commands/run-commands.impl.ts
Original file line number Diff line number Diff line change
Expand Up @@ -516,12 +516,12 @@ export function interpolateArgsIntoCommand(
args += ` ${opts.args}`;
}
if (opts.__unparsed__?.length > 0) {
const filterdParsedOptions = filterPropKeysFromUnParsedOptions(
const filteredParsedOptions = filterPropKeysFromUnParsedOptions(
opts.__unparsed__,
opts.parsedArgs
);
if (filterdParsedOptions.length > 0) {
args += ` ${filterdParsedOptions
if (filteredParsedOptions.length > 0) {
args += ` ${filteredParsedOptions
.map(wrapArgIntoQuotesIfNeeded)
.join(' ')}`;
}
Expand All @@ -540,9 +540,14 @@ function parseArgs(
if (!args) {
return { ...unknownOptions, ...unparsedCommandArgs };
}
return yargsParser(args.replace(/(^"|"$)/g, ''), {
configuration: { 'camel-case-expansion': false },
});

return {
...unknownOptions,
...yargsParser(args.replace(/(^"|"$)/g, ''), {
configuration: { 'camel-case-expansion': true },
}),
...unparsedCommandArgs,
};
}

/**
Expand Down

0 comments on commit 59ab43a

Please sign in to comment.