Skip to content
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

[Live Share Web] File Explorer doesn't auto-update on adding a file or folder #117606

Closed
jramsay opened this issue Feb 24, 2021 · 13 comments
Closed
Assignees

Comments

@jramsay
Copy link
Member

jramsay commented Feb 24, 2021

Version: 1.54.0-insider (also repros on Stable)
Commit: c47da72
Date: 2021-02-24T11:27:23.847Z
Browser: Mozilla/5.0 (Windows NT 10.0; Win64; x64) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/88.0.4324.182 Safari/537.36 Edg/88.0.705.74

This is an issue specific to VS Code Online so it only repros in Live Share Web and not in our desktop extension

Issue: when the Live Share host (on VS Code desktop) adds a file or folder, the guest on Live Share Web does not see it until they manually click the "Refresh Explorer" button. It worth noting the issue appears to be specific to add. File or folder rename and delete work correctly - the File Explorer updates automatically and no manual refresh is required.

Our Live Share Web extension has a WorkspaceFileSystemProvider that extends vscode.FileSystemProvider. I've debugged this and can see that when the host adds, deletes, or renames an item, Live Share's file service sends an event that fires onDidChangeFile.:

public readonly onDidChangeFile: vscode.Event<vscode.FileChangeEvent[]> = this.onFilesChangedEmitter.event;

image

I'm not seeing any of the FileSystemProvider methods get called after the add event fires (Stat, readFile, readDirectory, etc)

I'm unable to debug VS Code minified as for some reason Pretty Print workbench.web.api.js doesn't work anymore on Chrome or Edge (known issue?). I've taken a look at the callstack for Rename events that do refresh correctly and I can see our FileSystemProvider's Stat() gets called from this call stack - so I'm presuming it is going wrong somewhere here for the add event:

image

Steps to Reproduce:

  1. Host on VS Code Desktop: Sign in and start a Live Share session

  2. Guest: Copy the session link directly to Chrome/Edge Chromium browser to join this session with MSA/GitHub accounts.

  3. Host: Add a file or folder

Expected:

The file tree should be refreshed automatically on the guest side

Actual:

The file tree will update only when refresh it manually

Does this issue occur when all extensions are disabled?: No

@jramsay jramsay changed the title [Live Share Web] File Explorer view doesn't auto-update on adding a file or folder [Live Share Web] File Explorer doesn't auto-update on adding a file or folder Feb 24, 2021
@isidorn
Copy link
Contributor

isidorn commented Feb 24, 2021

@jramsay hi! Ok, so the Explorer tree will listen on Root changes and should update on both sides.
What seems to be missing on the Guest side is for that root change event to come, otherwise the tree would update.
So it seems like the contextService does not fire a onDidChangeWorkspaceFolders on which we react here.

@jramsay Are you updating the worksspace configuration file on the guest side? Becuase if you did the Explorer would react on that I believe.

@sandy081 when does the contextService fire this event apart from when the workspace configuration is changed?

@jramsay
Copy link
Member Author

jramsay commented Feb 24, 2021

@isidorn: thanks for the info! It might be worth noting that if I comment out the line where we trigger FileSystemProvider.onDidChangeFile on the Guest side for the delete event it stops refreshing automatically for that too.

We have a memory workspace file. The content is initially set to this:

{
'folders': [
{
'uri': vsls:/?${workspaceId},
'name': workspaceName
}
]
}

We update the contents on writeFile as follows:

public async writeFile(uri: vscode.Uri, content: Uint8Array, options: { create: boolean, overwrite: boolean }): Promise {
if (uri.path === this.workspaceFile?.uri.path) {
this.setWorkspaceFileInfo(content);
return;
}

and return the memory workspace file on stat & readfile:

public async stat(uri: vscode.Uri): Promise<vscode.FileStat> {
if (uri.path === this.workspaceFile?.uri.path) {
return this.workspaceFile;
}

@sandy081
Copy link
Member

sandy081 commented Feb 25, 2021

@isidorn onDidChangeWorkspaceFolders event is triggered when ever there is a change in workspace folders which includes: name, addition, removal, and order changes.

@isidorn
Copy link
Contributor

isidorn commented Feb 25, 2021

@jramsay but do you send out a file change events when the memory workspace file is changed?
@sandy081 you should still listen to these changes even if the worksapce file is in memory and provided by the file system provider, right?

@sandy081
Copy link
Member

Yes, given that the underlying FSP provide change events like disk FSP. In short, I listen changes from FSP and react on them.

@jramsay
Copy link
Member Author

jramsay commented Feb 26, 2021

@isidorn - we weren't doing this. I tried it out though and it still doesn't work.

image

I was able to debug workbench.web.api.js and it isn't able to getWorkspaceFolder in the call to findClosest():

image

Should there be more items in the foldersMap or only the top root item?

@isidorn
Copy link
Contributor

isidorn commented Mar 1, 2021

@jramsay thanks for looking deeper. First of all I think you have to emit the file change event when your workspace configuration changes, so that would be a good change.
As for the foldersMap in the workspace.ts @sandy081 would know better, but I would think that it needs to have all root level folders (so in your case 2 folders). So this seems like the Workspace is not aware of the root folder you added
@sandy081 can you maybe give some hints to @jramsay on where an issue might be and where he can set some breakpoints to investigate further

@sandy081
Copy link
Member

sandy081 commented Mar 1, 2021

@jramsay May I know if you are trying to change first folder in the workspace? Can you please try adding second folder and check?

@jramsay
Copy link
Member Author

jramsay commented Mar 3, 2021

@sandy081: yes it is the first root (and only) folder in the memory .code-workspace.
image

So in the test I was adding a folder under /bin (which is already located under the root)

If I explicitly add a second folder in the .code-workspace called /bin then things work correctly:

image

image

but of course this isn't what we want. Does that help?

@sandy081
Copy link
Member

sandy081 commented Mar 5, 2021

If you try change the first folder in the workspace file, we restart extension host because of rootPath issue - #69335

Hence I think you might not be hitting the breakpoint in this case and things are working as expected.

@jramsay
Copy link
Member Author

jramsay commented Mar 5, 2021

@sandy081: hmm, not sure if that is related. I was able to work around the issue on the Live Share side by explicitly executing:

vscode.commands.executeCommand('workbench.files.action.refreshFilesExplorer');

when there is a FileChangeType.Created event.

But it feels like I shouldn't have to do that as it isn't required for Changed or Deleted events.

@sandy081
Copy link
Member

sandy081 commented Mar 8, 2021

@jramsay I am little lost here.. as I got chimed in in the middle.

I am sorry to ask, but can you please let me know what is the end-end scenario and what is not working so that I can understand what is broken?

@isidorn
Copy link
Contributor

isidorn commented Aug 13, 2021

Closing as no action is planned here

@isidorn isidorn closed this as completed Aug 13, 2021
@github-actions github-actions bot locked and limited conversation to collaborators Sep 27, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants