Add command to manage proposed api enablement

[JacksonKearl]

This PR fixes #106427. Well not really because we disallow it in stable, but it fixes the part where it broke in Insiders too.

New model:

1. User sets hidden setting "extensions.enableProposedAPIOverride": true
2. Command "Developer: Manage Proposed API for Extensions" becomes available
3. Upon running the command, a big scary warning is shown:
   
4. Upon accepting, user can pick from a set of extensions which have requested proposed api but are not granted it via product.json/being builtin/etc. (some extra entries are shown here as they aren't in OSS's product.json)
   
5. Upon choosing which extensions to enable, user gets notification to restart:
   

The list of overrides is stored in local storage, so we can't access it from the EnvironmentService (sad @jrieken), but also it doesn't require a file read (happy @alexdima). Also I got rid of all of the modifications to argv.json (happy @bpasero)

[jrieken]

@JacksonKearl I do not understand what's being solved here. If you are actively developing an extension than you are free to use proposed API anyways. It seems that you want to enable seamless selfhosting/consumption of extensions that use proposed API by anyone and that's something we should never support

[alexdima]

I agree that I would also like to understand what is the usecase and why do we think this is a good idea.

[gjsjohnmurray]

My take on the situation:

1. For some time now it has been possible for users of Stable or Insiders to set an extension id in the enable-proposed-api array of argv.json and thereby turn on the proposed APIs that extension uses. The extension will have had to be built with "enable-proposed-api": true in its package.json. This bars its publication on Marketplace, so the user will have had to download the VSIX from elsewhere (e.g. a GH release artifact).

2. Recently (enable-proposed-api in argv.json doesn't work in Stable #106427) there was a report of this no longer working for Stable, though personally I haven't seen this problem. Latterly @JacksonKearl reported the argv.json technique didn't work reliably even on Insiders. This led to PR Fix argv.json proposed api flag #110461, which wasn't popular and was ultimately closed unmerged.

3. This PR takes a different approach. It removes support for enable-proposed-api from argv.json, for both Stable and Insiders, and replaces it with a mechanism that only works in Insiders. Personally I'll be sad to lose the ability to get end-users of my extensions able to use proposed APIs on Stable. But if this happens I'll ask them to use Insiders. If they're nervous about getting daily updates I guess they can turn off automatic update and instead perform one manual update right after a new Insiders comes out right after a new Stable.

We're having to send our end users down the enable-proposed-api path (despite the inconvenience of having to download a VSIX and install it manually) because we use FileSystemProvider and both FileSearchProvider and TextSearchProvider remain stubbornly in proposed state with no indication of progress to finalization (see #73524 and #59921). I know @usernamehw shares similar frustrations about other proposed APIs.

[jrieken]

For some time now it has been possible for users of Stable

Should have never been like and I don't know of any doc/resource that states the opposite.

This bars its publication on Marketplace,

There are very easy ways around this.

Personally I'll be sad to lose the ability to get end-users of my extensions able to use proposed APIs on Stable. But if this happens I'll ask them to use Insiders.

That shows a fundamental misunderstanding of what proposed API is for. This is a feature for extension authors, not extension consumers.

Sorry, this will now go in a direction which you won't like: I will refresh my memory for why we added support to enable proposed API via command line flags and I will work towards removing that. We do support proposed API via product.json which is a very well curated list and it should be the only list we have

[jrieken]

My proposed follow up item: #110729

[JacksonKearl]

In my opinion, the reason we get so little feedback from extension authors regarding Proposed API is that it's too hard to selfhost extensions using Proposed API, so they end up not being created in the first place, or even if they are created they don't get enough hours of use for issues to really present themselves.

We get very little feedback on API and I don't want to blame anyone for that. -jrieken

If you ask me we can only blame ourselves -- extension authors aren't going to dedicate lots of their time to learning a new API and helping us out by providing feedback if we don't even let them selfhost the extension they're creating without jumping through hoops every time they launch the app.

Take for instance Notebooks, we only have a handful of Notebook extensions allowed in product.json, and none of them get much usage at the moment. That limits our ability to get feedback about the experience and means we're pushing more bugs/feature requests/polish off to when the API goes stable and many more users are encountering the issues (and it's harder to make changes as backwards-compatibility becomes essential), whereas we could instead be allowing dedicated extension authors to easily selfhost the API now and file issues now, such that we can quickly fix them while backwards-compatibility is non-essential.

If I understand correctly, the concern with opening up too many extensions to use Proposed API is:

• End users shouldn't have extensions break with VS Code updates, as that increases reluctance to upgrade and thereby increases fragmentation. Extensions that use Proposed API are likely to break with VS Code updates. We don't want fragmentation, so we should limit the ability for extensions to use Proposed API.

I agree that fragmentation is to be avoided, but I believe we mitigate this by only opening up to Insiders users (who have self-selected to update daily) and showing the flashing warning dialog explaining that this isn't to be used outside of testing purposes.

---

@gjsjohnmurray

instead perform one manual update right after a new Insiders comes out right after a new Stable.

Probably better to use the insiders right before the new Stable, as that's the same as Stable. The one right after a new Stable comes is the least hardened of any in the cycle (1+ week of dev work with 0 user testing 🙂)

[JacksonKearl]

After discussing with @kieferrm we agree the value added by getting more use of proposed api is substantial, accordingly I've removed the hidden setting feature flag and slightly modified the wording:

Also @sbatten had an interesting idea to disable all escape hatches in Stable (such as --enable-proposed-api via CLI), but in exchange open up some access points in Insiders (such as this, or even allowing proposed api for all extensions by default).

[alexr00]

I have had success with asking individuals who want specific API to try out a proposal and confirm that it works well for them. I have seen no need to enable selfhosting of proposed API.

[alexdima]

👍 LGTM.

It would be nice, as the complexity of ProposedApiController grows, that we extract the code to a separate file or even go so far and create a new service, IProposedApiService, which encapsulates all this logic.

I suggest that @JacksonKearl and @jrieken you get together and decide what you do. The proposed API feature was initially implemented by @jrieken and I think his review is more important than mine. I own the file, but not the proposed API mechanisms, that's why I suggest to extract the code somewhere else, since my review is not really needed.

[jrieken]

Still 👎 With this approach any user will be able use extensions consuming proposed API without our consent nor knowledge. That will play against our reputation of being a stable host for extensions. We will break proposed APIs and extensions will fail and what happens is that bugs are filed against us. You will see the dialog today and in a couple of month when your favourite extension stops working you will not remember that you have clicked a button. Take this issue as a sample: #110569. To blame is LiveShare which hasn't adopted a change in vscode.proposed.d.ts that was made ~2month ago. For LiveShare we at least know to whom to talk to (enough tho the promise was we don't need to do that) but for extension foo on machine bar we have zero knowledge and I will not deal with those issues and don't think that you have a good answer for how to deal with such issues.

Apart from the massive headaches this will be causing I am actually not convinced that "more users of feature using proposed API Y leads in good or more feedback on the proposed API Y". I cannot follow that logic and I actually don't recall issues where such feedback has been provided. Also not for the search api proposal which apparently gets used via command-line tricks.

[jrieken]

Since apparently nobody read #110729 (comment). It says

A stretch goal is to have a process for how to onboard extensions to be in the product.json allow list.

