Skip to content

Commit

Permalink
Fixes types for @types/node 18.17 (#5731)
Browse files Browse the repository at this point in the history
**What's the problem this PR addresses?**

DefinitelyTyped/DefinitelyTyped#66405 added
support for the `path` property to `fs.Dirent`; this revealed a couple
of errors in our types.

Fixes #5728

**How did you fix it?**

- Replaces `Exclude<fs.Stats, 'name'>` by `Omit<fs.Stats, 'name'>`
- Replaces `fs.Stats` by `Stats` in the nm linker

**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 committed Sep 13, 2023
1 parent d5431bd commit ca6a3d6
Show file tree
Hide file tree
Showing 15 changed files with 210 additions and 87 deletions.
135 changes: 74 additions & 61 deletions .pnp.cjs

Large diffs are not rendered by default.

16 changes: 14 additions & 2 deletions .pnp.loader.mjs

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Binary file not shown.
39 changes: 39 additions & 0 deletions .yarn/versions/c9506c5f.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1,39 @@
releases:
"@yarnpkg/cli": patch
"@yarnpkg/core": patch
"@yarnpkg/fslib": patch
"@yarnpkg/pnp": patch
"@yarnpkg/pnpify": patch

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-pnpm"
- "@yarnpkg/plugin-stage"
- "@yarnpkg/plugin-typescript"
- "@yarnpkg/plugin-version"
- "@yarnpkg/plugin-workspace-tools"
- vscode-zipfs
- "@yarnpkg/builder"
- "@yarnpkg/doctor"
- "@yarnpkg/extensions"
- "@yarnpkg/libzip"
- "@yarnpkg/nm"
- "@yarnpkg/sdks"
- "@yarnpkg/shell"
2 changes: 1 addition & 1 deletion package.json
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@
"@babel/preset-typescript": "^7.18.6",
"@types/jest": "^28.1.6",
"@types/micromatch": "^4.0.1",
"@types/node": "^18.15.11",
"@types/node": "^18.17.15",
"@types/react": "^16.8.0",
"@types/semver": "^7.1.0",
"@yarnpkg/cli": "workspace:^",
Expand Down
2 changes: 1 addition & 1 deletion packages/yarnpkg-core/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,7 @@
"@types/diff": "^5.0.0",
"@types/lodash": "^4.14.136",
"@types/micromatch": "^4.0.1",
"@types/node": "^18.15.11",
"@types/node": "^18.17.15",
"@types/tar": "^4.0.4",
"@types/tunnel": "^0.0.0",
"@yarnpkg/cli": "workspace:^",
Expand Down
4 changes: 2 additions & 2 deletions packages/yarnpkg-fslib/sources/FakeFS.ts
Original file line number Diff line number Diff line change
Expand Up @@ -18,12 +18,12 @@ export type BigIntStats = NodeBigIntStats & {
crc?: number;
};

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

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

Expand Down
46 changes: 40 additions & 6 deletions packages/yarnpkg-fslib/sources/NodeFS.ts
Original file line number Diff line number Diff line change
Expand Up @@ -38,23 +38,57 @@ export class NodeFS extends BasePortableFakeFS {
}

async opendirPromise(p: PortablePath, opts?: OpendirOptions): Promise<Dir<PortablePath>> {
return await new Promise<Dir<PortablePath>>((resolve, reject) => {
return await new Promise<fs.Stats>((resolve, reject) => {
if (typeof opts !== `undefined`) {
this.realFs.opendir(npath.fromPortablePath(p), opts, this.makeCallback(resolve, reject) as any);
} else {
this.realFs.opendir(npath.fromPortablePath(p), this.makeCallback(resolve, reject) as any);
}
}).then(dir => {
return Object.defineProperty(dir, `path`, {value: p, configurable: true, writable: true});
// @ts-expect-error
//
// We need a way to tell TS that the values returned by the `read`
// methods are compatible with `Dir`, especially the `name` field.
//
// We also can't use `Object.assign` to set the because the `path`
// field to a Filename, because the property isn't writable, so
// we need to use defineProperty instead.
//
const dirWithFixedPath: Dir<PortablePath> = dir;

Object.defineProperty(dirWithFixedPath, `path`, {
value: p,
configurable: true,
writable: true,
});

return dirWithFixedPath;
});
}

opendirSync(p: PortablePath, opts?: OpendirOptions) {
const dir = typeof opts !== `undefined`
? this.realFs.opendirSync(npath.fromPortablePath(p), opts) as Dir<PortablePath>
: this.realFs.opendirSync(npath.fromPortablePath(p)) as Dir<PortablePath>;
const dir: Omit<fs.Dir, `path`> = typeof opts !== `undefined`
? this.realFs.opendirSync(npath.fromPortablePath(p), opts)
: this.realFs.opendirSync(npath.fromPortablePath(p));

// @ts-expect-error
//
// We need a way to tell TS that the values returned by the `read`
// methods are compatible with `Dir`, especially the `name` field.
//
// We also can't use `Object.assign` to set the because the `path`
// field to a Filename, because the property isn't writable, so
// we need to use defineProperty instead.
//
const dirWithFixedPath: Dir<PortablePath> = dir;

Object.defineProperty(dirWithFixedPath, `path`, {
value: p,
configurable: true,
writable: true,
});

return Object.defineProperty(dir, `path`, {value: p, configurable: true, writable: true});
return dirWithFixedPath;
}

async readPromise(fd: number, buffer: Buffer, offset: number = 0, length: number = 0, position: number | null = -1) {
Expand Down
4 changes: 3 additions & 1 deletion packages/yarnpkg-fslib/sources/algorithms/opendir.ts
Original file line number Diff line number Diff line change
Expand Up @@ -77,7 +77,9 @@ export function opendir<P extends Path>(fakeFs: FakeFS<P>, path: P, entries: Arr
if (typeof filename === `undefined`)
return null;

return Object.assign(fakeFs.statSync(fakeFs.pathUtils.join(path, filename)), {
const entryPath = fakeFs.pathUtils.join(path, filename);

return Object.assign(fakeFs.statSync(entryPath), {
name: filename,
path: undefined,
});
Expand Down
23 changes: 23 additions & 0 deletions packages/yarnpkg-fslib/tests/NodeFS.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,29 @@ const ifAtLeastNode20It = !process.version.match(/^v1[89]\./) ? it : it.skip;
const ifNotWin32It = process.platform !== `win32` ? it : it.skip;

describe(`NodeFS`, () => {
describe(`opendir`, () => {
it(`shouldn't crash`, async () => {
// The `path` property of fs.Dir only has a getter; if our implementation
// overrides it, it'll crash (we need to defineProperty it instead). This
// test makes sure we don't accidentally remove it.

const tmpdir = await xfs.mktempPromise();
await xfs.writeFilePromise(ppath.join(tmpdir, `foo`), ``);

const dir1 = xfs.opendirSync(tmpdir);
expect(dir1.path).toEqual(tmpdir);
expect(dir1.readSync()).toMatchObject({
name: `foo`,
});

const dir2 = await xfs.opendirPromise(tmpdir);
expect(dir2.path).toEqual(tmpdir);
await expect(dir2.read()).resolves.toMatchObject({
name: `foo`,
});
});
});

describe(`readdir`, () => {
ifAtLeastNode20It(`should support recursive directory listing`, async () => {
const tmpdir = await xfs.mktempPromise();
Expand Down
2 changes: 1 addition & 1 deletion packages/yarnpkg-pnp/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@
"./package.json": "./package.json"
},
"dependencies": {
"@types/node": "^18.15.11",
"@types/node": "^18.17.15",
"@yarnpkg/fslib": "workspace:^"
},
"devDependencies": {
Expand Down
2 changes: 1 addition & 1 deletion packages/yarnpkg-pnp/sources/esm-loader/built-loader.js

Large diffs are not rendered by default.

2 changes: 1 addition & 1 deletion packages/yarnpkg-pnp/sources/hook.js

Large diffs are not rendered by default.

6 changes: 3 additions & 3 deletions packages/yarnpkg-pnpify/sources/NodeModulesFS.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import {DirentNoPath, ReaddirOptions} from '@yarnpkg/fslib';
import {BigIntStats, DirentNoPath, ReaddirOptions, Stats} from '@yarnpkg/fslib';
import {Dirent, Filename, MkdirOptions, ExtractHintOptions, WatchFileCallback, WatchFileOptions, StatWatcher, OpendirOptions, Dir} from '@yarnpkg/fslib';
import {RmdirOptions} from '@yarnpkg/fslib';
import {FSPath, NativePath, PortablePath, npath, ppath, opendir} from '@yarnpkg/fslib';
Expand All @@ -8,7 +8,7 @@ import {CreateReadStreamOptions, CreateWriteStreamOptions}
import {NodeModulesTreeOptions, NodeModulesTree} from '@yarnpkg/nm';
import {buildNodeModulesTree} from '@yarnpkg/nm';
import {PnpApi} from '@yarnpkg/pnp';
import fs, {BigIntStats, Stats} from 'fs';
import fs from 'fs';

import {WatchManager} from './WatchManager';
import {dynamicRequireNoCache} from './dynamicRequire';
Expand Down Expand Up @@ -533,7 +533,7 @@ export class PortableNodeModulesFS extends FakeFS<PortablePath> {
name,
path: undefined,
});
}) ;
});
} else {
return await this.baseFs.readdirPromise(pnpPath.resolvedPath, opts as any);
}
Expand Down
14 changes: 7 additions & 7 deletions yarn.lock
Original file line number Diff line number Diff line change
Expand Up @@ -6025,10 +6025,10 @@ __metadata:
languageName: node
linkType: hard

"@types/node@npm:*, @types/node@npm:^18.15.11":
version: 18.16.18
resolution: "@types/node@npm:18.16.18"
checksum: 4692b4c927bf63efbc3aed6e18c1a264eb8b5673fa7cc0649a38f394ab34d4d4d0e7a2b98f4235bc8dc7615e88080a1b443a06eac9324f67e426398ed857b4a1
"@types/node@npm:*, @types/node@npm:^18.17.15":
version: 18.17.15
resolution: "@types/node@npm:18.17.15"
checksum: 5b28c68f44657399264efb1308cf87ddbf065299aa9e75abbd9f4d96785e5d7f1fdb0b263f3d7080b89036845e0d907c3edc51f75813c099a7ec311660188928
languageName: node
linkType: hard

Expand Down Expand Up @@ -7252,7 +7252,7 @@ __metadata:
"@types/diff": "npm:^5.0.0"
"@types/lodash": "npm:^4.14.136"
"@types/micromatch": "npm:^4.0.1"
"@types/node": "npm:^18.15.11"
"@types/node": "npm:^18.17.15"
"@types/semver": "npm:^7.1.0"
"@types/tar": "npm:^4.0.4"
"@types/treeify": "npm:^1.0.0"
Expand Down Expand Up @@ -7575,7 +7575,7 @@ __metadata:
"@mdx-js/react": "npm:^1.6.22"
"@types/jest": "npm:^28.1.6"
"@types/micromatch": "npm:^4.0.1"
"@types/node": "npm:^18.15.11"
"@types/node": "npm:^18.17.15"
"@types/react": "npm:^16.8.0"
"@types/semver": "npm:^7.1.0"
"@yarnpkg/cli": "workspace:^"
Expand Down Expand Up @@ -8085,7 +8085,7 @@ __metadata:
dependencies:
"@rollup/plugin-commonjs": "npm:^21.0.1"
"@rollup/plugin-node-resolve": "npm:^11.0.1"
"@types/node": "npm:^18.15.11"
"@types/node": "npm:^18.17.15"
"@types/semver": "npm:^7.1.0"
"@yarnpkg/fslib": "workspace:^"
"@yarnpkg/libzip": "workspace:^"
Expand Down

0 comments on commit ca6a3d6

Please sign in to comment.