nx icon indicating copy to clipboard operation
nx copied to clipboard

feat(core): allow optional configuration in infix run command

Open akwodkiewicz opened this issue 1 year ago • 5 comments

It looks as if the current shape of args.target that is parsed by the run infix version and passed to the command handler makes the configuration unrecognizable. It's probably because the "regular" run invocation parses target and configuration separately, and passes them as 2 distinct properties, while the infix version of the command contains the concatenated target:configuration in the target argument.

As explained in the linked issue, I couldn't leverage yargs positional arguments syntax in the command template, because the configuration is an optional argument in the middle of the invocation. Custom parsing in the handler must be used. Thankfully it's just about splitting the argument on the first :.

I do not know if it won't break any existing behaviours, for example the ability to run NPM scripts that contain colons in their names.

Current Behavior

Infix notation does not allow passing target configuration.

Expected Behavior

Infix notation allows passing target configuration.

Related Issue(s)

Duplicate of #28348 that got closed by mistake. Fixes #28335.

akwodkiewicz avatar Oct 09 '24 07:10 akwodkiewicz

The latest updates on your projects. Learn more about Vercel for Git ↗︎

1 Skipped Deployment
Name Status Preview Updated (UTC)
nx-dev ⬜️ Ignored (Inspect) Visit Preview Oct 9, 2024 7:55am

vercel[bot] avatar Oct 09 '24 07:10 vercel[bot]

☁️ Nx Cloud Report

CI is running for commit e419528849a02c3cc876baeb927812687a9d936a.

📂 Click to track the progress, see the status, the terminal output, and the build insights.


Sent with 💌 from NxCloud.

nx-cloud[bot] avatar Oct 09 '24 07:10 nx-cloud[bot]

Oh, right, this one does not pass

 FAIL   e2e-nx  src/run.test.ts (647.795 s)
  Nx Running Tests
    running targets
      ✓ should execute long running tasks (62317 ms)
      ✕ should run npm scripts (9439 ms)
Original command: pnpm exec nx echo:dev mylib4312143 -- positional --a=123 --no-b 

NX   Cannot find configuration for task mylib4312143:echo

So with my change nx echo:dev mylib is not able to run echo:dev NPM script.

I'm afraid if we wanted to solve #28335, we'd need to either:

  • A) rewrite how we're handling NPM scripts (append value from configuration to target, before looking at the manifest)

or

  • B) rewrite how we're handling run itself

Both of which increase the complexity of the solution.

A) would mean that nx run lib:target --configuration=foobar is going to run target:foobar NPM script. It might be an issue when somebody actually tries to pass a --configuration flag to the target NPM script.

B) would mean we should come up with a precedence of targets/scripts. For example, given nx foo:bar lib, we could:

  1. First search for foo targets with bar configuration. If there is a match for both, run it.
  2. If there is a foo task defined, but no bar configuration for this task, don't run it.
  3. Look for foo:bar NPM script.

Or in point 2. we could always try to run foo with ignored --configuration bar, as this is probably more in line with regular run invocation.

akwodkiewicz avatar Oct 09 '24 09:10 akwodkiewicz

yes, you are right, it broke the npm script. maybe we should use b option, before splitting the target by ':', but should check whether the target is already valid. this should not be that complex. let me look into this option.

xiongemi avatar Oct 10 '24 23:10 xiongemi

i submit another pr to fix this: https://github.com/nrwl/nx/pull/28402

xiongemi avatar Oct 15 '24 20:10 xiongemi

As discussed, we won't be extending the format of running targets when using nx <target>:<config> <project>. You can use nx <target> <project> --configuration=<config> instead.

jaysoo avatar Oct 23 '24 20:10 jaysoo

This pull request has already been merged/closed. If you experience issues related to these changes, please open a new issue referencing this pull request.

github-actions[bot] avatar Oct 29 '24 00:10 github-actions[bot]