Skip to content
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

Allow circular references of project references #33685

Open
5 tasks done
RyanCavanaugh opened this issue Sep 30, 2019 · 14 comments · May be fixed by #54216
Open
5 tasks done

Allow circular references of project references #33685

RyanCavanaugh opened this issue Sep 30, 2019 · 14 comments · May be fixed by #54216
Labels
Awaiting More Feedback This means we'd like to hear from more people who would be helped by this feature Suggestion An idea for TypeScript

Comments

@RyanCavanaugh
Copy link
Member

RyanCavanaugh commented Sep 30, 2019

Search Terms

circular reference project references graph

Suggestion

Currently, project reference graphs are required to be acyclical because

  • Project references imply a .d.ts file is loaded, and .d.ts files can't exist before the build occurs
  • Only acyclic graphs can be topologically sorted

Both of these problems are solvable without too much work.

With the work done in #32028, we can effectively toggle the redirecting behavior, fixing the first issue. This would be done during the initial build phase only when an upstream project dependency isn't built yet.

The other problem is that project build order might not be predictable if we arbitrarily pick some starting point in the unsortable graph. This is fixable if we force solution authors to indicate which edges in the graph should be treated as ignored for the purposes of the topological sort:

    "references": [
        { "path": "../a" },
        { "path": "../b", "circular": true },
                          ^^^^^^^^^^^^^^^^
        { "path": "../c" }
    ],

The benefits of this are:

  • In case of suboptimal .d.ts generation or memory pressure, developers can control where the "weak" link occurs in the build process
  • The build ordering is fully deterministic regardless of starting point
  • Graphs can't become "accidentally" circular - this is a clear opt-in

Use Cases

npm and other package managers do allow circularities in dependency graphs, so this is apparently a thing.
We've also gotten feedback from some industry partners that they want to move to project references, but their dependency graph is circular in a way that would require a lot of work to "fix".

Examples

Given the graph:

A -> B -> C -(circular)-> A

The build order is deterministically C, B, A. During C's compilation, source file redirects from C to A are not active.

Checklist

My suggestion meets these guidelines:

  • This wouldn't be a breaking change in existing TypeScript/JavaScript code
  • This wouldn't change the runtime behavior of existing JavaScript code
  • This could be implemented without emitting different JS based on the types of the expressions
  • This isn't a runtime feature (e.g. library functionality, non-ECMAScript syntax with JavaScript output, etc.)
  • This feature would agree with the rest of TypeScript's Design Goals.
@RyanCavanaugh RyanCavanaugh added Suggestion An idea for TypeScript Awaiting More Feedback This means we'd like to hear from more people who would be helped by this feature labels Sep 30, 2019
@AnyhowStep
Copy link
Contributor

AnyhowStep commented Sep 30, 2019

In general, I like that project references are acyclic, honestly. It enforces a proper separation of concerns.

Like how F# doesn't like circular references,
https://fsharpforfunandprofit.com/posts/cyclic-dependencies/

However, being able to explicitly opt-in to circular project references safely would be fantastic!


At the moment, when one has multiple directories that circularly depend on each other, you have to make these directories part of the same subproject. Otherwise, you get a circular reference error.

In general, this is great. But...

If these directories are huge, then you lose the benefit of chunked incremental builds. Changing code in one directory causes all directories to be re-checked.

