vercel / turborepo

Build system optimized for JavaScript and TypeScript, written in Rust
https://turbo.build/repo/docs
MIT License
26.41k stars 1.84k forks source link

Using pass through args drops options when run w/ Yarn 1 #7864

Open ianduvall opened 7 months ago

ianduvall commented 7 months ago

Verify canary release

Link to code that reproduces this issue

https://github.com/ianduvall/turborepo-issue-7864

What package manager are you using / does the bug impact?

Yarn v1

What operating system are you using?

Mac

Which canary version will you have in your reproduction?

turbo@1.13.1-canary.3

Describe the Bug

Using pass through arguments seems to drop any specified options. For example:

% yarn turbo run format --filter=// -- -- --check
yarn run v1.22.15
warning From Yarn 1.0 onwards, scripts don't require "--" for options to be forwarded. In a future version, any explicit "--" will be forwarded as-is to the scripts.
$ /Users/.../my-turborepo/node_modules/.bin/turbo run format -- --check
• Packages in scope: //, app-a, app-b, pkg-a, pkg-b, tooling-config
• Running format in 6 packages
...

Using one set of -- as the docs suggest doesn't seem to work.

% yarn turbo run format --filter=// -- --check   
yarn run v1.22.15
warning From Yarn 1.0 onwards, scripts don't require "--" for options to be forwarded. In a future version, any explicit "--" will be forwarded as-is to the scripts.
$ /Users/.../my-turborepo/node_modules/.bin/turbo run format --check
 ERROR  unexpected argument '--check' found

  tip: a similar argument exists: '--check-for-update'
  tip: to pass '--check' as a value, use '-- --check'

Usage: turbo run <--cache-dir <CACHE_DIR>|--cache-workers <CACHE_WORKERS>|--concurrency <CONCURRENCY>|--continue|--dry-run [<DRY_RUN>]|--single-package|--force [<FORCE>]|--framework-inference [<BOOL>]|--global-deps <GLOBAL_DEPS>|--graph [<GRAPH>]|--env-mode [<ENV_MODE>]|--filter <FILTER>|--scope <SCOPE>|--ignore <IGNORE>|--since <SINCE>|--include-dependencies|--no-deps|--no-cache|--daemon|--no-daemon|--output-logs <OUTPUT_LOGS>|--log-order <LOG_ORDER>|--only|--parallel|--pkg-inference-root <PKG_INFERENCE_ROOT>|--profile <PROFILE>|--anon-profile <ANON_PROFILE>|--remote-only [<BOOL>]|--remote-cache-read-only [<BOOL>]|--summarize [<SUMMARIZE>]|--log-prefix <LOG_PREFIX>|TASKS|PASS_THROUGH_ARGS|--experimental-space-id <EXPERIMENTAL_SPACE_ID>>

For more information, try '--help'.

error Command failed with exit code 1.

Expected Behavior

I would expect using pass through arguments to work with turbo options. For the given example, I would expect yarn turbo run format --filter=// -- -- --check to run the format script only in the root package.json with the arguments --check appended.

To Reproduce

Run yarn turbo run format --filter=// -- -- --check in a repo with a script format in the root package.json. See https://github.com/ianduvall/turborepo-issue-7864.

Additional context

Related issue: https://github.com/vercel/turbo/issues/1731

anthonyshew commented 7 months ago

turbo format --filter=// -- --check is working correctly for me.

CleanShot 2024-03-29 at 06 04 51

So this must be a yarn problem or an "intersection of yarn and turbo" problem. 🤔 There's likely a magic combination of arguments that works but I can't quite sort out what it is.

A couple workarounds:

ianduvall commented 7 months ago

So this must be a yarn problem or an "intersection of yarn and turbo" problem. 🤔 There's likely a magic combination of arguments that works but I can't quite sort out what it is.

Definitely, I'm also stumped.

A couple workarounds:

  • Use global turbo

I'm trying to avoid using a global version of turbo since global dependencies isn't a blessed pattern on my team and will make upgrading versions in the future a pain.

  • Make two Prettier scripts in package.json, one with --check and one without. This would also mean two turbo.json tasks, which I'd argue this is a better practice, anyway (but that may be only my opinion). 😄

A better example would be running Jest with --shard in CI in which case the shard count is determined in CI so needs to get passed as an arg. My current workaround is putting the turbo run call with options in its own script in package.json ("turbo-root-test-unit": "turbo run root-test-unit --filter=//") but that prevents us from specifying dynamic turbo run options in CI. Another issue is changing an option would involve editing package.json which would cache bust every task which isn't ideal.

chris-olszewski commented 7 months ago

This is an issue with yarn arg parsing that happens before turbo gets invoked.

It can be displayed by replacing the turbo binary with a script that just echo's back the args passed to it (I took the liberty of removing some yarn output to make behavior more obvious):

[0 olszewski@chriss-mbp] /tmp/yarn1 $ cat node_modules/turbo-darwin-arm64/bin/turbo      
#!/bin/bash

echo $@
[0 olszewski@chriss-mbp] /tmp/yarn1 $ yarn turbo run format --filter=// -- -- --check                  
$ /private/tmp/yarn1/node_modules/.bin/turbo run format -- --check
run format -- --check
[0 olszewski@chriss-mbp] /tmp/yarn1 $ yarn turbo run format --filter=// -- --check   
$ /private/tmp/yarn1/node_modules/.bin/turbo run format --check
run format --check
[0 olszewski@chriss-mbp] /tmp/yarn1 $ yarn turbo -- run format --filter=// -- --check
$ /private/tmp/yarn1/node_modules/.bin/turbo run format --filter=// -- --check
run format --filter=// -- --check

If you want to invoke turbo through yarn, but not through a script, then yarn turbo -- is the correct way to do it as every argument after -- will get passed directly to turbo.

I'm trying to avoid using a global version of turbo since global dependencies isn't a blessed pattern on my team and will make upgrading versions in the future a pain.

Using this opportunity to clarify that global turbo will defer to whatever version of turbo is installed locally e.g.:

[0 olszewski@chriss-mbp] /tmp/yarn1 $ turbo --skip-infer bin
/Users/olszewski/.nvm/versions/node/v20.11.1/lib/node_modules/turbo/node_modules/turbo-darwin-arm64/bin/turbo
[0 olszewski@chriss-mbp] /tmp/yarn1 $ turbo --skip-infer --version
1.12.5
[0 olszewski@chriss-mbp] /tmp/yarn1 $ turbo bin                   
/private/tmp/yarn1/node_modules/turbo-darwin-arm64/bin/turbo
[0 olszewski@chriss-mbp] /tmp/yarn1 $ turbo --version
1.13.1-canary.3
[0 olszewski@chriss-mbp] /tmp/yarn1 $ yarn turbo -- --version
yarn run v1.22.19
warning From Yarn 1.0 onwards, scripts don't require "--" for options to be forwarded. In a future version, any explicit "--" will be forwarded as-is to the scripts.
$ /private/tmp/yarn1/node_modules/.bin/turbo --version
1.13.1-canary.3
✨  Done in 0.38s.

This should allow upgrading turbo to be as simple as bumping the turbo version in your package.json

psychobolt commented 7 months ago

Quick question: Why is this not working for me? I'm on Yarn v4

/package.json

{
  "scripts": {
    "format" "echo foo-$@",
  }
}

/turbo.json

{
  "pipeline": {
    "//#format": {},
    "format": {}
  }
}
yarn turbo run format --filter=// --force -- bar # prints 'foo-bar'
yarn turbo run //#format --force -- bar # prints 'foo-'

I expected both to have same result when running without cache enabled.