Extension development shouldn't check for engines.vscode being *

[joaomoreno]

Testing #31448

We often have to change engines.vscode to * in order to get the latest vscode.d.ts while developing an extension. If you F5 that extension you'll get an error that the vscode version isn't valid. This is always such a pain to go through every time one needs to test an extension using a new API.

IMO in extension development mode, the extension should still be loaded. This error shouldn't be there.

[bpasero]

Yeah agree.

@sandy081 moving to you since it seems the API check is done in extensionValidator.

I wonder if we should just always allow a version to be "*" given that we allow this when you run npm install in the extension?

Otherwise, the simplest way of knowing that you are running inside the extension development host is via IEnvironmentService.isExtensionDevelopment

[sandy081]

What does it mean if an extension has engines.vscode value * in a production version? This extension is compatible with any vscode? Not sure about complete impact.

Better improvement would be to allow it only during extension development.

[moshfeu]

I faced this pain when I tried to understand which engine I should specify to get the typings for createTerminal.
The solution can be either specify in the docs, from which version it can be used (just like jQuery). Or specify the version in the vscode.d.ts.

[sandy081]

Ignoring * while development might confuses the author that why it works while development and not in production (after published).

@joaomoreno What is the validation in vsce? Does it allow * ?

@alexandrudima FYI

[joaomoreno]

Though I think it does not apply since one uses VSCE when ready to publish, at which point the engine should not be *, VSCE does accept *: https://github.com/Microsoft/vscode-vsce/blob/master/src/validation.ts#L40

[sandy081]

Since we do not support * in production, we can have a guard in VSCE to prevent this?

Given that VSCE will guard it, I think it is ok to allow * while testing an extension in development

[alexdima]

I disagree with changing the engine check logic.

[sandy081]

@alexandrudima Can you please state your reasons.

[alexdima]

The engine check logic should be 100% the same when a developer tries out their extension and when the extension is installed on a user machine. In other words, it is very important to have reproducibility, especially around something that can brick your extension.

And what is the use-case? What percentage of extension authors need to use * to use the vscode.d.ts on master? Moreover, is this even the case anymore with @octref 's work? I think the vscode.d.ts is now coming in from @types, and no longer downloaded based on the engines version, so the whole issue is IMHO artificial and outdated and dangerous as it would lead to a bad change.

[octref]

I have this item in #71048 (will do it in May):

Provide a module vscode-dts, so we can run npx vscode-dts dev or npx vscode-dts 1.30 etc for testing.

The logic for npx vscode-dts dev would be:

• Download latest vscode.d.ts from master branch
• Replace the vscode.d.ts in either vscode or @types/vscode
• npx vscode-dts restore would restore correct version of vscode.d.ts
  • For vscode, restore according to engines.vscode
  • For @types/vscode, restore according to node_modules/@types/vscode/package.json

Also plan to have a hidden command npx vscode-dts proposed which does all above + downloading a copy of vscode.proposed.d.ts + enableProposedAPI: true. Hidden because we don't want too much attention on using proposed APIs.

[sandy081]

If we are going away from using vscode.engine to download vscode.d.ts, then we should make * as invalidate everywhere.

[alexdima]

If we are going away from using vscode.engine to download vscode.d.ts, then we should make * as invalidate everywhere.

It is already invalid everywhere in the runtime checks, that's why I kindly ask to not touch the logic.

[sandy081]

I mean also while installing extensions, where we check this to get the compatible extension to install.

[alexdima]

@sandy081 Do you mean there are today extensions published in the marketplace with the engine *? How is that possible? They would never load in VS Code. We only allow built-in extensions to use *.

[sandy081]

@alexandrudima Yes, there can be extensions published with * as VSCE does not restrict it, see comment. Also while looking for a compatible extension to install, we take * as valid value. There is some gap and inconsistency here.

I am sure there are no such extensions because as you said we do not load them in ext host.

[octref]

We often have to change engines.vscode to * in order to get the latest vscode.d.ts while developing an extension.

That is no longer the case. vscode-dts will download and remove the duplicate vscode.d.ts from either node_modules/vscode or node_modules/@types/vscode upon your confirmation.

Can we close this issue then? I also agree we should keep the engines.vscode working as it is now.