[node] fs: add `ref`/`unref` to `FSWatcher` by Semigradsky · Pull Request #68300 · DefinitelyTyped/DefinitelyTyped

Semigradsky · 2024-01-23T20:03:31Z

Closes #68296

typescript-bot · 2024-01-23T20:04:12Z

@Semigradsky Thank you for submitting this PR!

This is a live comment which I will keep updated.

1 package in this PR

node — on npm, on unpkg (author is owner)
- owner-approval: @peterblazejewicz

Code Reviews

Because this is a widely-used package, a DT maintainer will need to review it before it can be merged.

You can test the changes of this PR in the Playground.

Status

✅ No merge conflicts
✅ Continuous integration tests have passed
✅ Most recent commit is approved by a DT maintainer

All of the items on the list are green. To merge, you need to post a comment including the string "Ready to merge" to bring in your changes.

Diagnostic Information: What the bot saw about this PR

{
  "type": "info",
  "now": "-",
  "pr_number": 68300,
  "author": "Semigradsky",
  "headCommitOid": "b75c492892081e4f3e41da5ab72b4c60f6066e2e",
  "mergeBaseOid": "e9586c654a38c1720fb21f4da18195eb927ec36d",
  "lastPushDate": "2024-01-23T20:03:31.000Z",
  "lastActivityDate": "2024-01-24T05:40:28.000Z",
  "mergeOfferDate": "2024-01-23T21:47:07.000Z",
  "mergeRequestDate": "2024-01-24T05:40:28.000Z",
  "mergeRequestUser": "Semigradsky",
  "hasMergeConflict": false,
  "isFirstContribution": false,
  "tooManyFiles": false,
  "hugeChange": false,
  "popularityLevel": "Critical",
  "pkgInfo": [
    {
      "name": "node",
      "kind": "edit",
      "files": [
        {
          "path": "types/node/fs.d.ts",
          "kind": "definition"
        },
        {
          "path": "types/node/test/fs.ts",
          "kind": "test"
        },
        {
          "path": "types/node/ts4.8/fs.d.ts",
          "kind": "definition"
        },
        {
          "path": "types/node/ts4.8/test/fs.ts",
          "kind": "test"
        },
        {
          "path": "types/node/v16/fs.d.ts",
          "kind": "definition"
        },
        {
          "path": "types/node/v16/ts4.8/fs.d.ts",
          "kind": "definition"
        },
        {
          "path": "types/node/v18/fs.d.ts",
          "kind": "definition"
        },
        {
          "path": "types/node/v18/ts4.8/fs.d.ts",
          "kind": "definition"
        }
      ],
      "owners": [
        "Microsoft",
        "jkomyno",
        "alvis",
        "r3nya",
        "btoueg",
        "smac89",
        "touffy",
        "DeividasBakanas",
        "eyqs",
        "Hannes-Magnusson-CK",
        "hoo29",
        "kjin",
        "ajafff",
        "islishude",
        "mwiktorczyk",
        "mohsen1",
        "n-e",
        "galkin",
        "parambirs",
        "eps1lon",
        "ThomasdenH",
        "WilcoBakker",
        "wwwy3y3",
        "samuela",
        "kuehlein",
        "bhongy",
        "chyzwar",
        "trivikr",
        "yoursunny",
        "qwelias",
        "ExE-Boss",
        "peterblazejewicz",
        "addaleax",
        "victorperin",
        "ZYSzys",
        "NodeJS",
        "LinusU",
        "wafuwafu13",
        "mcollina",
        "Semigradsky"
      ],
      "addedOwners": [],
      "deletedOwners": [],
      "popularityLevel": "Critical"
    }
  ],
  "reviews": [
    {
      "type": "approved",
      "reviewer": "peterblazejewicz",
      "date": "2024-01-23T20:22:23.000Z",
      "isMaintainer": true
    }
  ],
  "mainBotCommentID": 1906834043,
  "ciResult": "pass"
}

typescript-bot · 2024-01-23T20:04:14Z

🔔 @microsoft @jkomyno @alvis @r3nya @btoueg @smac89 @Touffy @DeividasBakanas @eyqs @Hannes-Magnusson-CK @hoo29 @kjin @ajafff @islishude @mwiktorczyk @mohsen1 @n-e @galkin @parambirs @eps1lon @ThomasdenH @WilcoBakker @wwwy3y3 @samuela @kuehlein @bhongy @chyzwar @trivikr @yoursunny @qwelias @ExE-Boss @peterblazejewicz @addaleax @victorperin @ZYSzys @nodejs @LinusU @wafuwafu13 @mcollina — please review this PR in the next few days. Be sure to explicitly select Approve or Request Changes in the GitHub UI so I know what's going on.

peterblazejewicz

@Semigradsky 🙇🏻

Semigradsky · 2024-01-24T05:40:28Z

Ready to merge

josdejong · 2024-01-24T10:04:30Z

When updating from @types/[email protected] to @types/[email protected] in my project lossless-json that uses Vite, I get the following error:

node_modules/vite/dist/node/index.d.ts:68:15 - error TS2420: Class 'import(".../node_modules/vite/dist/node/index").FSWatcher' incorrectly implements interface 'import("fs").FSWatcher'.
  Type 'FSWatcher' is missing the following properties from type 'FSWatcher': ref, unref

