vscode: Layer violations in Structure101

In the process of testing our plugin/flavour for Structure101/g which parses JS/TS projects, we identified a few problems with the layering in VSCode 08b92738094. In a private channel, @egamma asked us to enter this issue.

Steps to Reproduce:

Install Structure 101/g Studio
Install the flavour/plugin net.betterdeveloper.javascript.imports 1.1.2
Get the VSCode commit hash 08b92738094 (to make sure it is the same src I looked at)
File->New, choose the right flavour/plugin above, and run pointing at the VS Code src dir and the tsconfig file, like this:

When the conversion finishes, you will get some layer violations:

platform is using editor. “/platform/browser/contextScopedHistoryWidget.ts imports /editor/contrib/suggest/suggest.ts”. This violation was reported in private and issue #140856 was entered by @egamma
bootstrap-amp.js uses “vs” and vice-versa:

bootstrap-amd.js	imports	vs/base/common/performance.js		
bootstrap-amd.js	imports	vs/loader.js		

vs/editor/test/common/model/benchmark/bootstrap.js	imports	bootstrap-amd.js		
vs/server/cli.js	imports	bootstrap-amd.js		
vs/server/main.js	imports	bootstrap-amd.js

base uses platform: (although that seems due to the fact that test code is stored in the same folder as production code. We will send another analysis with synthetic model transformation to emulate tests in a separate dir tree)

/base/parts/ipc/test/node/ipc.net.test.ts	imports	/platform/product/common/product.ts		
/base/test/parts/quickinput/browser/quickinput.test.ts	imports	/platform/list/browser/listService.ts

I will enable/explain Model Transformations and enter a separate comment for that mode (extracting tests out into a separate structure).

About this issue

Original URL
State: closed
Created 2 years ago
Reactions: 1
Comments: 24 (10 by maintainers)

Most upvoted comments

@sglebs I am trying on a MacBook Pro 2019, 32GB RAM.

There must have been some unlucky timing, maybe you were releasing 1.1.3rc-1 exactly when I was trying to install it. Now, on macOS I see 1.1.3rc-1, but when I tried earlier I only saw 1.1.0, so the macOS Out-Of-Memory occurred with 1.1.0. I also pointed to vscode instead of vscode/src.

I then switched to a Windows 11 VM with 8GB, where I could see 1.1.3rc-1, but which failed to install the first few times (I think the zip was being uploaded at the same time I was trying to download/install it). Also here I pointed to vscode instead of vscode/src when the Out-Of-Memory occurred.

But after correcting and pointing to vscode/src, the analysis runs fine now for me 💯 .

alexdima on Jan 19, 2022

did you point s101 at the sources in vscode/src ? Or did you point it at the root dir “vscode”?

🤦 That was the problem! Sorry about that, I’m now able to run the analysis. Thank you!

alexdima on Jan 19, 2022

@alexdima

it looks like ILaunchMainService has a method called getRemoteDiagnostics that might be better to be moved to IDiagnosticsService. I’m not sure who owns this code anymore

I can look into that if you report an issue.

bpasero on Jan 19, 2022

@bpasero it may be the case that the existing “packages”(folders) are not true components and/or layers, so circularity among them is not really a violation of the Acyclic Dependency Principle among components. In this case a logical layer containing more than one “physical” package/folder can be created as well (at least in Structure101 this is easy), so you might have circularity inside but the thing is supposed to be reused as a whole, so no big problem. Other characteristics of the layers/components should be checked (Reuse-release Equivalence Principle, etc - more details here) to decide on the proper layering. Alternatively you can create a logical layer/component with parts from separate physical folders. For example, one can create an [api] folder/package/layer as a logical thing and enforce rules for it, despite having parts spread out “physically” across folders. The possibilities are many, but I do not know how to enforce this with eslint rules.

GOTCHA: Note that bundling packages/components to form a bigger logical package may get you off the hook regarding Tangle at higher levels, but it may produce a bigger component, which increases what Structure101 calls Fat. Here is the current situation of Fat Versus Tangle in VSCode according to the analysis I did:

Trading Tangle for Fat won’t get you any closer to the origin (0,0) in most cases.

Feel free to contact me privately if you want to bounce some ideas or just want to see how things are in the tool. If I can be of any help to the project, just let me know.

sglebs on Jan 19, 2022