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-stack] Analyzing .ts files instead of .d.ts files #1379

Closed
tbrannam opened this issue Jul 12, 2019 · 3 comments
Closed

[rush-stack] Analyzing .ts files instead of .d.ts files #1379

tbrannam opened this issue Jul 12, 2019 · 3 comments
Labels
general discussion Not a bug or enhancement, just a discussion

Comments

@tbrannam
Copy link

I'm curious as to why Midgard is looking for another solution to their mono-repo and how does that reflect on Rush?

see: https://medium.com/@vincentbailly/evolving-a-large-typescript-repository-4a32c61e94fd

@octogonz
Copy link
Collaborator

Microsoft doesn't centrally dictate how a team should do their work. Each product group has quite a lot of autonomy in the approaches and tools that they choose. The Midgard developers aren't even in the same time zone as the Office group where Rush originated, so it's perhaps not directly on their radar, but I know they evaluated Rush and asked questions about it in the past.

It's healthy for different teams to experiment with different approaches. Today Rush focuses on a fairly specific niche (the "scalable" monorepo servicing lots of teams and projects). This niche has its pros and cons. Personally, my own interest is to solve those engineering problems as best we can; the motivation isn't really about download statistics or adoption. If every monorepo at Microsoft ends up using Rush, that should happen because each team determined that Rush was the best tool for the job, not because they felt pressure to "align" with the rest of the company. Of course, I can only speak for myself. :-)

@octogonz octogonz added the general discussion Not a bug or enhancement, just a discussion label Jul 13, 2019
@octogonz octogonz changed the title Midgard and Rush [rush-stack] Analyzing .ts files instead of .d.ts files Jul 13, 2019
@octogonz
Copy link
Collaborator

octogonz commented Jul 13, 2019

We should use this issue to discuss the actual topic of the blog post, since it's very relevant to the proposed watch improvement for Rush / Rush Stack. Vincent's idea is summarized in this excerpt:

  • Confusing watch output: Because the interface of a package is its JS and DTS files, a process needs to be run to update them at each time we change a TS file. To do this, each package has a process called “watch”. It is a process which is idle most of the time and updates the JS and DTS files when a TS file changes. We need to run this watch process on every package we may edit. Since we don’t know in advance which packages we will end up editing, we usually use Lerna to run the watch process in each package of the dependency tree of the component we work on. This means that all the watch processes run in the same console making the console output confusing, a build error can easily go unnoticed.
  • IntelliSense implicit dependency: If we don’t run the “watch” process in every package we modify, IntelliSense will provide wrong type information and some features may be broken. This is because IntelliSense relies on the DTS files. This means that IntelliSense depends on a separate process that we need to run manually, which is not obvious.
  • Incomplete refactoring experience: When we use DTS files as package interface, IntelliSense cannot provide certain refactoring features. IntelliSense does not seem to always be able to find the consumers of a package public interface. This means that features like “Rename symbol” or “Find all references” do not consistently work cross-packages.

As mentioned at the beginning of this article, let’s explore an idea which could solve the issues we just described.

The idea is to change our packages pubic interfaces from JS+DTS files to TS files.

This idea is illustrated in the tsloader branch of the sample repository.

The Rush community proposed a somewhat different approach in #1151. Rather than launching a separate --watch process for each project, our idea was to have a single Rush NodeJS process centrally monitor the entire monorepo (or rather, a designated subset of projects that the developer has chosen to work on). Then there would be an API contract that allows Rush to invoke the toolchain for individual projects when they need to rebuild.

Having the compiler analyze .ts files instead of .d.ts files is an interesting alternative. What's particularly interesting is that they treat IntelliSense as a completely separate compiler configuration from the build+bundling (+ other stuff like API Extractor .d.ts rollup) that needs to happen for a "real" build.

The advantage is that the VS Code refactoring can treat the entire repo as a single compiler analysis.

The disadvantage seems to be that the analysis is overall more expensive. For example, a while ago we changed API Extractor to analyze .d.ts files instead of .ts files, and it greatly reduced the amount of work that the compiler has to do. The .d.ts files are very quick to process, because they contain only type signatures, and those signatures are in a nice normalized form.

In my own work, I never open VS Code at the root of a monorepo. Instead, I open a separate VS Code window for each project that I want to edit. This limits each IntelliSense analysis to a single tsconfig.json for a single project. If you want to scale up to thousands of NPM packages (instead of 100), intuitively it seems that at some point the compiler would choke trying to analyze the entire monorepo. But this is just a speculation. We'd need to do some scientific measurements to know for sure. And "incomplete refactoring experience" is indeed a casualty of that approach.

@tbrannam
Copy link
Author

Thanks for the considered response, I look forward to hearing more about the approaches and also look forward to reading up on rush stack

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
general discussion Not a bug or enhancement, just a discussion
Projects
None yet
Development

No branches or pull requests

3 participants