fix(fslib): make recursive mkdir return the first created directory (#…

…4462)
yarnpkg · Jun 24, 2022 · 66e75d6 · 66e75d6
1 parent c301f1d
commit 66e75d6
Show file tree

Hide file tree

Showing 9 changed files with 101 additions and 21 deletions.
diff --git a/.pnp.cjs b/.pnp.cjs
diff --git a/.yarn/versions/4ef8937b.yml b/.yarn/versions/4ef8937b.yml
@@ -0,0 +1,40 @@
+releases:
+  "@yarnpkg/cli": patch
+  "@yarnpkg/core": patch
+  "@yarnpkg/fslib": patch
+  "@yarnpkg/pnp": patch
+
+declined:
+  - "@yarnpkg/esbuild-plugin-pnp"
+  - "@yarnpkg/plugin-compat"
+  - "@yarnpkg/plugin-constraints"
+  - "@yarnpkg/plugin-dlx"
+  - "@yarnpkg/plugin-essentials"
+  - "@yarnpkg/plugin-exec"
+  - "@yarnpkg/plugin-file"
+  - "@yarnpkg/plugin-git"
+  - "@yarnpkg/plugin-github"
+  - "@yarnpkg/plugin-http"
+  - "@yarnpkg/plugin-init"
+  - "@yarnpkg/plugin-interactive-tools"
+  - "@yarnpkg/plugin-link"
+  - "@yarnpkg/plugin-nm"
+  - "@yarnpkg/plugin-npm"
+  - "@yarnpkg/plugin-npm-cli"
+  - "@yarnpkg/plugin-pack"
+  - "@yarnpkg/plugin-patch"
+  - "@yarnpkg/plugin-pnp"
+  - "@yarnpkg/plugin-pnpm"
+  - "@yarnpkg/plugin-stage"
+  - "@yarnpkg/plugin-typescript"
+  - "@yarnpkg/plugin-version"
+  - "@yarnpkg/plugin-workspace-tools"
+  - vscode-zipfs
+  - "@yarnpkg/builder"
+  - "@yarnpkg/doctor"
+  - "@yarnpkg/extensions"
+  - "@yarnpkg/json-proxy"
+  - "@yarnpkg/nm"
+  - "@yarnpkg/pnpify"
+  - "@yarnpkg/sdks"
+  - "@yarnpkg/shell"
diff --git a/packages/acceptance-tests/pkg-tests-core/sources/utils/fs.ts b/packages/acceptance-tests/pkg-tests-core/sources/utils/fs.ts
@@ -162,7 +162,7 @@ export const createTemporaryFile = async (filePath: PortablePath = `file` as Por
   return ppath.resolve(folderPath, filePath as PortablePath);
 };
 
-export const mkdirp = async (target: PortablePath): Promise<void> =>
+export const mkdirp = async (target: PortablePath): Promise<string | undefined> =>
   await xfs.mkdirpPromise(target);
 
 export const writeFile = async (target: PortablePath, body: string | Buffer): Promise<void> => {

diff --git a/packages/yarnpkg-core/sources/worker-zip/index.js b/packages/yarnpkg-core/sources/worker-zip/index.js
diff --git a/packages/yarnpkg-fslib/sources/FakeFS.ts b/packages/yarnpkg-fslib/sources/FakeFS.ts
@@ -193,8 +193,8 @@ export abstract class FakeFS<P extends Path> {
   abstract chownPromise(p: P, uid: number, gid: number): Promise<void>;
   abstract chownSync(p: P, uid: number, gid: number): void;
 
-  abstract mkdirPromise(p: P, opts?: MkdirOptions): Promise<void>;
-  abstract mkdirSync(p: P, opts?: MkdirOptions): void;
+  abstract mkdirPromise(p: P, opts?: MkdirOptions): Promise<string | undefined>;
+  abstract mkdirSync(p: P, opts?: MkdirOptions): string | undefined;
 
   abstract rmdirPromise(p: P, opts?: RmdirOptions): Promise<void>;
   abstract rmdirSync(p: P, opts?: RmdirOptions): void;
@@ -333,13 +333,15 @@ export abstract class FakeFS<P extends Path> {
     }
   }
 
-  async mkdirpPromise(p: P, {chmod, utimes}: {chmod?: number, utimes?: [Date | string | number, Date | string | number]} = {}) {
+  async mkdirpPromise(p: P, {chmod, utimes}: {chmod?: number, utimes?: [Date | string | number, Date | string | number]} = {}): Promise<string | undefined> {
     p = this.resolve(p);
     if (p === this.pathUtils.dirname(p))
-      return;
+      return undefined;
 
     const parts = p.split(this.pathUtils.sep);
 
+    let createdDirectory: P | undefined;
+
     for (let u = 2; u <= parts.length; ++u) {
       const subPath = parts.slice(0, u).join(this.pathUtils.sep) as P;
 
@@ -354,6 +356,8 @@ export abstract class FakeFS<P extends Path> {
           }
         }
 
+        createdDirectory ??= subPath;
+
         if (chmod != null)
           await this.chmodPromise(subPath, chmod);
 
@@ -365,15 +369,19 @@ export abstract class FakeFS<P extends Path> {
         }
       }
     }
+
+    return createdDirectory;
   }
 
