Update node types.isMap tests #61389
Conversation
microsoft/TypeScript#49625 improves handling of unions in type predicates so that unions are correctly preserved. This breaks types.isMap in node. For now I just changed the tests' expected type, but the type of isMap doesn't make much sense to me. It should probably be changed, but that's a much more complex task. This break is tracked at microsoft/TypeScript#49988 although it's correct, I think, so not very likely to be reverted.
|
@sandersn Thank you for submitting this PR! This is a live comment which I will keep updated. 1 package in this PRCode 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
Once every item on this list is checked, I'll ask you for permission to merge and publish the changes. Diagnostic Information: What the bot saw about this PR{
"type": "info",
"now": "-",
"pr_number": 61389,
"author": "sandersn",
"headCommitOid": "de41ab6c557ab2f8d667d37c179fdb4ce7d1c93e",
"mergeBaseOid": "0096577e5a57944e0e1fb5ac71dee1187147bf09",
"lastPushDate": "2022-07-22T15:44:30.000Z",
"lastActivityDate": "2022-07-22T16:06:05.000Z",
"hasMergeConflict": false,
"isFirstContribution": false,
"tooManyFiles": false,
"hugeChange": false,
"popularityLevel": "Critical",
"pkgInfo": [
{
"name": "node",
"kind": "edit",
"files": [
{
"path": "types/node/test/util_types.ts",
"kind": "test"
},
{
"path": "types/node/v14/test/util.ts",
"kind": "test"
},
{
"path": "types/node/v16/test/util_types.ts",
"kind": "test"
}
],
"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": "mcollina",
"date": "2022-07-22T16:06:05.000Z",
"isMaintainer": false
},
{
"type": "stale",
"reviewer": "peterblazejewicz",
"date": "2022-07-21T18:39:47.000Z",
"abbrOid": "59e7b9d"
}
],
"mainBotCommentID": 1191814022,
"ciResult": "fail",
"ciUrl": "https://github.com/DefinitelyTyped/DefinitelyTyped/commit/de41ab6c557ab2f8d667d37c179fdb4ce7d1c93e/checks?check_suite_id=7480486468"
} |
|
🔔 @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 |
|
@sandersn 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. |
typescript-bot
added
Owner Approved
|
@sandersn 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 only test failure is in weak-napi. It's unrelated to this change, although it is a result of microsoft/TypeScript#49625 too. |
microsoft/TypeScript#49625 improves handling of unions in type predicates so that unions are correctly preserved. This breaks types.isMap in node.
For now I just changed the tests' expected type, but the type of isMap doesn't make much sense to me. It should probably be changed, but that's a much more complex task.
Edit: isSet is broken on node 14 too.
This break is tracked at microsoft/TypeScript#49988 although it's correct, I think, so not very likely to be reverted.