[rush] Add --from to rush install
#2435
Conversation
wbern
requested review from
apostolisms,
D4N14L,
halfnibble,
iclanton,
octogonz and
sachinjoseph
as code owners
apps/rush-lib/src/logic/installManager/WorkspaceInstallManager.ts
Outdated
Show resolved
Hide resolved
| // "<package>..." selects the specified package and all direct and indirect dependencies | ||
| .map((p) => p.packageName + '...') | ||
| // ..."<package>" selects the specified package and all direct and indirect dependents of that package | ||
| .concat(this.options.fromProjects.map((p) => '...' + p.packageName)); |
There was a problem hiding this comment.
So, the original PR that brought this functionality in actually had this. I was talked out of it by @octogonz (though I think it may have been a Gitter discussion since I can't find it now...). Main reason it was removed was that I couldn't think of a great justification for how this would be used. So he's likely going to need some convincing :)
There was a problem hiding this comment.
Our justification is that in order to use the rush build --from flag, we need to also install the deps of those dependents coming up from the --from flag. If package A relies on B, we can't build A when B changes, if we don't install the dependencies for A via the --from flag.
The workaround for now is to do rush install without any filtering, but that is largely inefficient for our large monorepos.
There was a problem hiding this comment.
Regardless of whether or not upstream projects need to be built, there are plenty of instances where merely referencing the upstream project results in references to the node_modules of the upstream project, so it would not surprise me if, unlike in the rush build case (where the developer may be running it after an earlier rush build), rush install --from is completely unusable if it skips the dependencies of upstream local projects.
|
The code looks fine. There is a slightly interesting nuance in the design. From the discussion about PR #2422, we said that:
Whereas if I understand correctly:
Would this inconsistency be confusing to anyone? (And if so, would we rename |
|
@octogonz Honestly I think --from should include everything that's needed. As a colleague of mine said, to and from makes an hourglass, or at least so we thought at first. 🙂 |
|
In #2422 (comment) we're proposing to use |
|
@wbern This was released with Rush 5.37.0 |
My first real PR to rush, finally. ❤️
Summary
I added support for
--from-based installs usingrush install, currently there is only support for--to, and the change was trivial to make, following pnpm workspaces documentation here: https://pnpm.js.org/en/filtering#filter-lt-package_name-Fixes #2434
Details
In my org, we want to install/build/test only the changed projects in our CI, which we are accomplishing with the
--toflag. However, we see that if we change a widely used package, we want to build and test all the dependents of that package when it gets changed, and publish those packages as well. Hence we have a need for the--fromflag forrush install, a flag already present in the other rush commands we use around building things.How it was tested
I created an alias called
testrushlocally on my machine and used in our biggest repo, I then didtestrush install --to @org/my-libI inspected the output being
Scope: 4 out of 124 workspace projects,then I ran
testrush install --to @org/my-lib --from @org/my-libWhich gave me the output of
Scope: 115 of 124 workspace projects.I also tested
testrush install --from @org/my-libwhich returnedScope: 112 of 124 workspace projects, so it adds up.The output from the filtered install in this case is
pnpm install --filter ...@org/my-lib --filter @org/my-lib....Final notes
I know we're changing names of --from/--to to other things later, so open for suggestions how to merge this.