这是indexloc提供的服务,不要输入任何密码
Skip to content

feat(turborepo): pass through arguments only for root tasks #8089

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Conversation

HelloWorld017
Copy link

@HelloWorld017 HelloWorld017 commented May 4, 2024

Description

In the document, it says that cli arguments are not passed through task dependencies.

Note that these additional arguments will not be passed to any additional tasks that are run due to dependencies from the pipeline configuration.

But when I actually run a task with additional arguments (e.g. turbo run dev -- --env=stage) the arguments (e.g. --env=stage) are passed through all the dependencies, and their caches are missed.

Related Issues: #1744 #5743

Fixes

Only pass the additional cli arguments to the root tasks. (= tasks which are not required by other tasks)

Testing Instructions

  1. Open the design-system example.
  2. In docs, run turbo run build --summarize -- --disable-telemetry.
  3. Ensure that --disable-telemetry is only passed in @repo/docs#build, not @acme/ui#build
  4. In docs, run turbo run build --summarize -- --test
  5. Ensure that @acme/ui#build is cached.

@HelloWorld017 HelloWorld017 requested a review from a team as a code owner May 4, 2024 16:14
@turbo-orchestrator turbo-orchestrator bot added needs: triage New issues get this label. Remove it after triage owned-by: turborepo labels May 4, 2024
Copy link

vercel bot commented May 4, 2024

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

7 Ignored Deployments
Name Status Preview Comments Updated (UTC)
examples-basic-web ⬜️ Ignored (Inspect) Visit Preview May 5, 2024 7:52am
examples-designsystem-docs ⬜️ Ignored (Inspect) Visit Preview May 5, 2024 7:52am
examples-kitchensink-blog ⬜️ Ignored (Inspect) Visit Preview May 5, 2024 7:52am
examples-native-web ⬜️ Ignored (Inspect) Visit Preview May 5, 2024 7:52am
examples-svelte-web ⬜️ Ignored (Inspect) Visit Preview May 5, 2024 7:52am
examples-tailwind-web ⬜️ Ignored (Inspect) Visit Preview May 5, 2024 7:52am
examples-vite-web ⬜️ Ignored (Inspect) Visit Preview May 5, 2024 7:52am

Copy link

vercel bot commented May 4, 2024

@HelloWorld017 is attempting to deploy a commit to the Vercel Team on Vercel.

A member of the Team first needs to authorize it.

@HelloWorld017 HelloWorld017 force-pushed the fix/wrong-passthrough-args-cache-key branch from 11e8bb9 to 05409ce Compare May 5, 2024 07:52
@marisuxma
Copy link

This would be great!

@marisuxma
Copy link

@jaredpalmer

@mehulkar mehulkar requested review from chris-olszewski and removed request for gsoltis May 30, 2024 05:37
@chris-olszewski chris-olszewski self-assigned this May 30, 2024
@anthonyshew
Copy link
Contributor

Hey, sorry about this but that behavior must have been misdocumented. The behavior you're seeing is the one we intend to have and the docs reflect that today.

It's possible that I could bring up this behavior with the team and see if it could be a configuration or flag. Does that interest the both of you?

