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

Explorer refresh does not show latest data of the file system provider #185312

Closed
yiliang114 opened this issue Jun 16, 2023 · 8 comments
Closed
Assignees
Labels
*dev-question VS Code Extension Development Question file-explorer Explorer widget issues info-needed Issue requires more information from poster

Comments

@yiliang114
Copy link
Contributor

Define Type

export interface FileSystemProvider {
        /**
         * This function is triggered when the user clicks the Explorer's refresh button. 
         * Generally used to trigger the re-acquisition of data. When this asynchronous function finishes, the fileSystemService will re-read data from the file tree
         *
         * @param uri The uri of the file or folder to be watched.
         * @param options Configures the watch.
         * @returns A disposable that tells the provider to stop watching the `uri`.
         */
        refresh<T = any>(uri: Uri, options: { readonly recursive: boolean; readonly excludes: readonly string[] }): Promise<T>;
}

Use

class MemFS implements vscode.FileSystemProvider, vscode.Disposable {
   //...
     async refresh() {
        // Re-request data Promise
        return await this.xxxService.getData();
      }
}

Why

image

When I click the refresh button, it will only trigger the readDirectory of FileSystemProvider and will not re-request the data of the file tree.

My understanding of FileSystemProvider is that it does not pay attention to the source of the data it uses and when the data is requested. It only pays attention to the implementation of its own readFile, watch and other functions. When the FileSystemService of vscode executes related functions, it will call provider corresponding functions. This is true for most scenarios, but for refresh operations (especially for Web scenarios), just re-reading the folder (without re-requesting the data) after the refresh operation has no effect. Therefore, it is best to provide a refresh function that allows the provider to re-perform its own fetching operation, wait for the end, then let the file system re-read the data of the file tree, and then re-render the file tree.

vscode.dev is not a good example because it installs the git extension by default and has Sync button to get the latest data. But if git is not installed, I think refresh is quite useful.
image

@yiliang114
Copy link
Contributor Author

hi @bpasero ask you for some time, Do you think there is such a need?

@bpasero
Copy link
Member

bpasero commented Jun 16, 2023

Everything should be there in the fs-provider to response to the explorer refreshing. I see no need to add a new method. The explorer should use the existing methods we have: read directories and stat their entries.

@bpasero bpasero closed this as completed Jun 16, 2023
@yiliang114
Copy link
Contributor Author

Everything should be there in the fs-provider to response to the explorer refreshing. I see no need to add a new method. The explorer should use the existing methods we have: read directories and stat their entries.

In some cases, the data of the explorer file tree comes from an asynchronous API, the data structure of the tree is usually constructed when the fs-provider is initialized and provided to read directories and stat their entries.

if the back-end data is updated, unless I refresh the entire page, I won't get the latest data if I just click the explorer refresh button.

@bpasero bpasero reopened this Jun 16, 2023
@bpasero
Copy link
Member

bpasero commented Jun 16, 2023

Lets move this to explorer component, maybe something there is not working properly when you refresh.

Do you have a test extension for us to play with?

@bpasero bpasero changed the title Provide vscode.FileSystemProvider with a function named refresh to call when the user click Refresh Explorer Explorer refresh does not show latest data of the file system provider Jun 16, 2023
@bpasero bpasero assigned lramos15 and unassigned bpasero Jun 16, 2023
@bpasero bpasero added info-needed Issue requires more information from poster file-explorer Explorer widget issues labels Jun 16, 2023
@yiliang114
Copy link
Contributor Author

The following is a reproduced repo. I modified it after fork vscode-web-playground.

The main change is to change the data source of the file tree.

https://github.com/yiliang114/vscode-web-playground-reproduction/blob/9e64a1d098d6c0a9cf83fbea4642de1e5e45cd6b/src/memfs.ts#L110

Next are the steps to reproduce the problem I described:

  1. git clone https://github.com/yiliang114/vscode-web-playground-reproduction.git
  2. cd vscode-web-playground-reproduction and yarn install
  3. yarn watch to compile extension and yarn run run-in-browser to start a browser to develop the extension
  4. Open network to find a request named treeData.json, which is the source of file tree.
  5. Then click Refresh Explorer button once or more times, there will not to be any request. That's what I mean the Refresh Explorer appears to be invalid for file system like this.

image

@bpasero bpasero added the *dev-question VS Code Extension Development Question label Jun 21, 2023
@VSCodeTriageBot
Copy link
Collaborator

We have a great extension developer community over on GitHub discussions and Slack where extension authors help each other. This is a great place for you to ask questions and find support.

Happy Coding!

@VSCodeTriageBot VSCodeTriageBot closed this as not planned Won't fix, can't repro, duplicate, stale Jun 21, 2023
@bpasero
Copy link
Member

bpasero commented Jun 21, 2023

This is not a problem in VS Code but your sources. You only call getTreeData once and cache the Promise it seems.

@yiliang114
Copy link
Contributor Author

This is not a problem in VS Code but your sources. You only call getTreeData once and cache the Promise it seems.

Yes, but shouldn't it be the right thing to cache the data requested once In an in-memory file system? And thank you @bpasero , I tried to ask others how to deal with this memory file system case.

@github-actions github-actions bot locked and limited conversation to collaborators Aug 5, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
*dev-question VS Code Extension Development Question file-explorer Explorer widget issues info-needed Issue requires more information from poster
Projects
None yet
Development

No branches or pull requests

5 participants