esm - remove graceful-fs (#214164)

* esm - remove `graceful-fs`

This module was used to patch `fs`, which is not supported in ESM. Besides, it makes us behave different from standard node.js that has meanwhile evolved from the time where `graceful-fs` was created.

* update comment

* update comment

* use more `fs.promises`

* 💄

.eslintrc.json

@@ -645,7 +645,6 @@
 							"events",
 							"fs",
 							"fs/promises",
-							"graceful-fs",
 							"http",
 							"https",
 							"minimist",

package.json

@@ -89,7 +89,6 @@
     "@xterm/addon-webgl": "0.19.0-beta.19",
     "@xterm/headless": "5.6.0-beta.19",
     "@xterm/xterm": "5.6.0-beta.19",
-    "graceful-fs": "4.2.11",
     "http-proxy-agent": "^7.0.0",
     "https-proxy-agent": "^7.0.2",
     "jschardet": "3.1.2",
@@ -112,7 +111,6 @@
     "@swc/core": "1.3.62",
     "@types/cookie": "^0.3.3",
     "@types/debug": "^4.1.5",
-    "@types/graceful-fs": "4.1.2",
     "@types/gulp-svgmin": "^1.2.1",
     "@types/http-proxy-agent": "^2.0.1",
     "@types/kerberos": "^1.1.2",

remote/package.json

@@ -22,7 +22,6 @@
     "@xterm/headless": "5.6.0-beta.19",
     "@xterm/xterm": "5.6.0-beta.19",
     "cookie": "^0.4.0",
-    "graceful-fs": "4.2.11",
     "http-proxy-agent": "^7.0.0",
     "https-proxy-agent": "^7.0.2",
     "jschardet": "3.1.2",

remote/yarn.lock

@@ -289,7 +289,7 @@ github-from-package@0.0.0:
   resolved "https://registry.yarnpkg.com/github-from-package/-/github-from-package-0.0.0.tgz#97fb5d96bfde8973313f20e8288ef9a167fa64ce"
   integrity sha1-l/tdlr/eiXMxPyDoKI75oWf6ZM4=
 
-graceful-fs@4.2.11, graceful-fs@^4.1.6, graceful-fs@^4.2.0:
+graceful-fs@^4.1.6, graceful-fs@^4.2.0:
   version "4.2.11"
   resolved "https://registry.yarnpkg.com/graceful-fs/-/graceful-fs-4.2.11.tgz#4183e4e8bf08bb6e05bbb2f7d2e0c8f712ca40e3"
   integrity sha512-RbJ5/jmFcNNCcDV5o9eTnBLJ/HszWV0P73bc+Ff4nS/rJj+YaS6IGyiOL0VoBYX+l1Wrl3k63h/KrH+nhJ0XvQ==

src/vs/base/node/pfs.ts

@@ -60,14 +60,6 @@ async function rimraf(path: string, mode = RimRafMode.UNLINK, moveToPath?: strin
 async function rimrafMove(path: string, moveToPath = randomPath(tmpdir())): Promise<void> {
 	try {
 		try {
-			// Intentionally using `fs.promises` here to skip
-			// the patched graceful-fs method that can result
-			// in very long running `rename` calls when the
-			// folder is locked by a file watcher. We do not
-			// really want to slow down this operation more
-			// than necessary and we have a fallback to delete
-			// via unlink.
-			// https://github.com/microsoft/vscode/issues/139908
 			await fs.promises.rename(path, moveToPath);
 		} catch (error) {
 			if (error.code === 'ENOENT') {
@@ -87,7 +79,7 @@ async function rimrafMove(path: string, moveToPath = randomPath(tmpdir())): Prom
 }
 
 async function rimrafUnlink(path: string): Promise<void> {
-	return promisify(fs.rm)(path, { recursive: true, force: true, maxRetries: 3 });
+	return fs.promises.rm(path, { recursive: true, force: true, maxRetries: 3 });
 }
 
 export function rimrafSync(path: string): void {
@@ -118,12 +110,12 @@ export interface IDirent {
 async function readdir(path: string): Promise<string[]>;
 async function readdir(path: string, options: { withFileTypes: true }): Promise<IDirent[]>;
 async function readdir(path: string, options?: { withFileTypes: true }): Promise<(string | IDirent)[]> {
-	return handleDirectoryChildren(await (options ? safeReaddirWithFileTypes(path) : promisify(fs.readdir)(path)));
+	return handleDirectoryChildren(await (options ? safeReaddirWithFileTypes(path) : fs.promises.readdir(path)));
 }
 
 async function safeReaddirWithFileTypes(path: string): Promise<IDirent[]> {
 	try {
-		return await promisify(fs.readdir)(path, { withFileTypes: true });
+		return await fs.promises.readdir(path, { withFileTypes: true });
 	} catch (error) {
 		console.warn('[node.js fs] readdir with filetypes failed with error: ', error);
 	}
@@ -493,20 +485,18 @@ function ensureWriteOptions(options?: IWriteFileOptions): IEnsuredWriteFileOptio
  * - allows to move across multiple disks
  * - attempts to retry the operation for certain error codes on Windows
  */
-async function rename(source: string, target: string, windowsRetryTimeout: number | false = 60000 /* matches graceful-fs */): Promise<void> {
+async function rename(source: string, target: string, windowsRetryTimeout: number | false = 60000): Promise<void> {
 	if (source === target) {
 		return;  // simulate node.js behaviour here and do a no-op if paths match
 	}
 
 	try {
 		if (isWindows && typeof windowsRetryTimeout === 'number') {
 			// On Windows, a rename can fail when either source or target
-			// is locked by AV software. We do leverage graceful-fs to iron
-			// out these issues, however in case the target file exists,
-			// graceful-fs will immediately return without retry for fs.rename().
+			// is locked by AV software.
 			await renameWithRetry(source, target, Date.now(), windowsRetryTimeout);
 		} else {
-			await promisify(fs.rename)(source, target);
+			await fs.promises.rename(source, target);
 		}
 	} catch (error) {
 		// In two cases we fallback to classic copy and delete:
@@ -528,7 +518,7 @@ async function rename(source: string, target: string, windowsRetryTimeout: numbe
 
 async function renameWithRetry(source: string, target: string, startTime: number, retryTimeout: number, attempt = 0): Promise<void> {
 	try {
-		return await promisify(fs.rename)(source, target);
+		return await fs.promises.rename(source, target);
 	} catch (error) {
 		if (error.code !== 'EACCES' && error.code !== 'EPERM' && error.code !== 'EBUSY') {
 			throw error; // only for errors we think are temporary
@@ -670,30 +660,27 @@ async function doCopySymlink(source: string, target: string, payload: ICopyPaylo
 //#region Promise based fs methods
 
 /**
- * Prefer this helper class over the `fs.promises` API to
- * enable `graceful-fs` to function properly. Given issue
- * https://github.com/isaacs/node-graceful-fs/issues/160 it
- * is evident that the module only takes care of the non-promise
- * based fs methods.
+ * Provides promise based 'fs' methods by wrapping around the
+ * original callback based methods.
  *
- * Another reason is `realpath` being entirely different in
- * the promise based implementation compared to the other
- * one (https://github.com/microsoft/vscode/issues/118562)
+ * At least `realpath` is implemented differently in the promise
+ * based implementation compared to the callback based one. The
+ * promise based implementation actually calls `fs.realpath.native`.
+ * (https://github.com/microsoft/vscode/issues/118562)
  *
- * Note: using getters for a reason, since `graceful-fs`
- * patching might kick in later after modules have been
- * loaded we need to defer access to fs methods.
- * (https://github.com/microsoft/vscode/issues/124176)
+ * TODO@bpasero we should move away from this towards `fs.promises`
+ * eventually and only keep those methods around where we explicitly
+ * want the callback based behaviour.
  */
 export const Promises = new class {
 
 	//#region Implemented by node.js
 
-	get access() { return promisify(fs.access); }
+	get access() { return fs.promises.access; }
 
-	get stat() { return promisify(fs.stat); }
-	get lstat() { return promisify(fs.lstat); }
-	get utimes() { return promisify(fs.utimes); }
+	get stat() { return fs.promises.stat; }
+	get lstat() { return fs.promises.lstat; }
+	get utimes() { return fs.promises.utimes; }
 
 	get read() {
 
@@ -713,7 +700,7 @@ export const Promises = new class {
 			});
 		};
 	}
-	get readFile() { return promisify(fs.readFile); }
+	get readFile() { return fs.promises.readFile; }
 
 	get write() {
 
@@ -734,27 +721,27 @@ export const Promises = new class {
 		};
 	}
 
-	get appendFile() { return promisify(fs.appendFile); }
+	get appendFile() { return fs.promises.appendFile; }
 
-	get fdatasync() { return promisify(fs.fdatasync); }
-	get truncate() { return promisify(fs.truncate); }
+	get fdatasync() { return promisify(fs.fdatasync); } // not exposed as API in 20.x yet
+	get truncate() { return fs.promises.truncate; }
 
-	get copyFile() { return promisify(fs.copyFile); }
+	get copyFile() { return fs.promises.copyFile; }
 
-	get open() { return promisify(fs.open); }
-	get close() { return promisify(fs.close); }
+	get open() { return promisify(fs.open); } 			// changed to return `FileHandle` in promise API
+	get close() { return promisify(fs.close); } 		// not exposed as API due to the `FileHandle` return type of `open`
 
-	get symlink() { return promisify(fs.symlink); }
-	get readlink() { return promisify(fs.readlink); }
+	get symlink() { return fs.promises.symlink; }
+	get readlink() { return fs.promises.readlink; }
 
-	get chmod() { return promisify(fs.chmod); }
+	get chmod() { return fs.promises.chmod; }
 
-	get mkdir() { return promisify(fs.mkdir); }
+	get mkdir() { return fs.promises.mkdir; }
 
-	get unlink() { return promisify(fs.unlink); }
-	get rmdir() { return promisify(fs.rmdir); }
+	get unlink() { return fs.promises.unlink; }
+	get rmdir() { return fs.promises.rmdir; }
 
-	get realpath() { return promisify(fs.realpath); }
+	get realpath() { return promisify(fs.realpath); }	// `fs.promises.realpath` will use `fs.realpath.native` which we do not want
 
 	//#endregion
 

src/vs/platform/files/node/diskFileSystemProvider.ts

@@ -3,8 +3,7 @@
  *  Licensed under the MIT License. See License.txt in the project root for license information.
  *--------------------------------------------------------------------------------------------*/
 
-import * as fs from 'fs';
-import { gracefulify } from 'graceful-fs';
+import { Stats } from 'fs';
 import { Barrier, retry } from 'vs/base/common/async';
 import { ResourceMap } from 'vs/base/common/map';
 import { VSBuffer } from 'vs/base/common/buffer';
@@ -24,22 +23,9 @@ import { readFileIntoStream } from 'vs/platform/files/common/io';
 import { AbstractNonRecursiveWatcherClient, AbstractUniversalWatcherClient, ILogMessage } from 'vs/platform/files/common/watcher';
 import { ILogService } from 'vs/platform/log/common/log';
 import { AbstractDiskFileSystemProvider, IDiskFileSystemProviderOptions } from 'vs/platform/files/common/diskFileSystemProvider';
-import { toErrorMessage } from 'vs/base/common/errorMessage';
 import { UniversalWatcherClient } from 'vs/platform/files/node/watcher/watcherClient';
 import { NodeJSWatcherClient } from 'vs/platform/files/node/watcher/nodejs/nodejsClient';
 
-/**
- * Enable graceful-fs very early from here to have it enabled
- * in all contexts that leverage the disk file system provider.
- */
-(() => {
-	try {
-		gracefulify(fs);
-	} catch (error) {
-		console.error(`Error enabling graceful-fs: ${toErrorMessage(error)}`);
-	}
-})();
-
 export class DiskFileSystemProvider extends AbstractDiskFileSystemProvider implements
 	IFileSystemProviderWithFileReadWriteCapability,
 	IFileSystemProviderWithOpenReadWriteCloseCapability,
@@ -139,7 +125,7 @@ export class DiskFileSystemProvider extends AbstractDiskFileSystemProvider imple
 		}
 	}
 
-	private toType(entry: fs.Stats | IDirent, symbolicLink?: { dangling: boolean }): FileType {
+	private toType(entry: Stats | IDirent, symbolicLink?: { dangling: boolean }): FileType {
 
 		// Signal file type by checking for file / directory, except:
 		// - symbolic links pointing to nonexistent files are FileType.Unknown

yarn.lock

@@ -1145,13 +1145,6 @@
     "@types/minimatch" "*"
     "@types/node" "*"
 
-"@types/graceful-fs@4.1.2":
-  version "4.1.2"
-  resolved "https://registry.yarnpkg.com/@types/graceful-fs/-/graceful-fs-4.1.2.tgz#fbc9575dbcc6d1d91dd768d30c5fc0c19f6c50bd"
-  integrity sha512-epDhsJAVxJsWfeqpzEDFhLnhHMbHie/VMFY+2Hvt5p7FemeW5ELM+6gcVYL/ZsUwdu3zrWpDE3VUTddXW+EMYg==
-  dependencies:
-    "@types/node" "*"
-
 "@types/gulp-svgmin@^1.2.1":
   version "1.2.1"
   resolved "https://registry.yarnpkg.com/@types/gulp-svgmin/-/gulp-svgmin-1.2.1.tgz#e18f344ea09560554652406b37e1dc3253a6bda2"
@@ -5127,11 +5120,6 @@ got@^11.8.5:
     p-cancelable "^2.0.0"
     responselike "^2.0.0"
 
-graceful-fs@4.2.11, graceful-fs@^4.2.11:
-  version "4.2.11"
-  resolved "https://registry.yarnpkg.com/graceful-fs/-/graceful-fs-4.2.11.tgz#4183e4e8bf08bb6e05bbb2f7d2e0c8f712ca40e3"
-  integrity sha512-RbJ5/jmFcNNCcDV5o9eTnBLJ/HszWV0P73bc+Ff4nS/rJj+YaS6IGyiOL0VoBYX+l1Wrl3k63h/KrH+nhJ0XvQ==
-
 graceful-fs@^4.0.0, graceful-fs@^4.1.11, graceful-fs@^4.1.6, graceful-fs@^4.2.0:
   version "4.2.4"
   resolved "https://registry.yarnpkg.com/graceful-fs/-/graceful-fs-4.2.4.tgz#2256bde14d3632958c465ebc96dc467ca07a29fb"
@@ -5142,6 +5130,11 @@ graceful-fs@^4.1.2, graceful-fs@^4.2.4:
   resolved "https://registry.yarnpkg.com/graceful-fs/-/graceful-fs-4.2.6.tgz#ff040b2b0853b23c3d31027523706f1885d76bee"
   integrity sha512-nTnJ528pbqxYanhpDYsi4Rd8MAeaBA67+RZ10CM1m3bTAVFEDcd5AuA4a6W5YkGZ1iNXHzZz8T6TBKLeBuNriQ==
 
+graceful-fs@^4.2.11:
+  version "4.2.11"
+  resolved "https://registry.yarnpkg.com/graceful-fs/-/graceful-fs-4.2.11.tgz#4183e4e8bf08bb6e05bbb2f7d2e0c8f712ca40e3"
+  integrity sha512-RbJ5/jmFcNNCcDV5o9eTnBLJ/HszWV0P73bc+Ff4nS/rJj+YaS6IGyiOL0VoBYX+l1Wrl3k63h/KrH+nhJ0XvQ==
+
 graceful-fs@^4.2.9:
   version "4.2.10"
   resolved "https://registry.yarnpkg.com/graceful-fs/-/graceful-fs-4.2.10.tgz#147d3a006da4ca3ce14728c7aefc287c367d7a6c"