Make web/design a separate TypeScript project

[gzdunek]

Part of #21075

This PR makes web/design a separate TS project using project references.
This will allow us to enable strict: true gradually, starting for web/design.

When it comes to project references, AFAIK the best structure is to have tsconfig.json in each package, and then reference them all in the root (just how TypeScript does this). This is currently not possible in our case, because project references don't allow circular imports, and we still have quite a few web/teleport imports in web/shared. Because of that, I made web/design a separate project, everything else is handled by the root tsconfig.json (or tsconfig.node.json for Vite stuff).

Another thing when using project references is that the referenced project must at least generate declaration files. I was looking for a good place in our repo to store them and found build.assets/.cache. I'm not sure if this is the best option, so if you think there's a better place, I'm open to suggestions.

[gzdunek]

It appears that this tsconfig was just ignored before, because we didn't run tsc with --build flag (so project references didn't work).
After I added that flag, I got a few errors related to Vite files. I hope my fixes are fine 🤞

[gzdunek]

This works nice in IDE, where "go to file" goes directly to the source file, not a .d.ts file. In the CLI however, tsc prints a path to the .d.ts file:

web/packages/teleterm/src/ui/DocumentAccessRequests/RequestList/RequestList.tsx:122:13 - error TS2322: Type 'boolean' is not assignable to type 'string'.

122             isSortable: true,
                ~~~~~~~~~~

  build.assets/.cache/ts/design/src/DataTable/types.d.ts:76:5
    76     isSortable?: string;
           ~~~~~~~~~~
    The expected type comes from property 'isSortable' which is declared here on type 'TableColumn<AccessRequest>'

Unfortunately, I didn't find a way to overcome it.

[ryanclark]

If you make tsconfig.json

{
  "files": [],
  "references": [
    { "path": "./tsconfig.node.json" },
    { "path": "./tsconfig.root.json" },
    { "path": "./web/packages/design/tsconfig.json" }
  ]
}

and tsconfig.root.json (maybe this can go in a different folder)

{
  "extends": "./tsconfig.base.json",
  "include": [
    "e/web/**/*.ts",
    "e/web/**/*.tsx",
    "e/web/**/*.js",
    "e/web/**/*.jsx",
    "web/**/*.ts",
    "web/**/*.tsx",
    "web/**/*.js",
    "web/**/*.jsx",
    "../web/src/**/*.ts",
    "../web/src/**/*.tsx"
  ],
  "compilerOptions": {
    "noEmit": true,
    "types": ["node", "@types/wicg-file-system-access"]
  },
  "exclude": [
    "web/packages/design/**",
    "node_modules",
    "**/node_modules/*",
    "**/build/app/**",
    "**/build/release/**"
  ]
}

You can set noEmit in both tsconfig.node.json and design/tsconfig.json

{
  "compilerOptions": {
    "composite": true,
    "target": "ESNext",
    "module": "ESNext",
    "moduleResolution": "bundler",
    "allowSyntheticDefaultImports": true,
    "noEmit": true
  },
  "include": [
    "e/web/**/*.mts",
    "web/**/*.mts",
    "web/packages/build/vite",
    "web/packages/teleterm/csp.ts"
  ]
}

{
  "extends": "../../../tsconfig.base.json",
  "include": ["src", "@types", "../../../web/@types"],
  "compilerOptions": {
    "composite": true,
    "noEmit": true
  }
}

Type checking still seems to work as expected (I made a change in a vite config and it complained, I added strict: true to design's tsconfig.json and it complained only about strict things in the design directory)

It does, however, surface one type issue for some reason, that your PR doesn't

web/packages/design/src/Image/Image.tsx:59:12 - error TS2339: Property 'propTypes' does not exist on type 'styleFn'.

59   ...space.propTypes,
              ~~~~~~~~~

web/packages/design/src/Image/Image.tsx:60:12 - error TS2339: Property 'propTypes' does not exist on type 'styleFn'.

60   ...color.propTypes,
              ~~~~~~~~~

web/packages/design/src/Image/Image.tsx:61:12 - error TS2339: Property 'propTypes' does not exist on type 'styleFn'.

61   ...width.propTypes,
              ~~~~~~~~~

web/packages/design/src/Image/Image.tsx:62:13 - error TS2339: Property 'propTypes' does not exist on type 'styleFn'.

62   ...height.propTypes,
               ~~~~~~~~~

web/packages/design/src/Image/Image.tsx:63:15 - error TS2339: Property 'propTypes' does not exist on type 'styleFn'.

63   ...maxWidth.propTypes,
                 ~~~~~~~~~

web/packages/design/src/Image/Image.tsx:64:16 - error TS2339: Property 'propTypes' does not exist on type 'styleFn'.

64   ...maxHeight.propTypes,
                  ~~~~~~~~~

[gzdunek]

Yeah, it does seem to work... but I'm curious why :)

1. You set noEmit: true and composite: true in tsconfigs for design and node. I remember running into errors when I tried this. So why isn’t TypeScript complaining here?
2. tsconfig.root.json doesn't have composite: true set, although it is a referenced project. The docs says it is obligatory https://www.typescriptlang.org/docs/handbook/project-references.html#composite. After I set it, I get a lot of errors related to missing design files.

My concern is that this might be working due to relying on undocumented behavior, and a future TS version can break this.
While the setup in this PR is not ideal (I'd like to only have project references in tsconfig.json like in your comment), I see this as a temporary step toward the goal of having tsconfig.json in each package. After we get rid of teleport imports in the shared package, we should be able to do this.

What do you think?

[ryanclark]

tsconfig.root.json doesn't have composite: true set, although it is a referenced project. The docs says it is obligatory https://www.typescriptlang.org/docs/handbook/project-references.html#composite. After I set it, I get a lot of errors related to missing design files.

Ah, this is my bad. I thought I had composite on all of them. Makes sense that you get errors for missing design files - we could not ignore them in tsconfig.root.json (if the main goal is to have strict mode typing enabled for design, then it doesn't matter if they're type checked twice) but that also isn't the most ideal setup. Happy with whatever you prefer

[gzdunek]

Makes sense that you get errors for missing design files - we could not ignore them in tsconfig.root.json (if the main goal is to have strict mode typing enabled for design, then it doesn't matter if they're type checked twice) but that also isn't the most ideal setup. Happy with whatever you prefer

I guess I'd have to add design/tsconfig.json as a project reference to tsconfig.root.json so that it knows where to get these files from. But I think at this point the configuration without tsconfig.root.json is a bit simpler.

[ravicious]

When it comes to project references, AFAIK the best structure is to have tsconfig.json in each package, and then reference them all in the root (just how TypeScript does this). This is currently not possible in our case, because project references don't allow circular imports, and we still have quite a few web/teleport imports in web/shared.

How exactly does this fail? What kind of error do you get?

We have to curb somehow the growth of cyclic imports, even within individual packages as well. Otherwise we'll never get rid of this workaround. We can discuss that under #32940 I suppose.

[gzdunek]

How exactly does this fail? What kind of error do you get?

When each package has its own tsconfig.json, importing code from one package (e.g., design) in another (e.g., shared) requires adding design/tsconfig.json as a referenced project within shared/tsconfig.json (otherwise TS won't know where the files for design are).
If you then try to add shared as design's referenced project you will get a following error:

Project references may not form a circular graph. Cycle detected: /Users/grzegorz/code/teleport/web/design/tsconfig.json

[ravicious]

This would definitely be perfect to enable once we get rid of those bad imports.

[avatus]

LGTM!!