(Sidebar: @marisuxma, Jared doesn't work on this project anymore.) 😄

@anthonyshew anthonyshew removed the needs: triage New issues get this label. Remove it after triage label Jun 15, 2024
@turbo-orchestrator turbo-orchestrator bot added the needs: triage New issues get this label. Remove it after triage label Jun 15, 2024
@anthonyshew anthonyshew added the needs: team input Filter for core team meetings label Jun 16, 2024
@HelloWorld017
Copy link
Author

HelloWorld017 commented Jun 16, 2024

the docs reflect that today.

Actually the document does not describe the exact behaviour. Here's the summarized current behaviour.

  1. The arguments are passed to all tasks.
  2. The arguments are passed to all tasks, with same command name1
  3. The arguments are passed to only root tasks
1 2 3 Reference
Documentation
Cache Key task_hash.rs#L390
--summarize json summary/task_factory.rs#L162
Actual Running Task task_graph/visitor.rs#L637 opts.rs#L156
This PR

It's possible that I could bring up this behavior with the team and see if it could be a configuration or flag

Currently the Actual task running is different with cache key, which can create falsy cache, so I think they should be fixed.
And if all flags are passed through, ^build dependencies might cause turbo run test --coverage passing --coverage to build and turbo run dev --host passing --host to build.

Conclusion

So, if the 3 behavior (which is suggested by this PR) is not the preferred solution, I suggest making doucment / cache key / --summarize to reflect the current behavior (which is 2). And that can be represented like:

1 2 3 Reference
Documentation
Cache Key
--summarize json
Actual Running Task

If you want, I can open another PR, to do that.

Footnotes

  1. e.g. for test --coverage with ^build and ^test deps, the --coverage flag is only passed through the ^test deps.

@HelloWorld017
Copy link
Author

@anthonyshew (as a gentle reminder) would you check the comment I wrote?

@hchangjae
Copy link

@HelloWorld017
If you want to prevent unexpected additional parameters, This may help you.

#1744 (comment)

@chris-olszewski
Copy link
Member

Closing in favor of #10167. This behavior change would require a major version bump.

chris-olszewski added a commit that referenced this pull request Mar 14, 2025
### Description

Updating documentation around pass through args to address #8089.

This has been our behavior since 1.0 and I believe changing it would
require a major semver bump.

We pass any pass through args to tasks that match the name of one of the
tasks provided to the `turbo run` command. I believe this makes sense as
your `test` tool probably accepts different arguments than your `build`
tool so if `turbo test -- --coverage=true` passed `--coverage=true` to
not just `jest`, but also `esbuild` which might error when given that
argument. If you did want it to be passed to build you can achieve this
by running `turbo build test -- --global-flag`.

### Testing Instructions

Added a unit test for this behavior. 

Also spot checked that this has been the behavior since 1.2.0 (earliest
version I can get working without too much work). Tested by running
`pnpm turbo run test --filter=@repo/ui -- passthru`

1.2.0

```
• Packages in scope: @repo/ui
• Running test in 1 packages
@repo/ui:build: cache hit, replaying output 9509f6022cfb8b76
@repo/ui:build: 
@repo/ui:build: > @repo/ui@0.0.0 build /private/tmp/pass-thru/packages/ui
@repo/ui:build: > echo building: 
@repo/ui:build: 
@repo/ui:build: building:
@repo/ui:test: cache hit, replaying output 9b006b1111a435ab
@repo/ui:test: 
@repo/ui:test: > @repo/ui@0.0.0 test /private/tmp/pass-thru/packages/ui
@repo/ui:test: > echo testing:  "passthru"
@repo/ui:test: 
@repo/ui:test: testing: passthru

 Tasks:    2 successful, 2 total
Cached:    2 cached, 2 total
  Time:    40ms >>> FULL TURBO
```

1.6.0
```
• Packages in scope: @repo/ui
• Running test in 1 packages
• Remote caching disabled
@repo/ui:build: cache hit, replaying output 462fe8a87d3ab7c0
@repo/ui:build: 
@repo/ui:build: > @repo/ui@0.0.0 build /private/tmp/pass-thru/packages/ui
@repo/ui:build: > echo building: 
@repo/ui:build: 
@repo/ui:build: building:
@repo/ui:test: cache hit, replaying output 6fb610019c2f32a2
@repo/ui:test: 
@repo/ui:test: > @repo/ui@0.0.0 test /private/tmp/pass-thru/packages/ui
@repo/ui:test: > echo testing:  "passthru"
@repo/ui:test: 
@repo/ui:test: testing: passthru

 Tasks:    2 successful, 2 total
Cached:    2 cached, 2 total
  Time:    40ms >>> FULL TURBO
```

2.4.4
```
• Packages in scope: @repo/ui
• Running test in 1 packages
• Remote caching disabled
@repo/ui:build: cache hit, replaying logs 074103acac2242fe
@repo/ui:build: 
@repo/ui:build: �
@repo/ui:build: > @repo/ui@0.0.0 build /private/tmp/pass-thru/packages/ui
@repo/ui:build: > echo building: 
@repo/ui:build: 
@repo/ui:build: building:
@repo/ui:test: cache hit, replaying logs 0299610cfc01eeff
@repo/ui:test: 
@repo/ui:test: �
@repo/ui:test: > @repo/ui@0.0.0 test /private/tmp/pass-thru/packages/ui
@repo/ui:test: > echo testing:  "passthru"
@repo/ui:test: 
@repo/ui:test: testing: passthru

 Tasks:    2 successful, 2 total
Cached:    2 cached, 2 total
  Time:    42ms >>> FULL TURBO


```
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
needs: team input Filter for core team meetings needs: triage New issues get this label. Remove it after triage
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants