@thw0rted 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
  • Config files to check:
    • node/tsconfig.json: not the expected form and not moving towards it (check: compilerOptions.target)

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
• ✅ A DT maintainer needs to approve changes which affect module config files

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.

{
  "type": "info",
  "now": "-",
  "pr_number": 59905,
  "author": "thw0rted",
  "headCommitOid": "542cacd0803f50d8537a94b2dfa3e3e125c837ac",
  "mergeBaseOid": "a6009a6e4434021ed0e67e7b74225a1190bf64c9",
  "lastPushDate": "2022-09-13T08:46:59.000Z",
  "lastActivityDate": "2022-10-03T22:01:28.000Z",
  "mergeOfferDate": "2022-10-03T21:17:54.000Z",
  "mergeRequestDate": "2022-10-03T22:01:28.000Z",
  "mergeRequestUser": "thw0rted",
  "hasMergeConflict": false,
  "isFirstContribution": false,
  "tooManyFiles": false,
  "hugeChange": false,
  "popularityLevel": "Critical",
  "pkgInfo": [
    {
      "name": "node",
      "kind": "edit",
      "files": [
        {
          "path": "types/node/buffer.d.ts",
          "kind": "definition"
        },
        {
          "path": "types/node/dom-events.d.ts",
          "kind": "definition"
        },
        {
          "path": "types/node/events.d.ts",
          "kind": "definition"
        },
        {
          "path": "types/node/globals.d.ts",
          "kind": "definition"
        },
        {
          "path": "types/node/index.d.ts",
          "kind": "definition"
        },
        {
          "path": "types/node/perf_hooks.d.ts",
          "kind": "definition"
        },
        {
          "path": "types/node/stream.d.ts",
          "kind": "definition"
        },
        {
          "path": "types/node/stream/consumers.d.ts",
          "kind": "definition"
        },
        {
          "path": "types/node/test/buffer.ts",
          "kind": "test"
        },
        {
          "path": "types/node/test/crypto.ts",
          "kind": "test"
        },
        {
          "path": "types/node/test/events.ts",
          "kind": "test"
        },
        {
          "path": "types/node/test/perf_hooks.ts",
          "kind": "test"
        },
        {
          "path": "types/node/test/stream.ts",
          "kind": "test"
        },
        {
          "path": "types/node/test/url.ts",
          "kind": "test"
        },
        {
          "path": "types/node/test/util.ts",
          "kind": "test"
        },
        {
          "path": "types/node/test/wasi.ts",
          "kind": "test"
        },
        {
          "path": "types/node/test/worker_threads.ts",
          "kind": "test"
        },
        {
          "path": "types/node/tsconfig.json",
          "kind": "package-meta",
          "suspect": "not [the expected form](https://github.com/DefinitelyTyped/DefinitelyTyped#user-content-tsconfigjson) and not moving towards it (check: `compilerOptions.target`)"
        },
        {
          "path": "types/node/url.d.ts",
          "kind": "definition"
        },
        {
          "path": "types/node/util.d.ts",
          "kind": "definition"
        },
        {
          "path": "types/node/v16/buffer.d.ts",
          "kind": "definition"
        },
        {
          "path": "types/node/v16/stream/consumers.d.ts",
          "kind": "definition"
        },
        {
          "path": "types/node/worker_threads.d.ts",
          "kind": "definition"
        }
      ],
      "owners": [
        "Microsoft",
        "DefinitelyTyped",
        "jkomyno",
        "alvis",
        "r3nya",
        "btoueg",
        "smac89",
        "touffy",
        "DeividasBakanas",
        "eyqs",
        "Hannes-Magnusson-CK",
        "hoo29",
        "kjin",
        "ajafff",
        "islishude",
        "mwiktorczyk",
        "mohsen1",
        "n-e",
        "galkin",
        "parambirs",
        "eps1lon",
        "SimonSchick",
        "ThomasdenH",
        "WilcoBakker",
        "wwwy3y3",
        "samuela",
        "kuehlein",
        "bhongy",
        "chyzwar",
        "trivikr",
        "yoursunny",
        "qwelias",
        "ExE-Boss",
        "peterblazejewicz",
        "addaleax",
        "victorperin",
        "ZYSzys",
        "NodeJS",
        "LinusU",
        "wafuwafu13",
        "mcollina"
      ],
      "addedOwners": [],
      "deletedOwners": [],
      "popularityLevel": "Critical"
    }
  ],
  "reviews": [
    {
      "type": "approved",
      "reviewer": "rbuckton",
      "date": "2022-10-03T21:17:06.000Z",
      "isMaintainer": true
    },
    {
      "type": "stale",
      "reviewer": "peterblazejewicz",
      "date": "2022-08-03T21:10:28.000Z",
      "abbrOid": "70a166f"
    },
    {
      "type": "stale",
      "reviewer": "LinusU",
      "date": "2022-07-08T12:58:43.000Z",
      "abbrOid": "00417c0"
    },
    {
      "type": "stale",
      "reviewer": "sheetalkamat",
      "date": "2022-07-05T21:33:53.000Z",
      "abbrOid": "00417c0"
    }
  ],
  "mainBotCommentID": 1099384271,
  "ciResult": "pass"
}

🔔 @microsoft @DefinitelyTyped @jkomyno @alvis @r3nya @btoueg @smac89 @Touffy @DeividasBakanas @eyqs @Hannes-Magnusson-CK @hoo29 @kjin @ajafff @islishude @mwiktorczyk @mohsen1 @n-e @galkin @parambirs @eps1lon @SimonSchick @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.

If anybody has advice for how to test that Node types do not pollute the global namespace with a Blob interface, I'd love to add one. Failing that, behavior should remain the same as it was before.

@thw0rted The CI build failed! Please review the logs for more information.

Once you've pushed the fixes, the build will automatically re-run. Thanks!

Note: builds which are failing do not end up on the list of PRs for the DT maintainers to review.

@thw0rted The CI build failed! Please review the logs for more information.

Once you've pushed the fixes, the build will automatically re-run. Thanks!

Note: builds which are failing do not end up on the list of PRs for the DT maintainers to review.

This PR is going to require a bigger change than I originally expected. The problem is that the package tsconfig is using lib: ["dom"]. I changed that to "es6", which broke a few things. As a result, I have added a full interface declaration for Node's DOM-compatibility Event and related types, under a fictitious module (node:dom-events).

If they were exported at the global level -- which, to be clear, they should be! -- they would break every project that uses both @types/node and lib.dom.d.ts, which is of course how we got into this mess in the first place. There was a long conversation about how to handle this in another issue but I don't think we really got a good answer. (Consuming code can do some trickery with conditional types evaluated against globalThis but I don't see how to fix it for use within the types-package.)

I think we may need smarter Typescripters than myself to take a look at this and suggest a way forward. Are there other fundamentally-incompatible global types that differ between Node and the DOM? If so, how have they been handled? If not... now what?

@thw0rted The CI build failed! Please review the logs for more information.

Once you've pushed the fixes, the build will automatically re-run. Thanks!

Note: builds which are failing do not end up on the list of PRs for the DT maintainers to review.

The CI build keeps failing but I don't understand how the broken packages ever worked. Two of them depend on @types/mongodb which was removed last year (Mongo now ships their own types), and the old package is flat-out broken. One of them has a test that looks like $ExpectType "one" | "two" | "three" but that syntax never worked as far as I can tell. Any help with resolving the CI problems would be much appreciated.

The change I just pushed exposes Event and EventTarget globally via the same mechanism used in #58277. I'm not sure how to test and make sure this doesn't explode real-world users...

In hindsight, this PR has grown in scope, maybe too much. I just created https://github.com/thw0rted/DefinitelyTyped/tree/node-minus-dom which is a tiny patch that only changes the tsconfig lib setting. There were three places that broke:

• one reference to performance was simply mistaken; Node does not expose that as a global, you're supposed to import it from perf_hooks
• one reference to global WebAssembly which has not been implemented yet. All references in WASI just have a TODO comment, so I commented out the reference and left a copy of the same TODO comment in the relevant test.
• One reference to global EventTarget. I put in a workaround that refers to the (unexported) "DOMEventTarget" interface; at some point actually making a global EventTarget would be better.

If the reviewer(s) would prefer I can file a separate PR with just that change, then maybe a separate PR just for global Event/EventTarget and another one for the Blob/Stream stuff? Not sure what would represent less overall work/complexity/controversy.

@thw0rted The CI build failed! Please review the logs for more information.

Once you've pushed the fixes, the build will automatically re-run. Thanks!

Note: builds which are failing do not end up on the list of PRs for the DT maintainers to review.

@thw0rted Unfortunately, this pull request currently has a merge conflict 😥. Please update your PR branch to be up-to-date with respect to master. Have a nice day!

There are no merge conflicts anymore, but I don't see signoffs from any of the reviewers either. @LinusU, @rbuckton, @peterblazejewicz do you want to approve this now?

I don't want to derail this if somebody is actually planning to merge it, but at least a note for potential future work: #60924 (comment) points out that Node's fetch is actually just taken from undici (an official Node project), which ships its own types, including their interpretation of all the WHATWG streams and whatnot. Any types currently living under @types/node that also exist under undici should be removed and pulled from there instead by reference. (If Node wants to maintain their own types, that sounds great to me...)

Re-ping @microsoft, @DefinitelyTyped, @jkomyno, @alvis, @r3nya, @btoueg, @smac89, @Touffy, @DeividasBakanas, @eyqs, @Hannes-Magnusson-CK, @hoo29, @kjin, @ajafff, @islishude, @mwiktorczyk, @mohsen1, @n-e, @galkin, @parambirs, @eps1lon, @SimonSchick, @ThomasdenH, @WilcoBakker, @wwwy3y3, @samuela, @kuehlein, @bhongy, @chyzwar, @trivikr, @yoursunny, @qwelias, @ExE-Boss, @peterblazejewicz, @addaleax, @victorperin, @ZYSzys, @nodejs, @LinusU, @wafuwafu13, @mcollina:

This PR has been out for over a week, yet I haven't seen any reviews.

Could someone please give it some attention? Thanks!

Ready to merge

This PR is breaking Vue 3. I'll draft issue in both Vue repo and DefinitelyTyped repo soon.

See #62759 and vuejs/core#6899