Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

Appearance settings

Conversation

@rehmsen
Copy link
Contributor

@rehmsen rehmsen commented Nov 14, 2023

Before this change, IWorkbenchConstructionOptions in web.api.ts depended on IWorkspaceProvider in browserHostService.ts, which depends on some types from web.api.ts.

While this apparently does not pose problems in VS Code's build, other build system of downstream projects do not allow this, and it is also overall harder to follow the code if "the API" depends on types defined in ""the implementation".

Fixes #198220.

Before this change, `IWorkbenchConstructionOptions` in web.api.ts
depended on `IWorkspaceProvider` in browserHostService.ts, which
depends on some types from web.api.ts.

While this apparently does not pose problems in VS Code's build, other
build system of downstream projects do not allow this, and it is also
overall harder to follow the code if "the API" depends on types defined
in ""the implementation".
Copy link
Member

@bpasero bpasero left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks, I think this is fine to go in. There are more tangles with the web API (see #142646) but its unlikely I would pursue this any further because it does not cause issues in our build system.

@bpasero bpasero added this to the November 2023 milestone Nov 15, 2023
@bpasero bpasero merged commit b3a649f into microsoft:main Nov 15, 2023
@github-actions github-actions bot locked and limited conversation to collaborators Dec 30, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Dependency cycle between browserHostService.ts and web.api.ts

5 participants

Morty Proxy This is a proxified and sanitized view of the page, visit original site.