rushstack icon indicating copy to clipboard operation
rushstack copied to clipboard

[rush] Feature Request: Add support for true circular dependencies in rush

Open akaRem opened this issue 4 years ago • 4 comments

Summary

I didn't find any recommended approach to manage autogenerated code. Rush is focused on fast incremental builds and it plans/orchestrates them based on project dependencies spec.

If all the code is hand-written, it's not a problem and the dependency tree is easy to define. However, if we have autogenerated packages, they may depend on other packages where they are used as dependencies. The best example of such a case is autogenerated client libraries.

Details

Let's say, we have the following components:

  • Server
  • API Client
  • WebApp

The Server is written with the code-first approach so that API schemas (swagger/graphql) used to generate the API Client codebase may only be extracted from server files. At the same time, the Server uses this autogenerated API Client to run server-tests. The API Client is also used as the WebApp app dependency. (In real-world projects the setup may be even more complex)

So we have

  • Server depends on API Client
  • API Client depends on Server
  • WebApp depends on Server and API Client

and circular dependencies Server <-> API Client

What is your recommendation? Could you please add a section in documentation regarding codegen?

akaRem avatar Feb 04 '21 14:02 akaRem

Hmm.. Good point. I see on our website that we mention "Cyclic dependencies", but don't really say much else about them, other than the comment in the rush.json config reference page.

Rush does support what we are calling "cyclic dependencies" in the rush.json configuration:

  /**
    * A list of local projects that appear as devDependencies for this project, but cannot be
    * locally linked because it would create a cyclic dependency; instead, the last published
    * version will be installed in the Common folder.
    */
   "cyclicDependencyProjects": [
     // "my-toolchain"
   ],

Perhaps a tutorial or example on how this is used would be helpful.

halfnibble avatar Feb 04 '21 19:02 halfnibble

I changed the title of your issue to better reflect my understanding of it. However, if I am wrong, please feel free to change it back and add further clarification (and maybe a repro).

halfnibble avatar Feb 04 '21 19:02 halfnibble

@halfnibble

The issue I filed was different than improving documentation for cyclic dependencies.

I also disagree with the effort: easy label. I'll explain.

1. The current implementation for cyclic dependencies won't work.

the last published version will be installed in the Common folder.

A published package is usually built from the master or release branch and does not contain any changes made in a feature branch.

In the context of API-server testing, the server code will contain the current feature, and the client code will contain code generated for the master where this feature is not present => the new feature can not be tested using this client package installed/linked by the build tool.

The problem is even more severe if you have LTS branches. The testing on such branches (w/o extra effort) will be done with a client from the latest release which

  • will make tests flaky
  • will make it almost impossible to fix a bug that is already fixed in master as it won't be reproducible.

Question: Will this approach link different packages in the web app (where you don't have cyclic dep) and in the server (where you have)? If yes, it's is a critical bug in the tool.

2. The solution I used as an example is not the only way to organize a repo

I could imagine other ways to support the "codegen", but it's hard to say what is supported and how you and your team recommend doing it (that's why I proposed to document it).

Examples:

A. API client could be generated from prebuild or pretest of the server (or as a watcher on server code) Will Rush detect that changes in the server also affect 2 other packages? How to setup it?

B. It's possible to write a simple bash script/Makefile to generate all the autogenerated code before the actual running of rush tool. It's not convenient. Does Rush support anything like that out-of-the-box?

C. Some other tools have so-called "implicit dependencies"; you basically specify that "this package" must be rebuild/retested if "that file/folder" changes. Does Rush have anything like that?

D. It's possible to enforce devs to commit autogenerated code. AFAIK, it's antipattern; it will make the review process harder; it will cause merge conflicts.

etc.

3. It's the only problem of the "codegen" usage

If you write the client by hands, you will commit the changes, and they worth reviewing. The API client will not require the server package as its dependency and dependency graph becomes pure DAG.


PS: I don't have a repro, because I don't understand how to set up a monorepo with a "codegen" tool using Rush.

akaRem avatar Feb 05 '21 16:02 akaRem

this should be common case if we use mono repo and add some test utility packages.

cqcmdwym avatar Jul 13 '22 03:07 cqcmdwym