ONE-vscode icon indicating copy to clipboard operation
ONE-vscode copied to clipboard

Use filesystem via `vscode.workspace.fs` (not `fs`)

Open dayo09 opened this issue 2 years ago • 9 comments

What?

Let's use filesystem via vscode.worksapce.fs, not fs

Why?

They say that fs does not consider ssh support. It's safe to use vscode.workspace.fs instead of fs to support our extension on ssh on wheels.

Related Modules

Let's replace readFileSync, writeFileSync, etc functions inside these modules into vscode.workspace.fs's functions.

  • [ ] Circletracer
  • [ ] Jsontracer
  • [ ] CircleGraphCtrl
  • [ ] Circlereader
  • [ ] ConfigPanel
  • [ ] ConfigObject
  • [ ] OneExplore
  • [ ] Helpers

dayo09 avatar Jun 02 '22 06:06 dayo09

@dayo09

They say that fs does not consider ssh support. It's safe to use vscode.workspace.fs instead of fs to support our extension on ssh on wheels.

Could you provide reference URLs if any? Thanks!

hyunsik-yoon avatar Jun 03 '22 01:06 hyunsik-yoon

@hyunsik-yoon As you can see below, it's MS vscode's recommendation to use their api for filesystem approaches.

vscode/index.d.ts>vscode.workspace.fs


        /**
         * A {@link FileSystem file system} instance that allows to interact with local and remote
         * files, e.g. `vscode.workspace.fs.readDirectory(someUri)` allows to retrieve all entries
         * of a directory or `vscode.workspace.fs.stat(anotherUri)` returns the meta data for a
         * file.
         */
        export const fs: FileSystem;

From vscode api guide

Look out for usages of the fs node module for file system operations. If possible, use the vscode.workspace.fs API, which delegates to the appropriate file system provider.

From their release note

Call to Action: If your extension is currently using the fs module from Node.js, consider migrating to the new vscode.workspace.fs API

dayo09 avatar Jun 03 '22 04:06 dayo09

I've investigated as I don't know how different it is.

as-is (nodejs-api)

Buffer.from(): https://nodejs.org/dist/latest-v10.x/docs/api/buffer.html#buffer_class_method_buffer_from_string_encoding writeFile(): https://nodejs.org/dist/latest-v10.x/docs/api/fs.html#fs_fs_writefile_file_data_options_callback

  • file: <string> | <Buffer> | <URL> | <integer> filename or file descriptor
  • data: <string> | <Buffer> | <TypedArray> | <DataView>
const writeBytes = Buffer.from('Hello Node.js'); // Buffer.from(): nodejs api
const data = new Uint8Array(writeBytes);
fs.writeFile('message.txt', data, (err) => {
  if (err) throw err;
  console.log('The file has been saved!');
});

to-be (vscode-api)

fs: https://code.visualstudio.com/api/references/vscode-api#FileSystem uri: https://code.visualstudio.com/api/references/vscode-api#Uri

const good = URI.file('/coding/c#/project1');
good.scheme === 'file';
good.path === '/coding/c#/project1';
good.fragment === '';
const bad = URI.parse('file://' + '/coding/c#/project1');
bad.scheme === 'file';
bad.path === '/coding/c'; // path is now broken
bad.fragment === '/project1';

writeFile

  • uri: Uri. The uri of the file.
  • content: Uint8Array. The new content of the file.
const writeBytes = Buffer.from('Hello vscode'); // Buffer.from(): nodejs api
// someUri: Uri of vscode-api, used like `Uri.file('message.txt')`
// writeBytes: class Buffer -> Uint8Array
await vscode.workspace.fs.writeFile(someUri, writeBytes); // writeFile: fs of vscode-api

YongseopKim avatar Jun 09 '22 05:06 YongseopKim

vscode fs api doesn't take string as param. nodejs api does so it looks easier. It's good to support util classes to wrap vscode fs api.

YongseopKim avatar Jun 09 '22 05:06 YongseopKim

@YongseopKim Yes, if you use fs.promises.writeFile, then it's rather similar to vscode.workspace.fs.writeFile. The differences are

  1. fs.promises.writeFile returns promise while vscode.workspace.fs.writeFile returns Thenable.
  2. fs.promises.writeFile takes string while vscode.workspace.fs.writeFile takes Uint8Array, as you mentioned.

It looks like a good idea to provide a utility wrapping the Uint8Array conversion. :-D

dayo09 avatar Jun 09 '22 06:06 dayo09

Q. fs.promises.writeFile is a kind of api of vscode?

YongseopKim avatar Jun 09 '22 07:06 YongseopKim

@YongseopKim Nope, it's a basic fs module. They just support promises version of their functions.

dayo09 avatar Jun 29 '22 04:06 dayo09

NOTE: vscode.workspace.fs doesn't support synchronous functions.

https://github.com/microsoft/vscode/issues/84175#issuecomment-551445383

readFileSync() - currently using node's fs.readFileSync() Our design/architecture prohibits anything sync here. This will not happen.

It seems that they internally decided not to support sync functions for their apis, for whatever reason.

dayo09 avatar Jun 29 '22 04:06 dayo09

NOTE: vscode.workspace.fs is too slow

I tested the function latency.

As vscode.workspace.fs is asynchronouse function, I wrapped the statSync with promise.

  • vscode.workspace.fs: 1721 ms
  • Promise + fs.statSync: 9.92 ms
  console.time("1");

  console.timeLog("1", "Promise statSync start")
  const p = new Promise((resolve, reject)=>{
    fs.statSync("/home/dayo/git/ONE-vscode/res/modelDir/truediv/model.cfg")
    resolve("DONE")
  });
  p.then(()=>{
    // 9.928955078125 ms
    console.timeLog("1", "Promise statSync end")
  })


  console.time("2");
  console.timeLog("2", "vscode.workspace.fs start");

  vscode.workspace.fs
    .stat(
      vscode.Uri.file(
        "/home/dayo/git/ONE-vscode/res/modelDir/truediv/model.cfg"
      )
    )
    .then(
      (stat) => {
        // 1.721s
        console.timeLog("2", "vscode.workspace.fs end");
      },
      (error) => console.log(error)
    );

dayo09 avatar Jun 30 '23 02:06 dayo09