68 declare class FSWatcher extends EventEmitter implements fs.FSWatcher {
                 ~~~~~~~~~


Found 1 error in node_modules/vite/dist/node/index.d.ts:68

Can it be that this PR introduces a breaking change in how it's being used by Vite? How to solve this?

Semigradsky · 2024-01-24T11:09:42Z

@josdejong please open an issue in vite repo and link this PR

devversion · 2024-01-24T11:28:03Z

FYI: Seeing the same with chokidar.

../../../../node_modules/chokidar/types/index.d.ts:9:14 - error TS2420: Class 'import("X/node_modules/chokidar/types/index").FSWatcher' 
  incorrectly implements interface 'import("fs").FSWatcher'.
  Type 'FSWatcher' is missing the following properties from type 'FSWatcher': ref, unref

9 export class FSWatcher extends EventEmitter implements fs.FSWatcher {

Semigradsky · 2024-01-24T11:30:00Z

The same

Currently the ZoneJS typing tests executes outside of Bazel, as a legacy artifact of the monorepo merging (as it seems - not ideal at all). Looks like this test relies on its own node modules, that were NOT locked using a yarn lock file. This commit adds one, and specifically locks it to a `@types/node` version that does not include the most recent patch release (which seemingly introduced a breaking change) that causes issues with TypeScript's lib checking. Whenever we perform lock file maintenance in the future, we have the following options: - Consider disabling lib checking via `skipLibCheck` for this test. This may be acceptable. - Continue locking the node version, - Waiting for chokidar to comply with the new signature - Waiting for the breaking change to be rolled back. Culprit change: DefinitelyTyped/DefinitelyTyped#68300

devversion · 2024-01-24T11:37:28Z

@Semigradsky For sure, but note that people are capturing this here for easier discoverability, and to provide context, so that a good issue can be created then (without it potentially being one-offs)

Currently the ZoneJS typing tests executes outside of Bazel, as a legacy artifact of the monorepo merging (as it seems - not ideal at all). Looks like this test relies on its own node modules, that were NOT locked using a yarn lock file. This commit adds one, and specifically locks it to a `@types/node` version that does not include the most recent patch release (which seemingly introduced a breaking change) that causes issues with TypeScript's lib checking. Whenever we perform lock file maintenance in the future, we have the following options: - Consider disabling lib checking via `skipLibCheck` for this test. This may be acceptable. - Continue locking the node version, - Waiting for chokidar to comply with the new signature - Waiting for the breaking change to be rolled back. Culprit change: DefinitelyTyped/DefinitelyTyped#68300 PR Close #54048

Currently the ZoneJS typing tests executes outside of Bazel, as a legacy artifact of the monorepo merging (as it seems - not ideal at all). Looks like this test relies on its own node modules, that were NOT locked using a yarn lock file. This commit adds one, and specifically locks it to a `@types/node` version that does not include the most recent patch release (which seemingly introduced a breaking change) that causes issues with TypeScript's lib checking. Whenever we perform lock file maintenance in the future, we have the following options: - Consider disabling lib checking via `skipLibCheck` for this test. This may be acceptable. - Continue locking the node version, - Waiting for chokidar to comply with the new signature - Waiting for the breaking change to be rolled back. Culprit change: DefinitelyTyped/DefinitelyTyped#68300 PR Close angular#54048

peterblazejewicz · 2024-01-25T20:25:06Z

Can it be that this PR introduces a breaking change in how it's being used [...]

yes, every change here can introduce a problem outside. We're (i.e. community) trying to limit possible unavoidable repercussions, and @Semigradsky et al. are really god at that! thanks!

Currently the ZoneJS typing tests executes outside of Bazel, as a legacy artifact of the monorepo merging (as it seems - not ideal at all). Looks like this test relies on its own node modules, that were NOT locked using a yarn lock file. This commit adds one, and specifically locks it to a `@types/node` version that does not include the most recent patch release (which seemingly introduced a breaking change) that causes issues with TypeScript's lib checking. Whenever we perform lock file maintenance in the future, we have the following options: - Consider disabling lib checking via `skipLibCheck` for this test. This may be acceptable. - Continue locking the node version, - Waiting for chokidar to comply with the new signature - Waiting for the breaking change to be rolled back. Culprit change: DefinitelyTyped/DefinitelyTyped#68300 PR Close angular#54048

[node] fs: add ref/unref to FSWatcher

b75c492

Semigradsky requested review from eps1lon and peterblazejewicz as code owners January 23, 2024 20:03

typescript-bot added Critical package Author is Owner The author of this PR is a listed owner of the package. labels Jan 23, 2024

peterblazejewicz approved these changes Jan 23, 2024

View reviewed changes

typescript-bot added Maintainer Approved Self Merge This PR can now be self-merged by the PR author or an owner labels Jan 23, 2024

typescript-bot merged commit 7e822d4 into DefinitelyTyped:master Jan 24, 2024

devversion mentioned this pull request Jan 24, 2024

build: ensure zone.js typing test is hermetic angular/angular#54048

Closed

devversion mentioned this pull request Jan 24, 2024

Incompatibility of types with recent changes in @types/node paulmillr/chokidar#1299

Closed

hi-ogawa mentioned this pull request Jan 25, 2024

fix: add ref() and unref() to chokidar.d.ts for typescript build to work. vitejs/vite#15706

Merged

9 tasks

bdbvb mentioned this pull request Jan 25, 2024

type error | Type 'FSWatcher' is missing the following properties from type 'FSWatcher': ref, unref vitejs/vite#15714

Closed

7 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Comments

[node] fs: add `ref`/`unref` to `FSWatcher`#68300

[node] fs: add `ref`/`unref` to `FSWatcher`#68300
typescript-bot merged 1 commit intoDefinitelyTyped:masterfrom
Semigradsky:node-fix-68296

Semigradsky commented Jan 23, 2024

Uh oh!

typescript-bot commented Jan 23, 2024 •

edited

Loading

Uh oh!

typescript-bot commented Jan 23, 2024

Uh oh!

peterblazejewicz left a comment

Uh oh!

Semigradsky commented Jan 24, 2024

Uh oh!

josdejong commented Jan 24, 2024

Uh oh!

Semigradsky commented Jan 24, 2024

Uh oh!

devversion commented Jan 24, 2024

Uh oh!

Semigradsky commented Jan 24, 2024

Uh oh!

devversion commented Jan 24, 2024

Uh oh!

peterblazejewicz commented Jan 25, 2024

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

Comments

Conversation

Semigradsky commented Jan 23, 2024

Uh oh!

typescript-bot commented Jan 23, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

1 package in this PR

Code Reviews

Status

Uh oh!

typescript-bot commented Jan 23, 2024

Uh oh!

peterblazejewicz left a comment

Choose a reason for hiding this comment

Uh oh!

Semigradsky commented Jan 24, 2024

Uh oh!

josdejong commented Jan 24, 2024

Uh oh!

Semigradsky commented Jan 24, 2024

Uh oh!

devversion commented Jan 24, 2024

Uh oh!

Semigradsky commented Jan 24, 2024

Uh oh!

devversion commented Jan 24, 2024

Uh oh!

peterblazejewicz commented Jan 25, 2024

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

typescript-bot commented Jan 23, 2024 •

edited

Loading