New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Improve documentation for `typescript.tsdk` to make workspace usage clearer #42243

Open
OliverJAsh opened this Issue Jan 27, 2018 · 5 comments

Comments

Projects
None yet
3 participants
@OliverJAsh
Contributor

OliverJAsh commented Jan 27, 2018

The typescript.tsdk setting is documented as follows:

Specifies the folder path containing the tsserver and lib*.d.ts files to use.

Given the name of this setting and its corresponding documentation, if this setting is specified in workspace settings, it's reasonable for a user to expect for VSCode to begin using that TS version.

However, that is not the case. In addition to this step, the user must manually configure the TypeScript version through the UI. https://code.visualstudio.com/docs/languages/typescript#_using-the-workspace-version-of-typescript

I understand this additional opt-in step is required for security reasons, as detailed in #30069 (comment).

However I would like VSCode to make this requirement clearer. The documentation for typescript.tsdk should make it clear that VSCode will not automatically use the specified version and additional steps are required.

As part of this I also think the relationship between the typescript.tsdk setting and the UI's TypeScript version controls could be a lot clearer. After playing around with the setting, I was eventually able to infer that typescript.tsdk defines the "global" or "workspace" TypeScript version (depending on whether it's a user or workspace setting) as seen in the UI's TypeScript version picker. The current documentation for typescript.tsdk, however, suggests that it simply sets the TypeScript version—with no mention of the required additional steps.

@mjbvz mjbvz added the help wanted label Jan 27, 2018

@mjbvz mjbvz self-assigned this Jan 27, 2018

@mjbvz

This comment has been minimized.

Contributor

mjbvz commented Jan 27, 2018

PRs welcome for vscode-docs and/or the setting itself

@OliverJAsh

This comment has been minimized.

Contributor

OliverJAsh commented Jan 31, 2018

@mjbvz A few ideas:

  • When a user opens a workspace for the first time, and that workspace has a typescript.tsdk setting, VSCode could prompt the user to approve the TS version.
  • When a user sets the typescript.tsdk setting, VSCode could prompt the user to approve the TS version.
@marcobeltempo

This comment has been minimized.

Contributor

marcobeltempo commented Feb 16, 2018

I am interested in working on this issue.

@mjbvz Do you suggest implementing a prompt after confirming tsdkVersion was set?
vscode/extensions/typescript/src/utils/versionProvider.ts

		// Allow TS developers to provide custom version
		const tsdkVersion = workspace.getConfiguration().get<string | undefined>('typescript.tsdk_version', undefined);
		if (tsdkVersion) {
			return API.fromVersionString(tsdkVersion);
		}
		return undefined;
	}
@mjbvz

This comment has been minimized.

Contributor

mjbvz commented Feb 16, 2018

@marcobeltempo Sure. I personally think the "prompt when the workspace has a typescript.tsdk" would be the more useful of the two. We actually used to have this logic but removed it.

Two considerations:

  • The prompt/notification should only happen if there is a workspace tsdk setting.
  • The prompt/notification should only happen once per workspace. Use a workspaceState: Memento to track this

The prompt used to happen when we started the tsserver itself: https://github.com/Microsoft/vscode/blob/master/extensions/typescript/src/typescriptServiceClient.ts#L336 You'd probably want to look at the VersionPicker class too for implementing this. Let me know if you need some additional pointers or if you have any questions

@marcobeltempo

This comment has been minimized.

Contributor

marcobeltempo commented Mar 2, 2018

@mjbvz if the workspace "typescript.tsdk" is set, do we want the check to happen on startup...

  1. only if the bundled version is being used
  2. if the local version us being used, should we display an information message?
  3. check to see if the workspace "typescript.tsdk" is set at any point after startup and display an appropriate message
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment