New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
node: Backport #59905 to ts4.8 #62629
Conversation
Node.js types are currently out of sync between TS 4.9+ and TS <= 4.8 variants. Specifically, PR DefinitelyTyped#59905 introduced many changes that were not copied over to ts4.8. This patch is an exact copy of that PR, but applied to ts4.8. The goal is that `@types/node` behaves the same on all versions of TypeScript.
@meyfa Thank you for submitting this PR! This is a live comment which I will keep updated. 1 package in this PR
Code ReviewsBecause 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
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": 62629,
"author": "meyfa",
"headCommitOid": "e13e1b9c275dfc02fb37c7f346b33714dcd42a5b",
"mergeBaseOid": "29fc0c582c5d9dc2fd41b4c9ca576ea40ccd4d15",
"lastPushDate": "2022-10-09T10:24:50.000Z",
"lastActivityDate": "2022-10-10T21:10:56.000Z",
"mergeOfferDate": "2022-10-10T19:42:24.000Z",
"mergeRequestDate": "2022-10-10T21:10:56.000Z",
"mergeRequestUser": "meyfa",
"hasMergeConflict": false,
"isFirstContribution": false,
"tooManyFiles": false,
"hugeChange": false,
"popularityLevel": "Critical",
"pkgInfo": [
{
"name": "node",
"kind": "edit",
"files": [
{
"path": "types/node/ts4.8/buffer.d.ts",
"kind": "definition"
},
{
"path": "types/node/ts4.8/dom-events.d.ts",
"kind": "definition"
},
{
"path": "types/node/ts4.8/events.d.ts",
"kind": "definition"
},
{
"path": "types/node/ts4.8/globals.d.ts",
"kind": "definition"
},
{
"path": "types/node/ts4.8/index.d.ts",
"kind": "definition"
},
{
"path": "types/node/ts4.8/perf_hooks.d.ts",
"kind": "definition"
},
{
"path": "types/node/ts4.8/stream.d.ts",
"kind": "definition"
},
{
"path": "types/node/ts4.8/stream/consumers.d.ts",
"kind": "definition"
},
{
"path": "types/node/ts4.8/test/buffer.ts",
"kind": "test"
},
{
"path": "types/node/ts4.8/test/crypto.ts",
"kind": "test"
},
{
"path": "types/node/ts4.8/test/events.ts",
"kind": "test"
},
{
"path": "types/node/ts4.8/test/perf_hooks.ts",
"kind": "test"
},
{
"path": "types/node/ts4.8/test/stream.ts",
"kind": "test"
},
{
"path": "types/node/ts4.8/test/url.ts",
"kind": "test"
},
{
"path": "types/node/ts4.8/test/util.ts",
"kind": "test"
},
{
"path": "types/node/ts4.8/test/wasi.ts",
"kind": "test"
},
{
"path": "types/node/ts4.8/test/worker_threads.ts",
"kind": "test"
},
{
"path": "types/node/ts4.8/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/ts4.8/url.d.ts",
"kind": "definition"
},
{
"path": "types/node/ts4.8/util.d.ts",
"kind": "definition"
},
{
"path": "types/node/ts4.8/worker_threads.d.ts",
"kind": "definition"
},
{
"path": "types/node/v16/ts4.8/buffer.d.ts",
"kind": "definition"
},
{
"path": "types/node/v16/ts4.8/stream/consumers.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": "peterblazejewicz",
"date": "2022-10-10T19:41:39.000Z",
"isMaintainer": true
}
],
"mainBotCommentID": 1272511058,
"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 |
CC @thw0rted are you okay with this change? Thanks for the great work on the original patch, by the way! Very awesome! :) |
Thanks, I'm glad it's working out. I just skimmed the diff but it looks good, as long as all the tests are passing and whatnot. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@meyfa thanks!
Ready to merge |
v16.11.65 caused a regression on my end. Can be seen in https://github.com/wixplosives/pleb, by cloning, removing the lock file, and running
Can be worked around by adding "dom" to tsconfig->lib, which kinda beats the purpose of this PR. |
I think I see the issue. In the updated tests, we check for I put together a quick PR that will add the type-side of Blob (along with tests): #62654 |
|
||
// See comment above explaining conditional type | ||
type __EventTarget = typeof globalThis extends { onmessage: any, EventTarget: infer T } | ||
? T |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe T['prototype']
?
The spec for EventTarget says it has a constructor, so the type declaration for |
And for |
OK, I think I'm going to need someone smarter than I am to explain why changing |
Is anybody still looking at this? Is there maybe a new issue? I'm having a really hard time putting together a reliable reproduction. I just want to add tests that say |
Node.js types are currently out of sync between TS 4.9+ and TS <= 4.8 variants. Specifically, PR #59905 introduced many changes that were not copied over to ts4.8. This patch is an exact copy of that PR, but applied to ts4.8. The goal is that
@types/node
behaves the same on all versions of TypeScript.Please fill in this template.
npm test <package to test>
.If changing an existing definition:
ts4.8
directories were introduced in Update node types for TS 4.9 beta #62375.