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

[rush] DISCUSSION: project references for VSCode experience improvement #2604

Open
scamden opened this issue Apr 10, 2021 · 6 comments
Open
Labels
enhancement The issue is asking for a new feature or design change needs design The next step is for someone to propose the details of an approach for solving the problem

Comments

@scamden
Copy link
Contributor

scamden commented Apr 10, 2021

In this Zulip thread
we determined that TypeScript project references do make sense to use in
a Rush repo, for the purposes of improving the VS Code experience,
even if they are not used during the actual build.

Here's a sample repo for illustration: https://github.com/creditiq/simple-repros/tree/rush-project-references

This uses a script to sync the references automatically from pnmp workspace deps to tsconfig, but that's not strictly required (it's just really onerous to have to relist the deps otherwise). I believe there are tools that do the same for yarn and maybe npm workspaces as well.

@scamden scamden changed the title [rush] DISCUSSION: project references for VScode experience improvement [rush] DISCUSSION: project references for VSCode experience improvement Apr 10, 2021
@scamden
Copy link
Contributor Author

scamden commented Apr 10, 2021

The problem:

I open VSCode at the root of my monorepo as a workspace (or as some constellation of it's projects). I go to package B and add or change some exported thing. I go to package A which depends on B to use my newly exported thing, but VSCode says it doesn't exist. If I have a rush build:watch running, the output of B may have been compiled by this point or it may not, but vscode will often take a long time to recognize the new types even if it has been. I find myself frequently watching the build:watch till B runs and then running Restart TS Server in vscode to get the change. This takes a longish time and interrupts my flow of thought.

Proposed Solution:

If my tsconfig.json in A lists B in references, vscode will do its own internal compile and when I go to A in the scenario above my change will already be recognized as though the two packages were one code base. This is magical 💖.

Limitations of references:

  1. They are annoying to maintain, as it is essentially an exact duplication of your workspace dependencies (https://github.com/Bessonov/set-project-references script really helps here if you use pnpm workspaces)
  2. Any pre compile steps such as sass-typings will still need a rush / heft build running in the background and they will still take the extra time to show up in vscode (but this is a much smaller set of things to wait for)
  3. references should not be used to actually build the monorepo in rush as that bypasses caching and all the other rush goodness. thankfully typescript will not follow references unless you run compile with the --build flag (thank you typescript for staying backward compatible)
  4. These will not work across projects that have different typescript versions, but neither would having a single instance of vscode open in the first place (since it forces one global version of typescript for the whole workspace)
  5. if i use project references internally to a project we have problems. see below [rush] DISCUSSION: project references for VSCode experience improvement #2604 (comment)

@octogonz octogonz added enhancement The issue is asking for a new feature or design change needs design The next step is for someone to propose the details of an approach for solving the problem labels Apr 10, 2021
@iclanton
Copy link
Member

We could add support for this in Heft's TypeScript builder without having to touch the tsconfig.json files on disk. Obviously that would only work for repos that use both Rush and Heft. @dmichon-msft has been working on a multi-project watch feature for Rush, so this might naturally fit into that work.

Just spitballing, but how cumbersome would it be to have rush update also update the references in tsconfig.json files?

@scamden
Copy link
Contributor Author

scamden commented Apr 12, 2021

for reference: microsoft/TypeScript#25376

typescript is considering adding that same inference, but the trick is differing patterns between types of workspace implementations. so if rush or heft wants to do this automatically it will need to consider which installer is used and how they do workspaces. i love the idea of this happening automatically however, even if it was just supported for pnpm

@dmichon-msft
Copy link
Contributor

The tsconfig changes would be pretty simple, so shouldn't be too complex to add to rush update, though we'd want an experiments flag or rush json config option, obviously.

@scamden
Copy link
Contributor Author

scamden commented Apr 13, 2021

ok found another pretty major gotcha. this one is worth discussing i think, but i'll add it as 5 to the list above as well.

if i use project references internally to a project we have problems. so if in B i make B/src/subfolder with it's own tsconfig.json and index.ts and reference it from B's root one, when I run tsc or heft build on B it simply errors saying that B/src/subfolder/index.d.ts has not been built. There are only two ways to resolve this, which are:

  1. cd B/src/subfolder && tsc before building
    or
  2. tsc --build

neither of these is great because in 1) we have to write a script that knows the sub package dependencies and compiles them all in order which is terrible and reinvents the wheel of what tsc --build does. In 2) running with that flag will cause typescript not only to compile B/src/subfolder and other references within B but also all the other references to our other packages, which presumably have already been built nicely in dep order by rush. This isn't horrible because tsc --build is inherently incremental so each build is super fast but still not instant. Heft also doesn't currently offer the option to run tsc with --build and I'm not sure it should..

Not sure what the answer is here:

  1. Suggest that no one use sub project references (even though they can really speed up compile on large projects)
  2. allow the --build flag in heft and suggest that people not rely on it for cross package builds?
  3. something better? any thoughts?

(i've added an example of it to the repo, you just need to comment in the reference in libraries/webpack-side-effects-bug-dep/tsconfig.json and run rush build to see the error)

@UberMouse
Copy link
Contributor

UberMouse commented Mar 4, 2022

Stumbled across this issue trying to look up solutions to being able to use Find All References across projects and refactoring across all projects. It seems like the only solution is to use project references to cause the TypeScript language server to actually load all the projects in the repo, instead of just the ones you have open files for.

Did any further progress get made on investigating this or figuring out best practices for doing it in Rush? I'm not quite clear on how all the project references fit together but my impression is that the following needs to be done

  1. Create a root tsconfig.json to store references to every package in the monorepo
  2. Each tsconfig.json for each package needs to reference other monorepo packages the package depends on
  3. These references need to be maintained whenever dependencies change

Unclear on 2, I believe that needs to have composite set for all the packages tsconfigs because only composite packages can be referenced? Not clear on how that interacts with Heft, it gets ignored?

I don't see any configuration to set this up for the Rush monorepo, is this just something you aren't concerned about (seeing references across projects/refactoring etc) or is there another solution I'm missing?

Edit: I received an email about a comment reply mentioning declaration maps, but I don't see it? Anyway, all the projects are configured to use declaration maps so go to definition works great across projects. It's just references/refactoring that don't since only the open typescript projects in VSCode are loaded and accessible to the typescript language server

@iclanton iclanton moved this to General Discussions in Bug Triage Aug 15, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement The issue is asking for a new feature or design change needs design The next step is for someone to propose the details of an approach for solving the problem
Projects
Status: General Discussions
Development

No branches or pull requests

5 participants