Investigation: do not restart Extension host when first folder changes

[bpasero]

Today we restart the extension host whenever the first folder changes because workspace.rootPath is always set to that first folder and it never used to change after the extension host started once.

If we would not restart the extension host anymore an extension:

• could use workspace.rootPath as before, it would still point to the first folder always (which can be undefined if the workspace contains no folders)
• could subscribe to the onDidChangeWorkspaceFolders event to get notified about updates
• would see the value of workspace.rootPath changing during runtime whenever the first folder changes

//cc @sandy081 @jrieken

[roblourens]

This is affecting Live Share because they want to be able to track workspace changes during a live share session. If the EH reloads, then the host will have to restart their session.

If we wanted to more aggressively deprecate rootPath, we could make it undefined whenever a workspace is open. I wonder how many extensions still rely on it.

FYI @cait1inhennessy

[roblourens]

As a workaround, they can keep a dummy folder in the workspace during a session, but the user needs to know to not touch that folder, so it's not a very good workaround.

[bpasero]

@roblourens

This is affecting Live Share because they want to be able to track workspace changes during a live share session. If the EH reloads, then the host will have to restart their session.

To be clear: the transition from an empty window or single-folder workspace to a multi-root workspace will always require an extension host restart because the workspace ID changes and thus the state location for example. Please check with them if they really start from an empty workspace and then add a folder.

[roblourens]

Yeah, we realized that, and they are able to start from an actually empty workspace. I think that if you cause a window reload, it will at least be obvious why your liveshare session terminated.

[bpasero]

I am all up for making this change, but we have to decide if:

• rootPath is undefined in multi-root
• rootPath changes in multi-root based on what is the first folder

I feel the former is a change with more impact on todays extensions compared to the latter.

From the API

/**
 * ~~The folder that is open in the editor. `undefined` when no folder
 * has been opened.~~
 *
 * @deprecated Use [`workspaceFolders`](#workspace.workspaceFolders) instead.
 */
export const rootPath: string | undefined;

I think we need to bring this up in an API call to decide, but I will not have cycles. This needs a champion to drive.

[roblourens]

I think we need an idea of how many extensions this would affect. @sandy081 I am using your tool but it seems to be showing me old versions of extensions that used this api, how do I limit it to newer versions?

I think the first option would actually be less impactful. It would mean that some extensions can't work in a multiroot workspace, but they should handle the case of no folder already for an empty window. The second option would break an assumption that those extensions may be making, that rootPath will not change, and we don't know what that would do.

I can bring it up in an API call.

@cait1inhennessy we will discuss it some more because I think this is the right change to make, but don't count on us changing it this month, you will probably have to plan to use the workaround with a dummy folder in the short term.

[jrieken]

@roblourens What is your thinking when the first workspace-folder isn't a folder on disk. E.g what would rootPath be when the first folder is fooScheme://barServer/bazz/path? Do you want to go with undefined or just pretend it's /bazz/path?

[jrieken]

I think the first option would actually be less impactful.

Oh, ignore my last comment...

but they should handle the case of no folder already for an empty window.

From what I remember when we checked back then extensions often show a message like "Open a folder first"

[bpasero]

Yes, now that I think about it, I would also prefer that rootPath is undefined once you are in a multi-root workspace (independent from how many folders you configure). That is a scenario that every extension out there must gracefully handle already, so we will not break any assumptions. It may however result in certain extensions becoming useless (because maybe no longer maintained).

[jrieken]

step 1: know how many users use multi root workspaces and could be affected by this
step 2: know what extensions will be affected by this and how

[bpasero]

@roblourens feel free to delete WorkspaceChangeExtHostRelauncher once decided that we go with the approach outlined.

rootPath is defined here and would need to be changed accordingly.

If you cannot reserve time this milestone, feel free to move back to me "On Deck".

[jrieken]

If you cannot reserve time this milestone,

Since this is a high risk change I would not do anything else than gathering data this milestone. Maybe show an aggressive prompt when developing extensions that use this API.

Also adding @dbaeumer since I believe that the LSP-client has a reason to use the API (maybe not anymore?)

[gjsjohnmurray]

@dbaeumer unfortunately I haven't seen much activity on the issues I filed. I'm not sure exactly what we need to move forward

@roblourens I just nudged those issues you raised a year ago.

[gjsjohnmurray]

Does this deserve to be on a milestone? For a while during 2020 it was.

[roblourens]

I think the latest status is still #69335 (comment). Don't know whether anything came out of @dbaeumer's discussions. Still not a lot of activity on the affected extensions it seems.

[dbaeumer]

Actually I am not sure. Lets ping @egamma again. @egamma please see: #69335 (comment)

[bpasero]

In the light of data loss issues such as #122992 I really think we should stop restarting extension hosts.

[michaeljones]

I'd like to ask for clarity on this as I'm seeing an issue with the extension I am working on. What exactly does "first folder changes" mean?

We have an operation that adds a new folder to VSCode and if we do that in an empty or non-empty saved workspace then the extension host seems to restart all the extensions. The folder does appear at the top of the workspace list. Is that this issue? Is there anything we can do about it? Or should extensions be resilient to restarts? We're seeing "retain context when hidden" webviews being left unresponsive as they are associated with the previously extension "run time" or "instance".

[gjsjohnmurray]

When your operation adds a folder to a workspace that already has at least one folder, does the new folder get added in first position, pushing the other folders down? If so, try changing how you call the API to add the folder. For instance, append it.

[michaeljones]

Thank you @gjsjohnmurray. Kind of you to take the time.

I have just found the docs on it and realise that it is documented: https://code.visualstudio.com/api/references/vscode-api#workspace.updateWorkspaceFolders - we'll change our approach accordingly. Though I guess it might still cause issues for the first folder added.

[juancampa]

@bpasero Perhaps this can be implemented risk-free by using an opt-in boolean:

canHandleNotHavingARootPathWhenInWorkspace: true

(obviously different name).

[g-arjones]

@roblourens Could you please clarify why this is not considered important anymore?