Skip to content

Commit

Permalink
fix(core): not passing props of run-commands to underlying command
Browse files Browse the repository at this point in the history
  • Loading branch information
xiongemi committed Apr 5, 2024
1 parent 640c61d commit 56160da
Show file tree
Hide file tree
Showing 2 changed files with 113 additions and 8 deletions.
49 changes: 45 additions & 4 deletions packages/nx/src/executors/run-commands/run-commands.impl.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,10 +7,6 @@ import runCommands, {
} from './run-commands.impl';
import { env } from 'npm-run-path';

const {
devDependencies: { nx: version },
} = require('package.json');

function normalize(p: string) {
return p.startsWith('/private') ? p.substring(8) : p;
}
Expand Down Expand Up @@ -40,6 +36,20 @@ describe('Run Commands', () => {
expect(readFile(f)).toEqual('123');
});

it('should not pass --args into underlying command', async () => {
const f = fileSync().name;
const result = await runCommands(
{
command: `echo`,
__unparsed__: ['--args=--key=123'],
args: '--key=123',
unparsedCommandArgs: { args: '--key=123' },
},
context
);
expect(result.terminalOutput.trim()).not.toContain('--args=--key=123');
});

it('should interpolate all unknown args as if they were --args', async () => {
const f = fileSync().name;
const result = await runCommands(
Expand Down Expand Up @@ -180,6 +190,37 @@ describe('Run Commands', () => {
).toEqual('echo one -a=b');
});

it('should not forward all unparsed args when the options is a prop to run command', () => {
expect(
interpolateArgsIntoCommand(
'echo',
{
__unparsed__: ['--args', 'test', 'hello'],
unparsedCommandArgs: { args: 'test' },
} as any,
true
)
).toEqual('echo hello');

expect(
interpolateArgsIntoCommand(
'echo',
{
__unparsed__: ['--args=test', 'hello'],
} as any,
true
)
).toEqual('echo hello');

expect(
interpolateArgsIntoCommand(
'echo',
{ __unparsed__: ['--parallel=true', 'hello'] } as any,
true
)
).toEqual('echo hello');
});

it('should add all args when forwardAllArgs is true', () => {
expect(
interpolateArgsIntoCommand(
Expand Down
72 changes: 68 additions & 4 deletions packages/nx/src/executors/run-commands/run-commands.impl.ts
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ import {
getPseudoTerminal,
PseudoTerminal,
} from '../../tasks-runner/pseudo-terminal';
import { output } from '../../utils/output';

export const LARGE_BUFFER = 1024 * 1000000;

Expand Down Expand Up @@ -84,6 +85,9 @@ export interface NormalizedRunCommandsOptions extends RunCommandsOptions {
parsedArgs: {
[k: string]: any;
};
unparsedCommandArgs?: {
[k: string]: string;
};
args?: string;
}

Expand Down Expand Up @@ -230,6 +234,7 @@ function normalizeOptions(
options.unknownOptions,
options.args as string
);
options.unparsedCommandArgs = unparsedCommandArgs;

(options as NormalizedRunCommandsOptions).commands.forEach((c) => {
c.command = interpolateArgsIntoCommand(
Expand Down Expand Up @@ -301,13 +306,17 @@ async function createProcess(
!isParallel &&
usePty
) {
let terminalOutput = chalk.dim('> ') + commandConfig.command + '\r\n\r\n';
if (streamOutput) {
process.stdout.write(terminalOutput);
}

const cp = pseudoTerminal.runCommand(commandConfig.command, {
cwd,
jsEnv: env,
quiet: !streamOutput,
});

let terminalOutput = '';
return new Promise((res) => {
cp.onOutput((output) => {
terminalOutput += output;
Expand Down Expand Up @@ -341,7 +350,10 @@ function nodeProcess(
readyWhen: string,
streamOutput = true
): Promise<{ success: boolean; terminalOutput: string }> {
let terminalOutput = '';
let terminalOutput = chalk.dim('> ') + commandConfig.command + '\r\n\r\n';
if (streamOutput) {
process.stdout.write(terminalOutput);
}
return new Promise((res) => {
const childProcess = exec(commandConfig.command, {
maxBuffer: LARGE_BUFFER,
Expand Down Expand Up @@ -450,7 +462,11 @@ export function interpolateArgsIntoCommand(
command: string,
opts: Pick<
NormalizedRunCommandsOptions,
'args' | 'parsedArgs' | '__unparsed__' | 'unknownOptions'
| 'args'
| 'parsedArgs'
| '__unparsed__'
| 'unknownOptions'
| 'unparsedCommandArgs'
>,
forwardAllArgs: boolean
): string {
Expand All @@ -477,7 +493,13 @@ export function interpolateArgsIntoCommand(
args += ` ${opts.args}`;
}
if (opts.__unparsed__?.length > 0) {
args += ` ${opts.__unparsed__.join(' ')}`;
const filterdParsedOptions = filterPropKeysFromUnParsedOptions(
opts.__unparsed__,
opts.unparsedCommandArgs
);
if (filterdParsedOptions.length > 0) {
args += ` ${filterdParsedOptions.join(' ')}`;
}
}
return `${command}${args}`;
} else {
Expand All @@ -497,3 +519,45 @@ function parseArgs(
configuration: { 'camel-case-expansion': false },
});
}

/**
* This function filters out the prop keys from the unparsed options
* @param __unparsed__ e.g. ['--prop1', 'value1', '--prop2=value2', '--args=test']
* @param unparsedCommandArgs e.g. { prop1: 'value1', prop2: 'value2', args: 'test'}
* @returns filtered options that are not part of the propKeys array e.g. ['--prop1', 'value1', '--prop2=value2']
*/
function filterPropKeysFromUnParsedOptions(
__unparsed__: string[],
unparsedCommandArgs: {
[k: string]: string;
} = {}
): string[] {
const parsedOptions = [];
for (let index = 0; index < __unparsed__.length; index++) {
const element = __unparsed__[index];
if (element.startsWith('--')) {
const key = element.replace('--', '');
if (element.includes('=')) {
if (!propKeys.includes(key.split('=')[0].split('.')[0])) {
// check if the key is part of the propKeys array
parsedOptions.push(element);
}
} else {
// check if the next element is a value for the key
if (propKeys.includes(key)) {
if (
index + 1 < __unparsed__.length &&
__unparsed__[index + 1] === unparsedCommandArgs[key]
) {
index++; // skip the next element
}
} else {
parsedOptions.push(element);
}
}
} else {
parsedOptions.push(element);
}
}
return parsedOptions;
}

0 comments on commit 56160da

Please sign in to comment.