Skip to content

Commit

Permalink
fixup! docs(rfcs): add type-checking-perf-improvements
Browse files Browse the repository at this point in the history
  • Loading branch information
Hotell committed Feb 9, 2024
1 parent c86d50f commit aaab11f
Showing 1 changed file with 14 additions and 11 deletions.
25 changes: 14 additions & 11 deletions rfcs/shared/build-system/08-type-checking-perf-improvements.md
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ _Feb 2024_

During our research about "[Faster CI](https://github.com/microsoft/fluentui/issues/27359)" we discovered various opportunities for improvement.

This RFC will tackle approach on "Stop using TS path aliases on CI", aka make type-checking fast.
This RFC will tackle "Stop using TS path aliases on CI" problem, aka _make type-checking fast 🚅_.

## Problem statement

Expand Down Expand Up @@ -68,13 +68,13 @@ There are multiple options how to achieve this.
1. generate `react-components` suite dts before any `type-check` runs
2. get rid of circular dependencies

### generate `react-components` suite dts before any `type-check` runs
### 1. generate `react-components` suite dts before any `type-check` runs

We can imperatively traverse graph and generate `d.ts` for packages that cause circular dependencies as can bee seen in this [PR](https://github.com/microsoft/fluentui/pull/28002/files#diff-38d7f0411164173eb390d7191ad068bfe70935082bc37198b6b35b00408b2b42).

**This dts generation can be done in 2 ways:**
This dts generation can be done in 2 ways:

1. Run it manually [before all tasks](https://github.com/microsoft/fluentui/pull/28002/files#diff-7915b9b726a397ae7ba6af7b9703633d21c031ebf21682f3ee7e6a4ec52837a5)
**1. Run it manually [before all tasks](https://github.com/microsoft/fluentui/pull/28002/files#diff-7915b9b726a397ae7ba6af7b9703633d21c031ebf21682f3ee7e6a4ec52837a5)**

```yml
- script: node ./scripts/executors/type-check-ci-hack.js --base $(targetBranch)
Expand All @@ -90,7 +90,7 @@ We can imperatively traverse graph and generate `d.ts` for packages that cause c
- very hard to repro on local machine
2. Leverage task graph and define dependency on every v9 `type-check`
**2. Leverage task graph and define dependency on every v9 `type-check`**

```jsonc
// @filename <package-root>/project.json
Expand All @@ -114,7 +114,7 @@ We can imperatively traverse graph and generate `d.ts` for packages that cause c

- all problematic packages needs to override `targets#type-check` with `dependsOn` declaration

#### Pros and Cons
#### Solution overall Pros and Cons

**Pros:**

Expand All @@ -125,7 +125,7 @@ We can imperatively traverse graph and generate `d.ts` for packages that cause c
- brittle solution for future library requirements
- a "hack" that is not solving circular dependencies

### get rid of circular dependencies
### 2. get rid of circular dependencies

In order to get rid of circular dependencies, we need to move problematic parts of code to separate packages.

Expand Down Expand Up @@ -161,7 +161,7 @@ react-components/

- this will increase with every new scoped domain boundary introduction (`react-text-vr-test` etc)

- **2. nested folder structure**
**2. nested folder structure**

```diff
react-components/
Expand Down Expand Up @@ -193,13 +193,16 @@ react-components/
- ? not sure about consistency between node and web packages
- node packages don't have stories, so in order to be consistent we would need to move all implementation in a `libraries` subfolder as well

#### Pros and Cons
#### Solution overall Pros and Cons

**Pros:**

- clean domain separation that scales
- no "hacks" as in 1st solution
- ability to turn on eslint rules for circular dependencies
- no "hacks"
- ability to turn on eslint rules to prevent circular dependencies
- ability to leverage stories implementation further. for examples
- shipping demos to npm if needed
- leveraging storybook stories for more tests https://storybook.js.org/docs/writing-tests

**Cons:**

Expand Down

0 comments on commit aaab11f

Please sign in to comment.