-  mkdirpSync(p: P, {chmod, utimes}: {chmod?: number, utimes?: [Date | string | number, Date | string | number]} = {}) {
+  mkdirpSync(p: P, {chmod, utimes}: {chmod?: number, utimes?: [Date | string | number, Date | string | number]} = {}): string | undefined {
     p = this.resolve(p);
     if (p === this.pathUtils.dirname(p))
-      return;
+      return undefined;
 
     const parts = p.split(this.pathUtils.sep);
 
+    let createdDirectory: P | undefined;
+
     for (let u = 2; u <= parts.length; ++u) {
       const subPath = parts.slice(0, u).join(this.pathUtils.sep) as P;
 
@@ -388,6 +396,8 @@ export abstract class FakeFS<P extends Path> {
           }
         }
 
+        createdDirectory ??= subPath;
+
         if (chmod != null)
           this.chmodSync(subPath, chmod);
 
@@ -399,6 +409,8 @@ export abstract class FakeFS<P extends Path> {
         }
       }
     }
+
+    return createdDirectory;
   }
 
   copyPromise(destination: P, source: P, options?: {baseFs?: undefined, overwrite?: boolean, stableSort?: boolean, stableTime?: boolean, linkStrategy?: LinkStrategy}): Promise<void>;

diff --git a/packages/yarnpkg-fslib/sources/NodeFS.ts b/packages/yarnpkg-fslib/sources/NodeFS.ts
@@ -351,12 +351,14 @@ export class NodeFS extends BasePortableFakeFS {
   }
 
   async mkdirPromise(p: PortablePath, opts?: MkdirOptions) {
-    return await new Promise<void>((resolve, reject) => {
+    return await new Promise<string | undefined>((resolve, reject) => {
+      // @ts-expect-error - Types are outdated, the second argument in the callback is either a string or undefined
       this.realFs.mkdir(npath.fromPortablePath(p), opts, this.makeCallback(resolve, reject));
     });
   }
 
-  mkdirSync(p: PortablePath, opts?: MkdirOptions) {
+  mkdirSync(p: PortablePath, opts?: MkdirOptions): string | undefined {
+    // @ts-expect-error - Types are outdated, returns either a string or undefined
     return this.realFs.mkdirSync(npath.fromPortablePath(p), opts);
   }
 

diff --git a/packages/yarnpkg-fslib/sources/ZipFS.ts b/packages/yarnpkg-fslib/sources/ZipFS.ts
@@ -1240,10 +1240,8 @@ export class ZipFS extends BasePortableFakeFS {
   }
 
   mkdirSync(p: PortablePath, {mode = 0o755, recursive = false}: MkdirOptions = {}) {
-    if (recursive) {
-      this.mkdirpSync(p, {chmod: mode});
-      return;
-    }
+    if (recursive)
+      return this.mkdirpSync(p, {chmod: mode});
 
     if (this.readOnly)
       throw errors.EROFS(`mkdir '${p}'`);
@@ -1255,6 +1253,7 @@ export class ZipFS extends BasePortableFakeFS {
 
     this.hydrateDirectory(resolvedP);
     this.chmodSync(resolvedP, mode);
+    return undefined;
   }
 
   async rmdirPromise(p: PortablePath, opts?: RmdirOptions) {

diff --git a/packages/yarnpkg-fslib/tests/ZipFS.test.ts b/packages/yarnpkg-fslib/tests/ZipFS.test.ts
@@ -901,4 +901,26 @@ describe(`ZipFS`, () => {
 
     zipFs.discardAndClose();
   });
+
+  it(`should return the first created directory in mkdir recursive`, () => {
+    const zipFs = new ZipFS(null, {libzip: getLibzipSync()});
+
+    expect(zipFs.mkdirSync(`/foo` as PortablePath, {recursive: true})).toEqual(`/foo` as PortablePath);
+    expect(zipFs.mkdirSync(`/foo` as PortablePath, {recursive: true})).toEqual(undefined);
+    expect(zipFs.mkdirSync(`/foo/bar/baz` as PortablePath, {recursive: true})).toEqual(`/foo/bar` as PortablePath);
+    expect(zipFs.mkdirSync(`/foo/bar/baz` as PortablePath, {recursive: true})).toEqual(undefined);
+
+    zipFs.discardAndClose();
+  });
+
+  it(`should return the first created directory in mkdirp`, () => {
+    const zipFs = new ZipFS(null, {libzip: getLibzipSync()});
+
+    expect(zipFs.mkdirpSync(`/foo` as PortablePath)).toEqual(`/foo` as PortablePath);
+    expect(zipFs.mkdirpSync(`/foo` as PortablePath)).toEqual(undefined);
+    expect(zipFs.mkdirpSync(`/foo/bar/baz` as PortablePath)).toEqual(`/foo/bar` as PortablePath);
+    expect(zipFs.mkdirpSync(`/foo/bar/baz` as PortablePath)).toEqual(undefined);
+
+    zipFs.discardAndClose();
+  });
 });
diff --git a/packages/yarnpkg-pnp/sources/hook.js b/packages/yarnpkg-pnp/sources/hook.js