Web: Fix circular dependency warnings for discover and access list

[kimlisa]

Add the eslint rule 'import/no-cycle': [2, { ignoreExternal: true }] and get rid of circular dependecy's for

• [ ] discover
• [ ] access list

[ravicious]

Let's also inspect the difference in time it takes to run ESLint after enabling import/no-cycle. It might be better to delegate this responsibility to Vite with something like https://www.npmjs.com/package/vite-plugin-circular-dependency.

[ravicious]

Cyclic imports can lead not only to bad design, but sometimes they might impede enabling certain features of TypeScript or bundlers, see #47694.

import-no/cycle without ignoreExternal (I think we cannot set it as we want our own packages to count?) flags about 413 issues. The existing ones can be ignored with eslint-disable-inserter, just so that we avoid adding new issues.

However, just enabling import/no-cycle makes pnpm eslint go from 20s to 60s on my laptop. The docs for typescript-eslint do highlight that this rule is quite slow when used with TypeScript.

To be honest, I'd be fine with this perf hit as I think the benefits far outweigh it.

---

vite-plugin-circular-dependency works only during build time. This would mean that a developer wouldn't learn about a cyclic dep until the app was built, but we don't even do this in CI yet (#45056). The dev server seems to ignore errors provided by the plugin.

But even then, seeing the error only during the CI check is still better than not seeing it at all. The main problem here is that we'd first need to set up the CI to run app builds.

[ravicious]

@gzdunek I'm also not sure how effective both solutions presented here would be at preventing new "invalid" imports across packages, e.g. importing stuff from teleport in shared.

Because of the way our project is structured, adding such an import doesn't technically create a cyclic dep, as our individual packages in web/packages are not really considered to be packages, just local folders from which you can import files. We don't import from @gravitational/shared, we import from shared which is provided in tsconfig as a special path.

[gzdunek]

If we enable project references for each package, then TypeScript will take care of cross package imports. I will copy myself from the the other PR:

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

Another thing that comes to my mind is disabling TS paths and just import from @gravitational/* (I feel that having both monorepo setup and TS paths is confusing). Then we would have to disable hoisting workspace packages and simply specify dependencies in each package package.json.

[ravicious]

We can curb cyclic deps by adding import/no-cycle and ignoring existing issues with eslint-disable-inserter. Are we able to curb invalid cross package imports as well? Maybe something like import/no-extraneous-dependencies + ignoring existing issues?

[gzdunek]

import/no-extraneous-dependencies is cool. If I understand it correctly, it works with TS paths? We only need to add the respective packages to package.json's?