yarnpkg / berry

📦🐈 Active development trunk for Yarn ⚒
https://yarnpkg.com
BSD 2-Clause "Simplified" License
7.37k stars 1.1k forks source link

[Feature] Add option to build dependencies of workspace when using the --since option #4391

Open klemenoslaj opened 2 years ago

klemenoslaj commented 2 years ago

Describe the user story

Using the foreach with the topological flag causes certain command for all workspaces to be executed in a proper dependency order.

Together with the --since flag, the command will be executed starting with the changed workspace only. This is often enough, but not always - sometimes we'd need to execute the command for all the dependencies of the changed workspace.

Use case: build command.

Executing: yarn workspaces foreach -tvA --since=main run build

For the following workspaces structure:

lib-a
lib-b (changed since `main`)
 └ lib-a
lib-c
 ├ lib-a
 └ lib-b
lib-d
 └ lib-a

Will effectively execute:

And will omit:

Since lib-a was not built, the likelihood of lib-b or lib-c failing is quite large.

Describe the solution you'd like

Additional flag/option to force building all dependencies and dependents of the changed workspace.

From the example above, what we'd need is the option to build lib-a before lib-b, resulting in:

While omitting:

seansfkelley commented 2 years ago

The --recursive flag might help you, though it appears it only follows dependents (not dependencies) when used with --since:

https://github.com/yarnpkg/berry/blob/20612e82d26ead5928cc27bf482bb8d62dde87d3/packages/plugin-workspace-tools/sources/commands/foreach.ts#L149-L155

klemenoslaj commented 2 years ago

Yeah and that is exactly the problem I am trying to bring forward.

Oftentimes (like in the CI) you would wanna build dependencies before your changed workspace. If we try to execute build task for the changed workspace, how is that even suppose to work? If dependencies are not built they cannot be resolved.

On the other hand, the current behavior is perfect for some other tasks, like lint.

So maybe additional flag?

seansfkelley commented 2 years ago

Yeah, the flags are pretty confusing. https://github.com/yarnpkg/berry/issues/3591 highlights this same issue and suggests that it's been resolved, but I'm not sure where.

thdk commented 2 years ago

@seansfkelley didn't find anything that would resolve this issue either. Maybe #3591 should be edited to mark this as not done yet?

However, I don't see the need for a new flag. The current flags --since, -R and -t should be sufficient. Adding more flags might make the logic of what to do when flags are combined too hard to reason for many.

It took me a while to find out why yarn workspaces foreach -tR --since run build did not build the dependency (lib-a in example above)

What is the reason that -t --since won't also use the packages on which the changed packages depend?

seansfkelley commented 2 years ago

No idea. That behavior was in place before I opened any PRs against this plugin, though I think it could be simpler (and v4 a good time to make such a change) by replacing -R with --dependents and --dependencies, and leaving the rest of the flags alone.

I think the conceptual complexity stems from the fact that a workspace is related to (1) other workspaces mentioned in its package.json and (2) nested workspaces. -R tries to do both at once and falls flat. I personally have no use for nested workspaces (beyond the root-level one that contains all the rest), so I can't speak for how people might want to use these flags in that environment.

thdk commented 2 years ago

How I though yarn workspaces foreach works: (disclaimer, this cannot be seen as documentation)

use -R when you want command to be run on selected workspaces and all workspaces that are dependent on those. use -t when you want command to be run first on all the dependencies of selected workspaces (adding more workspaces to the selection when needed), in correct order use --since=[ref] to select workspaces that have been changed since [ref] or since changesetBaseRefs field in .yarnrc config file.

However, it seems there is

So by writing this, it does start to make sense to use --dependencies and --dependants instead of --recursive :)

thezzisu commented 8 months ago

Is there any updates on this? The newest source code at https://github.com/yarnpkg/berry/blob/e135fc407d3389924e2237dfa4006a41800380d7/packages/plugin-workspace-tools/sources/commands/foreach.ts#L203 is still missing abilitiy to select dependency workspaces when using both --since and -R.