[RFC] Enable topological DSL operator to be used alone. #7269
Replies: 19 comments
-
|
@connorjs You could call it "get-your-flu-shot" and it'll still work; |
Beta Was this translation helpful? Give feedback.
-
|
@nathanhammond - In #2855 (comment) you mentioned
I did not understand this Any comments on the word choice here? Does |
Beta Was this translation helpful? Give feedback.
-
I'm trying to figure out a similar thing: Should we use |
Beta Was this translation helpful? Give feedback.
-
|
I believe #2911 is still relevant. Any feedback would be appreciated. |
Beta Was this translation helpful? Give feedback.
-
|
@rubber-duck-software @DerYeger Docs for this behavior have landed! The opinion I've developed working through this issue and doccing is that both options ( @nathanhammond @tknickman Will leave it to y'all's judgment as far as if we think a DSL change is in order or if we can/should close this. 👍 |
Beta Was this translation helpful? Give feedback.
-
|
Ah, my bad. Because of |
Beta Was this translation helpful? Give feedback.
-
|
Hi @DerYeger! Thanks for continuing to engage on this. Your example of "tsc": {
"dependsOn": ["^tsc"]
}is different than the I have added a some sleep time into https://github.com/rubber-duck-software/turborepo-tsc-caching/tree/working-example so you can verify this behavior for yourself. My opinion is that this is a lot of subtlety for a fairly vanilla example. I am happy to help with documentation on this. :) |
Beta Was this translation helpful? Give feedback.
-
Yes, across all projects, but not dependency and dependent. it will still run in parallel for unrelated projects (i.e., no dependency on each other). |
Beta Was this translation helpful? Give feedback.
-
|
It is possible to run tsc in parallel and still get accurate type check failures. Per @nathanhammond's suggestions above, the correct turbo.json file for this would have been {
"$schema": "https://turborepo.org/schema.json",
"pipeline": {
"topo": {
"dependsOn": ["^topo"]
},
"tsc": {
"dependsOn": ["^topo"]
}
}
}You can see this example working at https://github.com/rubber-duck-software/turborepo-tsc-caching/tree/working-example This configuration will both run tsc in parallel across all projects and will ensure correct caching behavior. However, the configuration is subtle enough that I think most users will miss it and accidentally configure only partial parallelization or (worse) incorrect caching. |
Beta Was this translation helpful? Give feedback.
-
From your example:
Isn't this issue expected? Your pipeline is deliberately running That's also how I organized my monorepo: "typecheck": {
"dependsOn": ["^build"],
"inputs": [
"index.html",
"tsconfig.json",
"vite.config.ts",
"*.d.ts",
"public/**",
"src/**",
"test/**"
],
"outputs": []
}with |
Beta Was this translation helpful? Give feedback.
-
|
As for the number of people who head down this path, I am going to guess this is common because of typescript. The monorepo handbook discusses using typescript as a linter. I'd say any monorepo which uses typescript as a linter and wants to run typescript in parallel across their packages will run into this. See example repo https://github.com/rubber-duck-software/turborepo-tsc-caching Regarding the "^", I think it's a bit too subtle for my comfort, but I'm only one user. It could very well be introducing "^" is the best answer. This would clean up the most common examples of this issue at any rate and the |
Beta Was this translation helpful? Give feedback.
-
|
I am still open to being convinced that "just It's not new syntax, it's not a weird overload, and being able to explain everything as a cross product of options is also useful pedagogically. But I do want to give it time and see precisely how many people head down this path, and see if other features we end up building start to work with it in a graceful way. We pay a maintenance tax on every public API we support, so I try to be conservative about adding it. |
Beta Was this translation helpful? Give feedback.
-
|
Good point- I had not considered the performance impacts of my proposal. So, what I think I'm hearing is
If that is true, it would seem the "workaround" is no longer a workaround but instead the long term way of solving this. Thanks for the PR around documenting this @anthonyshew. I am also going to create a PR for adding documentation to the Monorepo Handbook under linting. I think running |
Beta Was this translation helpful? Give feedback.
-
|
Your alternative proposal is not quite equivalent. You're still not taking advantage of the dependency graph. You're instead specifying a list of inputs which can be individually considered as part of a cache key for a task. In other words, it's still this: The trick in the From a Turborepo perspective the inputs pattern you specify has one additional major drawback: each one of those input globs must be individually calculated. Putting it into a tree means it's calculated just once. We're definitely not going to add a way to express dependencies that are in a separate package like this, nor provide a way to make that less painful by using filter syntax. We want to encourage people into thinking in graphs. |
Beta Was this translation helpful? Give feedback.
-
|
Ahh- that example is very helpful. A tool like eslint will not be affected by changes in a dependency, but a tool like tsc will. As for the
If I understand the current configuration correctly from the docs dependsOn: The list of tasks this task depends on. If I understand the An equivalent way of solving this (though far more fragile) would be "test": {
"dependsOn": []
},
"multiply#test": {
"inputs": ["../add/*"]
},
"subtract#test": {
"inputs": ["../add/*"]
}Now understanding that, I guess I would phrase the request as Add the ability to express file dependencies (like the |
Beta Was this translation helpful? Give feedback.
-
|
Conceptually The reason for the distinction here is that some tasks (think: apps that have bundlers that will do the right thing for dependencies throughout the application, or lint tasks that are simply static analysis of all code unaware of the project architecture) don't need to be aware of any topological dependencies. We can already express those in our DSL gracefully. Your example is the inverse: topological dependencies, but no task dependencies. We can make our DSL do this, but it requires the |
Beta Was this translation helpful? Give feedback.
-
|
The workaround suggested looks like it works great! Thank you both for your responses. This addresses my immediate concern wonderfully :) As for a longer term solution, I think the
I love However, this get confusing in the example above, because my tasks don't depend on any other tasks. Ideally, I could express this as If "^" is added as additional syntax, the definition of Talking to other users, what are the use cases where changes in dependency code should not invalidate the cached task result? Would including the hash of the dependency's code be a more intuitive default? Honestly, as I read the docs and saw that |
Beta Was this translation helpful? Give feedback.
-
|
It's really clear what happens here if you take a look at how the graph gets built when it doesn't have a The issue here is that we don't have a way in our DSL to tell you to depend on your topological dependencies outside of a task. Or, in other words, Fortunately, that's very straightforward to express as a synthetic task in {
"$schema": "https://turbo.build/schema.json",
"pipeline": {
+ "topo": {
+ "dependsOn": ["^topo"]
+ },
"test": {
+ "dependsOn": ["topo"]
}
}
}Once you do that you get the behavior you want and the graph looks like this: It's plausible that we should add "just ^" as an additional syntax rule in the DSL, but it's low priority for us to do that given that there is a clear method to accomplish this task (and the DSL would, under the hood, desugar to exactly this behavior). |
Beta Was this translation helpful? Give feedback.
-
|
I faced a similar issue with my monorepo. As a workaround, I defined dummy build tasks for both packages: "@yeger/eslint-config#build": {
"inputs": ["src/**"],
"outputs": ["src/**"]
},
"@yeger/tsconfig#build": {
"inputs": ["base.json", "lib.json", "web.json"],
"outputs": ["base.json", "lib.json", "web.json"]
}This approach does not require a dummy If you now add "add#build": {
"inputs": ["src/**"],
"outputs": ["src/**"],
},
"test": {
"dependsOn": ["^build"]
}to your pipeline, the |
Beta Was this translation helpful? Give feedback.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
-
Describe the Feature Request
Turborepo should differentiate between task dependencies and code dependencies.
The
dependsOnoption isThe list of tasks this task depends on.It seems natural that this option would be used to define task dependencies. However, this option is also used to define code level dependencies in a way which is both confusing and prevents even some simple use cases.For example, lets say my monorepo looks like
where
multiplyandsubtractboth depend onadd.As a responsible developer, I add tests to all three of these workspaces.
addis not compiled. In fact, none of these workspaces require a build step before testing.Now, I want all these tests to run in parallel. So, for each of these tasks I set
dependsOn: []. After all,multiplytests do not depend onaddtests. So these things are safe to run in parallel.Secondly, I want a change in
addto forcemultiplytests to rerun. After all, a change inaddmight break themultiply.I don't think this is possible with the current configuration options! With
dependsOn: [], code changes inadddon't change the computed hash formultiply. WithdependsOn: ["^test"], my tests will no longer execute in parallel.To summarize, my requirements are
Expected Behavior
I expect two workspaces to be able to depend on each other from a code perspective (a change in one workspace affects the behavior of the other) without having the workspace tasks depend on each other (workspace A must finish this task before workspace B begins)
To Reproduce
Follow the steps in the readme for https://github.com/rubber-duck-software/turborepo-caching-demo
Reproduction Repo
https://github.com/rubber-duck-software/turborepo-caching-demo
WEB-922
TURBO-1140
Beta Was this translation helpful? Give feedback.
All reactions