Skip to content

Commit

Permalink
Implements the readdir recursive flag (#5514)
Browse files Browse the repository at this point in the history
**What's the problem this PR addresses?**
The `ZipFS` implementation doesn't support the `recursive` flag.

Fixes part of #5423

**How did you fix it?**
I implemented the `recursive` flag and updated the types to account for
it. However I didn't make `ZipOpenFS` automatically traverse through zip
archives ... My line of thinking was:

> - on one hand, tools implementing their own readdir-based recursive
traversal don't currently dig into zip archives
> - on the other hand, it can be argued that the other recursive Node.js
native operations also apply to the zip archives' content (`cp` / `rm`)
>
> But I tend to think the first point is more important ... I feel like
the recursive flag should just be an optimization, so it should have the
same behaviour as a manual traversal ...

**Checklist**
<!--- Don't worry if you miss something, chores are automatically
tested. -->
<!--- This checklist exists to help you remember doing the chores when
you submit a PR. -->
<!--- Put an `x` in all the boxes that apply. -->
- [x] I have read the [Contributing
Guide](https://yarnpkg.com/advanced/contributing).

<!-- See
https://yarnpkg.com/advanced/contributing#preparing-your-pr-to-be-released
for more details. -->
<!-- Check with `yarn version check` and fix with `yarn version check
-i` -->
- [x] I have set the packages that need to be released for my changes to
be effective.

<!-- The "Testing chores" workflow validates that your PR follows our
guidelines. -->
<!-- If it doesn't pass, click on it to see details as to what your PR
might be missing. -->
- [x] I will check that all automated PR checks pass before the PR gets
reviewed.
  • Loading branch information
arcanis authored Jun 24, 2023
1 parent 2259237 commit 69ecb99
Show file tree
Hide file tree
Showing 13 changed files with 394 additions and 119 deletions.
39 changes: 39 additions & 0 deletions .yarn/versions/cc34eb37.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1,39 @@
releases:
"@yarnpkg/cli": minor
"@yarnpkg/core": minor
"@yarnpkg/fslib": minor
"@yarnpkg/libzip": minor
"@yarnpkg/plugin-pnpm": minor
"@yarnpkg/pnpify": minor

declined:
- "@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-stage"
- "@yarnpkg/plugin-typescript"
- "@yarnpkg/plugin-version"
- "@yarnpkg/plugin-workspace-tools"
- vscode-zipfs
- "@yarnpkg/builder"
- "@yarnpkg/doctor"
- "@yarnpkg/extensions"
- "@yarnpkg/nm"
- "@yarnpkg/pnp"
- "@yarnpkg/sdks"
- "@yarnpkg/shell"
8 changes: 4 additions & 4 deletions packages/plugin-pnpm/sources/PnpmLinker.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import {Descriptor, FetchResult, formatUtils, Installer, InstallPackageExtraApi, Linker, LinkOptions, LinkType, Locator, LocatorHash, Manifest, MessageName, MinimalLinkOptions, Package, Project, miscUtils, structUtils, WindowsLinkType} from '@yarnpkg/core';
import {Dirent, Filename, PortablePath, setupCopyIndex, ppath, xfs} from '@yarnpkg/fslib';
import {Filename, PortablePath, setupCopyIndex, ppath, xfs, DirentNoPath} from '@yarnpkg/fslib';
import {jsInstallUtils} from '@yarnpkg/plugin-pnp';
import {UsageError} from 'clipanion';

Expand Down Expand Up @@ -339,9 +339,9 @@ function isPnpmVirtualCompatible(locator: Locator, {project}: {project: Project}
}

async function getNodeModulesListing(nmPath: PortablePath) {
const listing = new Map<PortablePath, Dirent>();
const listing = new Map<PortablePath, DirentNoPath>();

let fsListing: Array<Dirent> = [];
let fsListing: Array<DirentNoPath> = [];
try {
fsListing = await xfs.readdirPromise(nmPath, {withFileTypes: true});
} catch (err) {
Expand Down Expand Up @@ -377,7 +377,7 @@ async function getNodeModulesListing(nmPath: PortablePath) {
return listing;
}

async function cleanNodeModules(nmPath: PortablePath, extraneous: Map<PortablePath, Dirent>) {
async function cleanNodeModules(nmPath: PortablePath, extraneous: Map<PortablePath, DirentNoPath>) {
const removeNamePromises = [];
const scopesToRemove = new Set<Filename>();

Expand Down
53 changes: 39 additions & 14 deletions packages/yarnpkg-fslib/sources/FakeFS.ts
Original file line number Diff line number Diff line change
Expand Up @@ -18,28 +18,39 @@ export type BigIntStats = NodeBigIntStats & {
crc?: number;
};

export type Dirent = Exclude<NodeDirent, 'name'> & {
export type Dirent<T extends Path> = Exclude<NodeDirent, 'name'> & {
name: Filename;
path: T;
};

export type DirentNoPath = Exclude<NodeDirent, 'name'> & {
name: Filename;
};

export type Dir<P extends Path> = {
readonly path: P;

[Symbol.asyncIterator](): AsyncIterableIterator<Dirent>;
[Symbol.asyncIterator](): AsyncIterableIterator<DirentNoPath>;

close(): Promise<void>;
close(cb: NoParamCallback): void;

closeSync(): void;

read(): Promise<Dirent | null>;
read(cb: (err: NodeJS.ErrnoException | null, dirent: Dirent | null) => void): void;
read(): Promise<DirentNoPath | null>;
read(cb: (err: NodeJS.ErrnoException | null, dirent: DirentNoPath | null) => void): void;

readSync(): Dirent | null;
readSync(): DirentNoPath | null;
};

export type OpendirOptions = Partial<{
bufferSize: number;
recursive: boolean;
}>;

export type ReaddirOptions = Partial<{
recursive: boolean;
withFileTypes: boolean;
}>;

export type CreateReadStreamOptions = Partial<{
Expand Down Expand Up @@ -161,15 +172,29 @@ export abstract class FakeFS<P extends Path> {
abstract realpathPromise(p: P): Promise<P>;
abstract realpathSync(p: P): P;

abstract readdirPromise(p: P): Promise<Array<Filename>>;
abstract readdirPromise(p: P, opts: {withFileTypes: false} | null): Promise<Array<Filename>>;
abstract readdirPromise(p: P, opts: {withFileTypes: true}): Promise<Array<Dirent>>;
abstract readdirPromise(p: P, opts: {withFileTypes: boolean}): Promise<Array<Filename> | Array<Dirent>>;

abstract readdirSync(p: P): Array<Filename>;
abstract readdirSync(p: P, opts: {withFileTypes: false} | null): Array<Filename>;
abstract readdirSync(p: P, opts: {withFileTypes: true}): Array<Dirent>;
abstract readdirSync(p: P, opts: {withFileTypes: boolean}): Array<Filename> | Array<Dirent>;
abstract readdirPromise(p: P, opts?: null): Promise<Array<Filename>>;
abstract readdirPromise(p: P, opts: {recursive?: false, withFileTypes: true}): Promise<Array<DirentNoPath>>;
abstract readdirPromise(p: P, opts: {recursive?: false, withFileTypes?: false}): Promise<Array<Filename>>;
abstract readdirPromise(p: P, opts: {recursive?: false, withFileTypes: boolean}): Promise<Array<DirentNoPath | Filename>>;
abstract readdirPromise(p: P, opts: {recursive: true, withFileTypes: true}): Promise<Array<Dirent<P>>>;
abstract readdirPromise(p: P, opts: {recursive: true, withFileTypes?: false}): Promise<Array<P>>;
abstract readdirPromise(p: P, opts: {recursive: true, withFileTypes: boolean}): Promise<Array<Dirent<P> | P>>;
abstract readdirPromise(p: P, opts: {recursive: boolean, withFileTypes: true}): Promise<Array<Dirent<P> | DirentNoPath>>;
abstract readdirPromise(p: P, opts: {recursive: boolean, withFileTypes?: false}): Promise<Array<P>>;
abstract readdirPromise(p: P, opts: {recursive: boolean, withFileTypes: boolean}): Promise<Array<Dirent<P> | DirentNoPath | P>>;
abstract readdirPromise(p: P, opts?: ReaddirOptions | null): Promise<Array<Dirent<P> | DirentNoPath | P>>;

abstract readdirSync(p: P, opts?: null): Array<Filename>;
abstract readdirSync(p: P, opts: {recursive?: false, withFileTypes: true}): Array<DirentNoPath>;
abstract readdirSync(p: P, opts: {recursive?: false, withFileTypes?: false}): Array<Filename>;
abstract readdirSync(p: P, opts: {recursive?: false, withFileTypes: boolean}): Array<DirentNoPath | Filename>;
abstract readdirSync(p: P, opts: {recursive: true, withFileTypes: true}): Array<Dirent<P>>;
abstract readdirSync(p: P, opts: {recursive: true, withFileTypes?: false}): Array<P>;
abstract readdirSync(p: P, opts: {recursive: true, withFileTypes: boolean}): Array<Dirent<P> | P>;
abstract readdirSync(p: P, opts: {recursive: boolean, withFileTypes: true}): Array<Dirent<P> | DirentNoPath>;
abstract readdirSync(p: P, opts: {recursive: boolean, withFileTypes?: false}): Array<P>;
abstract readdirSync(p: P, opts: {recursive: boolean, withFileTypes: boolean}): Array<Dirent<P> | DirentNoPath | P>;
abstract readdirSync(p: P, opts?: ReaddirOptions | null): Array<Dirent<P> | DirentNoPath | P>;

abstract existsPromise(p: P): Promise<boolean>;
abstract existsSync(p: P): boolean;
Expand Down
34 changes: 23 additions & 11 deletions packages/yarnpkg-fslib/sources/MountFS.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
import {BigIntStats, constants, Stats} from 'fs';

import {WatchOptions, WatchCallback, Watcher, StatOptions, StatSyncOptions} from './FakeFS';
import {WatchOptions, WatchCallback, Watcher, StatOptions, StatSyncOptions, ReaddirOptions, DirentNoPath} from './FakeFS';
import {FakeFS, MkdirOptions, RmdirOptions, WriteFileOptions, OpendirOptions} from './FakeFS';
import {Dirent, SymlinkType} from './FakeFS';
import {CreateReadStreamOptions, CreateWriteStreamOptions, BasePortableFakeFS, ExtractHintOptions, WatchFileOptions, WatchFileCallback, StatWatcher} from './FakeFS';
Expand Down Expand Up @@ -806,11 +806,17 @@ export class MountFS<MountedFS extends MountableFS> extends BasePortableFakeFS {
});
}

async readdirPromise(p: PortablePath): Promise<Array<Filename>>;
async readdirPromise(p: PortablePath, opts: {withFileTypes: false} | null): Promise<Array<Filename>>;
async readdirPromise(p: PortablePath, opts: {withFileTypes: true}): Promise<Array<Dirent>>;
async readdirPromise(p: PortablePath, opts: {withFileTypes: boolean}): Promise<Array<Filename> | Array<Dirent>>;
async readdirPromise(p: PortablePath, opts?: {withFileTypes?: boolean} | null): Promise<Array<string> | Array<Dirent>> {
async readdirPromise(p: PortablePath, opts?: null): Promise<Array<Filename>>;
async readdirPromise(p: PortablePath, opts: {recursive?: false, withFileTypes: true}): Promise<Array<DirentNoPath>>;
async readdirPromise(p: PortablePath, opts: {recursive?: false, withFileTypes?: false}): Promise<Array<Filename>>;
async readdirPromise(p: PortablePath, opts: {recursive?: false, withFileTypes: boolean}): Promise<Array<DirentNoPath | Filename>>;
async readdirPromise(p: PortablePath, opts: {recursive: true, withFileTypes: true}): Promise<Array<Dirent<PortablePath>>>;
async readdirPromise(p: PortablePath, opts: {recursive: true, withFileTypes?: false}): Promise<Array<PortablePath>>;
async readdirPromise(p: PortablePath, opts: {recursive: true, withFileTypes: boolean}): Promise<Array<Dirent<PortablePath> | PortablePath>>;
async readdirPromise(p: PortablePath, opts: {recursive: boolean, withFileTypes: true}): Promise<Array<Dirent<PortablePath> | DirentNoPath>>;
async readdirPromise(p: PortablePath, opts: {recursive: boolean, withFileTypes?: false}): Promise<Array<PortablePath>>;
async readdirPromise(p: PortablePath, opts: {recursive: boolean, withFileTypes: boolean}): Promise<Array<Dirent<PortablePath> | DirentNoPath | PortablePath>>;
async readdirPromise(p: PortablePath, opts?: ReaddirOptions | null): Promise<Array<Dirent<PortablePath> | DirentNoPath | PortablePath>> {
return await this.makeCallPromise(p, async () => {
return await this.baseFs.readdirPromise(p, opts as any);
}, async (mountFs, {subPath}) => {
Expand All @@ -820,11 +826,17 @@ export class MountFS<MountedFS extends MountableFS> extends BasePortableFakeFS {
});
}

readdirSync(p: PortablePath): Array<Filename>;
readdirSync(p: PortablePath, opts: {withFileTypes: false} | null): Array<Filename>;
readdirSync(p: PortablePath, opts: {withFileTypes: true}): Array<Dirent>;
readdirSync(p: PortablePath, opts: {withFileTypes: boolean}): Array<Filename> | Array<Dirent>;
readdirSync(p: PortablePath, opts?: {withFileTypes?: boolean} | null): Array<string> | Array<Dirent> {
readdirSync(p: PortablePath, opts?: null): Array<Filename>;
readdirSync(p: PortablePath, opts: {recursive?: false, withFileTypes: true}): Array<DirentNoPath>;
readdirSync(p: PortablePath, opts: {recursive?: false, withFileTypes?: false}): Array<Filename>;
readdirSync(p: PortablePath, opts: {recursive?: false, withFileTypes: boolean}): Array<DirentNoPath | Filename>;
readdirSync(p: PortablePath, opts: {recursive: true, withFileTypes: true}): Array<Dirent<PortablePath>>;
readdirSync(p: PortablePath, opts: {recursive: true, withFileTypes?: false}): Array<PortablePath>;
readdirSync(p: PortablePath, opts: {recursive: true, withFileTypes: boolean}): Array<Dirent<PortablePath> | PortablePath>;
readdirSync(p: PortablePath, opts: {recursive: boolean, withFileTypes: true}): Array<Dirent<PortablePath> | DirentNoPath>;
readdirSync(p: PortablePath, opts: {recursive: boolean, withFileTypes?: false}): Array<PortablePath>;
readdirSync(p: PortablePath, opts: {recursive: boolean, withFileTypes: boolean}): Array<Dirent<PortablePath> | DirentNoPath | PortablePath>;
readdirSync(p: PortablePath, opts?: ReaddirOptions | null): Array<Dirent<PortablePath> | DirentNoPath | PortablePath> {
return this.makeCallSync(p, () => {
return this.baseFs.readdirSync(p, opts as any);
}, (mountFs, {subPath}) => {
Expand Down
50 changes: 31 additions & 19 deletions packages/yarnpkg-fslib/sources/NodeFS.ts
Original file line number Diff line number Diff line change
@@ -1,10 +1,10 @@
import fs, {BigIntStats, Stats} from 'fs';
import fs, {BigIntStats, Stats} from 'fs';

import {CreateReadStreamOptions, CreateWriteStreamOptions, Dir, StatWatcher, WatchFileCallback, WatchFileOptions, OpendirOptions} from './FakeFS';
import {Dirent, SymlinkType, StatSyncOptions, StatOptions} from './FakeFS';
import {BasePortableFakeFS, WriteFileOptions} from './FakeFS';
import {MkdirOptions, RmdirOptions, WatchOptions, WatchCallback, Watcher} from './FakeFS';
import {FSPath, PortablePath, Filename, ppath, npath} from './path';
import {CreateReadStreamOptions, CreateWriteStreamOptions, Dir, StatWatcher, WatchFileCallback, WatchFileOptions, OpendirOptions, ReaddirOptions, DirentNoPath} from './FakeFS';
import {Dirent, SymlinkType, StatSyncOptions, StatOptions} from './FakeFS';
import {BasePortableFakeFS, WriteFileOptions} from './FakeFS';
import {MkdirOptions, RmdirOptions, WatchOptions, WatchCallback, Watcher} from './FakeFS';
import {FSPath, PortablePath, Filename, ppath, npath} from './path';

export class NodeFS extends BasePortableFakeFS {
private readonly realFs: typeof fs;
Expand Down Expand Up @@ -422,12 +422,18 @@ export class NodeFS extends BasePortableFakeFS {
return this.realFs.readFileSync(fsNativePath, encoding);
}

async readdirPromise(p: PortablePath): Promise<Array<Filename>>;
async readdirPromise(p: PortablePath, opts: {withFileTypes: false} | null): Promise<Array<Filename>>;
async readdirPromise(p: PortablePath, opts: {withFileTypes: true}): Promise<Array<Dirent>>;
async readdirPromise(p: PortablePath, opts: {withFileTypes: boolean}): Promise<Array<Filename> | Array<Dirent>>;
async readdirPromise(p: PortablePath, opts?: {withFileTypes?: boolean} | null): Promise<Array<string> | Array<Dirent>> {
return await new Promise<Array<Filename> | Array<Dirent>>((resolve, reject) => {
async readdirPromise(p: PortablePath, opts?: null): Promise<Array<Filename>>;
async readdirPromise(p: PortablePath, opts: {recursive?: false, withFileTypes: true}): Promise<Array<DirentNoPath>>;
async readdirPromise(p: PortablePath, opts: {recursive?: false, withFileTypes?: false}): Promise<Array<Filename>>;
async readdirPromise(p: PortablePath, opts: {recursive?: false, withFileTypes: boolean}): Promise<Array<DirentNoPath | Filename>>;
async readdirPromise(p: PortablePath, opts: {recursive: true, withFileTypes: true}): Promise<Array<Dirent<PortablePath>>>;
async readdirPromise(p: PortablePath, opts: {recursive: true, withFileTypes?: false}): Promise<Array<PortablePath>>;
async readdirPromise(p: PortablePath, opts: {recursive: true, withFileTypes: boolean}): Promise<Array<Dirent<PortablePath> | PortablePath>>;
async readdirPromise(p: PortablePath, opts: {recursive: boolean, withFileTypes: true}): Promise<Array<Dirent<PortablePath> | DirentNoPath>>;
async readdirPromise(p: PortablePath, opts: {recursive: boolean, withFileTypes?: false}): Promise<Array<PortablePath>>;
async readdirPromise(p: PortablePath, opts: {recursive: boolean, withFileTypes: boolean}): Promise<Array<Dirent<PortablePath> | DirentNoPath | PortablePath>>;
async readdirPromise(p: PortablePath, opts?: ReaddirOptions | null): Promise<Array<Dirent<PortablePath> | DirentNoPath | PortablePath>> {
return await new Promise<any>((resolve, reject) => {
if (opts?.withFileTypes) {
this.realFs.readdir(npath.fromPortablePath(p), {withFileTypes: true}, this.makeCallback(resolve, reject) as any);
} else {
Expand All @@ -436,15 +442,21 @@ export class NodeFS extends BasePortableFakeFS {
});
}

readdirSync(p: PortablePath): Array<Filename>;
readdirSync(p: PortablePath, opts: {withFileTypes: false} | null): Array<Filename>;
readdirSync(p: PortablePath, opts: {withFileTypes: true}): Array<Dirent>;
readdirSync(p: PortablePath, opts: {withFileTypes: boolean}): Array<Filename> | Array<Dirent>;
readdirSync(p: PortablePath, opts?: {withFileTypes?: boolean} | null): Array<string> | Array<Dirent> {
readdirSync(p: PortablePath, opts?: null): Array<Filename>;
readdirSync(p: PortablePath, opts: {recursive?: false, withFileTypes: true}): Array<DirentNoPath>;
readdirSync(p: PortablePath, opts: {recursive?: false, withFileTypes?: false}): Array<Filename>;
readdirSync(p: PortablePath, opts: {recursive?: false, withFileTypes: boolean}): Array<DirentNoPath | Filename>;
readdirSync(p: PortablePath, opts: {recursive: true, withFileTypes: true}): Array<Dirent<PortablePath>>;
readdirSync(p: PortablePath, opts: {recursive: true, withFileTypes?: false}): Array<PortablePath>;
readdirSync(p: PortablePath, opts: {recursive: true, withFileTypes: boolean}): Array<Dirent<PortablePath> | PortablePath>;
readdirSync(p: PortablePath, opts: {recursive: boolean, withFileTypes: true}): Array<Dirent<PortablePath> | DirentNoPath>;
readdirSync(p: PortablePath, opts: {recursive: boolean, withFileTypes?: false}): Array<PortablePath>;
readdirSync(p: PortablePath, opts: {recursive: boolean, withFileTypes: boolean}): Array<Dirent<PortablePath> | DirentNoPath | PortablePath>;
readdirSync(p: PortablePath, opts?: ReaddirOptions | null): Array<Dirent<PortablePath> | DirentNoPath | PortablePath> {
if (opts?.withFileTypes) {
return this.realFs.readdirSync(npath.fromPortablePath(p), {withFileTypes: true} as any);
return this.realFs.readdirSync(npath.fromPortablePath(p), {withFileTypes: true} as any) as Array<any>;
} else {
return this.realFs.readdirSync(npath.fromPortablePath(p)) as Array<Filename>;
return this.realFs.readdirSync(npath.fromPortablePath(p)) as Array<any>;
}
}

Expand Down
Loading

0 comments on commit 69ecb99

Please sign in to comment.