What that means, and what the LiveShare case (#110569) shows, is that listing extension identifiers in product.json is already insufficient. So, instead of relaxing things even further (as with this PR) we should go the opposite direction, make things stricter, and then we can hopefully onboard more extensions onto proposed API usage.

What I mean: We have no idea what proposed API an extension is using (and shipping with). We have no idea when those extensions are broken because of our changes. Also, extension authors apparently have no idea when they are broken because their CI isn't proper and because they aren't on top of things. The result is a broken experience in VSCode and issues filed against us (again see #110569).

What I propose: We want to know when an extension (that uses proposed API) doesn't work any more. With that we would simply disable it and tell the user something like "extension foo is disabled because it didn't adopt changes of preview features it uses". I believe the granularity should be a proposal (which in vscode.proposed.d.ts is represented as regions, sample). The version can be implicit, like a content hash of the proposal, or explicit, like a version number in the region-comment. An extensions package.json will list a proposed API and version and we do the checks, disablement, informing users etc. Will all that in place I would feel comfortable to have more extensions listed in product.json or even not have it any more

[jrieken]

Would strict version of proposed api allow using it in Stable (since it will know if the api has changed or not) and publishing proposed extensions to the marketplace?

Yeah. At least that's how it is today with extensions listed in product.json. Tho, it is painful to react to API changes since you would then need two versions of your extension. @eamodio brought up the idea to add a way to check for the availability of a certain API, like vscode.env.hasProposedApi(proposalId: string, proposalVersion: string): boolean. That should make it simpler to write an extension using proposed API

[jrieken]

. @eamodio brought up the idea to add a way to check for the availability of a certain API, like vscode.env.hasProposedApi(proposalId: string, proposalVersion: string): boolean.

Wait... What we didn't consider is that such a function means that we cannot disable an extension when its declared api usage isn't matching the actual proposed API shape. We would need to rely on extensions using this check and otherwise obscure failures happen. I don't think we want that and that we should start with "little power"

[JacksonKearl]

We cannot disable an extension when its declared api usage isn't matching the actual proposed API shape

Spitballing some way we could make the dynamic proposal availability checking well-typed and enforceable...

vscode.proposed.d.ts

//#region https://github.com/microsoft/vscode/pull/110617
//#id ExampleProposal
export function myProposedAPI(): void;
//#endregion

vscode.proposalHashes.ts (generated at buildtime)

export const ProposalHashes = {
   ExampleProposal: 'S0M3HA5H',
   ....
}

vscode.proposalGates.d.ts (generated at buildtime)

declare module 'vscode' {
   export namespace env {
       export function acquireProposedApi(
          id: 'ExampleProposal', 
          version: ProposalHashes['ExampleProposal']
       ): undefined | vscode & { /* type of the proposal */ }
      ... etc for all proposed ...
  }
}

extHostImpl.ts

function myProposedAPI() {
    if (!extensionProposalService.hasProposalGrant(ext, ExampleProposal) 
      throw "Extension needs updating blah blah blah")
    doMyProposedAPI()
}

function acquireProposedApi(id: string, version: string) {
   if (version === ProposalHashes[id]) { 
      extensionProposalService.grantProposedAPI(ext) 
      return vscodeAPIImpl
   }
   console.warn(ext, 'wants proposed api but blah blah blah')
   return undefined
}

extension.ts

import * as vscode from 'vscode' // Only including vscode.d.ts and vscode.proposalGates.d.ts 

activate() {
   const vscodeWithProposal = acquireProposedApi('ExampleProposal', 'S0M3HA5H');
   if (vscodeWithProposal) {
      vscodeWithProposal.myProposedAPI();
   }
}

Some open questions remain, including how extensions migrate from proposed to stable when proposals become solidified, and how we could handle making backwards compatible changes to the API's without breaking everything when the hash changes.

[jrieken]

This is an interesting proposal but things are more complex (as always). Not all proposed APIs are symbols that you can probe for, some are just "receivers". Take this for instance:

vscode/src/vs/vscode.proposed.d.ts

Lines 1016 to 1022 in 12b808c

	export class TreeItem2 extends TreeItem {
	/**
	* Content to be shown when you hover over the tree item.
	*/
	tooltip?: string | MarkdownString | /* for compilation */ any;
	}

Newly proposed is the | MarkdownString overload and the enforcement of that API is done only when reading.

Also, in the approach this is assumed:

import * as vscode from 'vscode' // Only including vscode.d.ts and vscode.proposalGates.d.ts 

Assuming that proposals aren't defined in the vscode-module is really big trouble because them being so and being merged by TypeScript is the backbone of validating the actual api implementation.

In short, I stand by my original strictness of being able to tell that a proposal isn't available statically and then disabling extensions. I agree that feature probing is somewhat cool tech but it is a non-goal of the API proposal process.

For now, I would propose that we focus on the very first step which is to define a declaration for identifying and versioning proposal. Some format that can be authored in vscode.proposed.d.ts and that is trouble-tree. Ideas/questions

• Is the identifier of a proposal the issue number of its tracking issue?
• Is the version the content hash of the proposal source text? Would changing a comment or whitespace be an API change then?
• Is the version something the author needs manually update?
• Is there tooling that tells us when a proposal changed but its version hasn't been updated?
• How do extension authors know the proposal versions?

Given those considerations (which are likely incomplete) I would propose something like this:

• in vscode.proposed.d.ts we author (manually) the proposal id and version
• we add a lint rule that computes the version based on non-trivia text of the proposal
• the lint rule has an auto-fix to update the version

Sample:

//#region https://github.com/microsoft/vscode/issues/998877 cfe87ae

/**
 * Bla, api is cool
 */ 
export const foo: number;
//#end region

The following is true

• https://github.com/microsoft/vscode/issues/998877 is the proposal identifier (maybe 998877 as short form)
• cfe87ae is the version
• cfe87ae === magic_hash('export const foo: number;')

With that information extension authors can quickly look-up what proposal and version they are toying and we have little hassle with maintaining that information

[gjsjohnmurray]

Might be a good idea if Issue Reporter's list of extensions indicated which ones have proposed API enabled. Then we just need to get people to use Issue Reporter...

[jrieken]

This will not happen