Create EmbedderApiRegistry to construct web API for vscode-dev consumers

[tanhakabir]

Re: #147035 and https://github.com/microsoft/vscode-dev/issues/555

This PR:

1. Creates a EmbedderApiRegistry that individual API components will register to to create final web API shape defined in web.api.ts

2. Separate out env into individual components but maintain backwards compat. @connor4312 and I thought that env seemed like a random assortment of functions and with the web API we can split them up.

   The final shape of IWorkbench: (open for feedback)

interface IWorkbench {
	commands: {
		executeCommand(command: string, ...args: any[]): Promise<unknown>;
	};

	progress: {
		withProgress<R>(
			options: IProgressOptions | IProgressDialogOptions | IProgressNotificationOptions | IProgressWindowOptions | IProgressCompositeOptions,
			task: (progress: IProgress<IProgressStep>) => Promise<R>
		): Promise<R>;
	};

	logger: {
		log(id: string, level: LogLevel, message: string): Promise<void>;
	};

	product: {
		getUriScheme(): Promise<string>;
	};

	timer: {
		retrievePerformanceMarks(): Promise<[string, readonly PerformanceMark[]][]>;
	};

	env: {
		getUriScheme(): Promise<string>;
		retrievePerformanceMarks(): Promise<[string, readonly PerformanceMark[]][]>;
		openUri(target: URI): Promise<boolean>;
		readonly telemetryLevel: IObservableValue<TelemetryLevel>;
	};

	telemetry: {
		readonly telemetryLevel: IObservableValue<TelemetryLevel>;
	};

	openUri(target: URI): Promise<boolean>;
	shutdown: () => Promise<void>;
}

3. Expose a Logger for vscode-dev extension consumers to log into. This is one log available to the embedder for extensions that provide an embedder that is loaded on that route to use before the workbench is ready and their actual extension is activated.

Question:

@sandy081 with this refractoring would it be possible to move the Output code into vs/platform/output/common? I'm getting another error with the pre-commit check after my refactoring:

[14:38:58] /Users/tanha/Documents/GitHub/vscode/src/vs/platform/log/common/webEmbedderLog.ts: line 8, col 97, Warning - Imports violates 'vs/base/common/** or vs/base/parts/*/common/** or vs/platform/*/common/** or tas-client-umd or vs/nls' restrictions. See https://github.com/microsoft/vscode/wiki/Source-Code-Organization (code-import-patterns)

where it's upset that output lives in workbench/services

[connor4312]

I believe it's still useful to have the embedder API defined as an interface somewhere, as we still need it to use in consumers, but use contributions to add to it. In here we have an empty IEmbedderApi which is not particularly useful, e.g. saying something extends IEmbedderApi it is a no-op. Instead I think we should have the IEmbedderApi, or whatever it's called, be the complete API exposed to embedders like we have today, then have parts of vscode contribute its functionality by key. So register is instead register<T extends keyof IEmbedderApi>(key: T, descriptor: SyncDescriptor<IEmbedderApi[T]>): void;.

Also, I think the registered instances can implement the prescribed interface themselves, not sure there's a benefit to nesting it in a property.

[connor4312]

I think this is starting to look good, though Ben is the expert here. Probably some opportunities to reduce duplication 🙂

[connor4312]

Hm, I wonder if we can pull this file in versus re-declaring it as the IEmbedderApi?

[tanhakabir]

Hm I think if we move where the Web API sits this would be possible. Currently it complains about import rules when I tried to import it directly.

[connor4312]

I wonder if these could these be created as an object that's automatically exported, to reduce duplication of having to redefine the API here

[tanhakabir]

This would be nice yeah. @bpasero curious if you had thoughts here

[bpasero]

🆒 , will do a full review tomorrow, some high level things:

• I would have not expected this registry to be in platform because that layer is typically only for things that are shared between editor, workbench and maybe other processes
• I would probably introduce a new file similar to the contributions.ts file that also comes with a registry concept (I would not put it into the same file because that file is for workbench contributions only)

[tanhakabir]

After moving the embedder contributions to a singe file embedder.contribution.ts which lives under vs/workbench/browser I don't have any import errors anymore. So I'm going to publish this PR

[bpasero]

Sorry for the confusion but I had actually liked the approach of contributing to this new registry from the location of where the component is. The idea of registries are to decouple the components that contribute from each other. If for example we decide to expose an embedder API for the terminal component later on, the contrib/terminal component could contribute to the registry easily without coupling this into the core workbench.

This is somewhat similar/related to our extension API in workbench/api but there we have taken a different approach and basically grouped all providers into a single folder. As a consequence we had to relax our layers checker for workbench/api because that folder pretty much depends on everything, even though it is not in workbench/contrib. We could follow a similar model and have something like api-embedder folder in workbench, but I am not a big fan of that tbh.

@jrieken as the creator of workbench/api, I wonder if you had thoughts on whether we should follow a similar approach for embedder API or go with the registry approach that I had suggested. I see one advantage of the way extension API does it: you can immediately validate vscode.d.ts by implementing it from exthostApi.impl.ts. We will have a similiar challenge once we introduce an embedder API file (refs #125643).

But I think even for extension API it would be nice if respective components could provide the API implementation in a contribution style.

[jrieken]

@jrieken as the creator of workbench/api, I wonder if you had thoughts on whether we should follow a similar approach for embedder API or go with the registry approach that I had suggested.

The registry approach seems a tad over-engineered to me. I see how it keeps things in "their places", e.g folders, but I don't believe we plan for having multiple implementations of one API slice or that we would ever ship with a partial API. The way our API facades work is that they are a "pretty focal point" of everything.

[bpasero]

Fair enough. I think this maybe deserves a bit of more discussion to come to a solution how the embedder API shape is going to be in the future. Ideally we can put all the problems with our current solution on the table and find a good solution, so far I see:

• how to provide a place for the implementation of the API that scales in the future (similar to our extension API) and avoids duplication like we have today
• 💪 how to produce a good d.ts file out of the API that is also validated (like vscode.d.ts is) (Web: provide a workbench.web.api.d.ts for an embedder to develop against #125643)

Can we move this to May then and plan for it? Otherwise if the API is really needed now, then I suggest to just throw it into the current format and have it cleanup later. We just have to make sure that we remain the only ones using it if we are going to change it later.

[tanhakabir]

It would be nice to get in the embedder logger for this iteration. I can reopen my older PR which used the current format of how the API is constructed and we can see in the May iteration how to best refractor the embedder API?