Layer breaker in `environmentService.ts`

[alexdima]

/common/ code should not import /browser/ code. By convention vs/workbench/workbench.web.api is considered to be /browser/. Please consider extracting IWorkbenchConstructionOptions to /common/, or moving environmentService.ts to /browser/, or maybe moving IWorkbenchEnvironmentService.options to another service.

vscode/src/vs/workbench/services/environment/common/environmentService.ts

Lines 9 to 11 in d6ccc20

	// TODO@layers
	// eslint-disable-next-line code-import-patterns
	import type { IWorkbenchConstructionOptions as IWorkbenchOptions } from 'vs/workbench/workbench.web.api';

This import came in via 92dafb3 .

(ref #140857 #140874)

[bpasero]

Yeah that options thing is ugly to the max. Need to get a bigger axe to get rid of it...

[alexdima]

One thing that might help also with the tangle would be to stop importing workbench.web.api.ts in vs/workbench/~ or vs/workbench/services/~ and move types like IWorkbenchConstructionOptions, IWorkspaceProvider, etc. into vs/workbench/~ or vs/workbench/services/~.

In here, workbench.web.api.ts is renamed to a group called workbench-meta and it's only imported by 3 files. Removing those imports would untangle things a bit:

[bpasero]

@alexdima 💯 , can you add a rule to show a warning when anyone imports workbench.web.api.ts? In fact, I have seen accident imports of IDisposable that we re-export from there in our code.

[bpasero]

Actually the only file that is allowed to import this thing is src/vs/code/browser/workbench/workbench.ts because that one literally implements the API.

[alexdima]

These warnings would show up:

src/vs/workbench/browser/web.main.ts: line 35, col 76
src/vs/code/browser/workbench/workbench.ts: line 17, col 131
src/vs/workbench/services/environment/browser/environmentService.ts: line 13, col 73

once we remove these rules:

• vscode/.eslintrc.json

  Line 234 in 5c93230

  { "when": "hasBrowser", "pattern": "vs/workbench/workbench.web.api" },

• vscode/.eslintrc.json

  Line 265 in 5c93230

  { "when": "hasBrowser", "pattern": "vs/workbench/workbench.web.api" },

• vscode/.eslintrc.json

  Line 300 in 5c93230

  { "when": "hasBrowser", "pattern": "vs/workbench/workbench.web.api" }

[bpasero]

Ok thanks, I started to work on this and can update the rules then.

[bpasero]

In my branch I am done to the one usage that is expected:

{
	"target": "src/vs/code/~",
	"restrictions": [
		"vs/base/~",
		"vs/base/parts/*/~",
		"vs/platform/*/~",
		"vs/editor/~",
		"vs/code/~",
		{ "when": "hasBrowser", "pattern": "vs/editor/contrib/*/!(test){,/**}" },
		{ "when": "hasBrowser", "pattern": "vs/workbench/workbench.web.api" }
	]
}

[bpasero]

@alexdima fyi this is merged, I wasn't entirely sure if .eslint rules can change now, I had pushed updates to the file as well, but maybe I missed something.