You lose all the benefits of project references just because your directories are circular. Shame =(


I have a use case that would benefit from circular project references, actually.
As usual, it has to do with SQL query building.

I have 1060 functions that build SQL expression objects in my data access subproject.
I split these 1060 functions into 100+ directories that circularly reference each other.

Build times are slow at the moment and changing one function requires TS to re-check all 1060 functions (18s to check, 58s to emit).

I can't make each directory a separate subproject at the moment because,

  • An expression in directory X might rely on expressions in directory Y.
  • An expression in directory Y might rely on expressions in directory X.

So, explicit circular project references for my data access layer would be super awesome.

TL;DR Complex code modeling relational databases would benefit greatly from explicit circular project references


[Edit]
I like how this was proposed as a workaround for people with existing circular dependencies but I'm excited to use this and introduce circular dependencies.

@ozyman42
Copy link

ozyman42 commented Jan 16, 2020

Anyone have ideas for how to model project references as if they were circular until this is implemented? Maybe have two typescript processes in parallel, where one process uses graph A -> B and other process reads B -> A

@dqsully
Copy link

dqsully commented Feb 4, 2020

@alexleung I'm not sure if this helps, but my team has been using pnpm workspaces to have our monorepo projects symlinked as node_module dependencies. I don't know if this keeps any of the benefits of TypeScript project references, but it allows us to keep our code separated into distinct modules. The TypeScript compiler works great with this project structure, and the TypeScript Language Server for VSCode works too, although it can be a little slow at times in our project. I'm not sure if it doesn't understand our symlinked structure and sometimes loads the same file multiple times, or if our project is just huge and abusive of TypeScript's powers... Overall it works great for us though, and we've got 30+ modules

@Bnaya
Copy link

Bnaya commented Feb 4, 2020

@alexleung In most cases it's possible to break circularities using additional package that will contain this things B needs from A, and both A & B will import from it.
Or by making them depending on interfaces and not impl

@ozyman42
Copy link

ozyman42 commented Feb 4, 2020

@dqsully Thanks for that mention of pnpm! I like the idea a lot and have actually been working on a similar project (only 1 installation in entire workspace) which is specific to typescript workspaces (creates project references automatically). Not sure that I can use pnpm unfortunately because it's not flattening out the node_modules folder, which means that the behavior of a package maintained in a pnpm workspace would not be consistent with the package after being published to npm. For example, graphql-js would probably break in pnpm if multiple modules in a pnpm workspace depend on it then depend on each other, since graphql-js has a check where it enforces that schemas exported from a module cannot be imported into different instance of the same module. Npm package flattening solves that problem, but pnpm would not.

@Bnaya that isn't an option because I am actually creating a tool similar to lerna, yarn, and pnpm which allows users to define project dependencies as they want with the same restrictions as npm. Npm allows for circular dependencies so I need to find a way to get circular project references auto-generated.

@ilearnio
Copy link

ilearnio commented Apr 7, 2021

This is still an issue even when using typescript@next (4.3.0-dev.20210406). My circular depenndencies are

  "references": [
      { "path": "./module-A" },
      { "path": "./module-B" },
  ]

So two npm packages within a monorepo that are using each other.

The problem is that "module-A" compiles first and it expects "module-B" to be already compiled, but it isn't.

The error looks like so:

error TS2307: Cannot find module 'module-B' or its corresponding type declarations.

import { foo } from 'module-B';
                    ~~~~~~~~~~~

@nicolo-ribaudo
Copy link

nicolo-ribaudo commented May 1, 2021

We are moving away from composite: true in Babel because we have some circular references that cause problems: babel/babel#13122 (comment).
If you'll ever want to use it as a mid-size project to test circular refs in the real word I can help!

@JanKaczmarkiewicz
Copy link

JanKaczmarkiewicz commented Jan 9, 2022

I am trying to adopt strict mode in one package in monorepo. Packages have multiple circular dependencies, so right now project reference is not an option. It would be helpful to have the possibility to gradually migrate some of the packages to eg. strict mode without moving a lot of code between packages. I think many monorepo based projects can have similar problems and benefit from this proposal.

@Zzzen
Copy link
Contributor

Zzzen commented May 7, 2023

@RyanCavanaugh Do you have any updates on the issue? We're in the process of migrating a project to use project reference, but we've hit a roadblock due to circular dependencies.

@Zzzen Zzzen linked a pull request May 11, 2023 that will close this issue
@gabemeola
Copy link

Friendly ping. This would be helpful for our monorepo.

@dgieselaar
Copy link

dgieselaar commented Mar 19, 2024

@RyanCavanaugh I work on elastic/kibana - one of the biggest public TS repos on Github. We've split up the repo in TS projects to improve IDE responsiveness (I'm suspicious whether this was achieved) and CLI type checks. Unfortunately, this creates a problem of circular references between these different projects. It's a ton of work for us to break everything down into smaller packages projects, and having project references support circular references would help. I'd be happy to contribute, although I'm not sure how helpful that would be :)

@david-fong
Copy link

It's a ton of work for us to break everything down into smaller packages

@dgieselaar nit: you don't need to break into packages (assuming you mean in the NodeJS sense of the word). you need to break into TS projects.

@dgieselaar
Copy link

@david-fong thanks, slip of the tongue, edited the message 🙂

@mvarchdev
Copy link

Hey, it would be also helpful for us!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Awaiting More Feedback This means we'd like to hear from more people who would be helped by this feature Suggestion An idea for TypeScript
Projects
None yet
Development

Successfully merging a pull request may close this issue.