Update lodash tests for TS 4.8 #61480
Conversation
Typescript 4.8 now prefers the asserted type in type predicate narrowing
in the rare case where both types are assignable to each other. Previously it
preferred the type of the provided parameter.
Usually this is desired, since type predicates
are essentially type assertions. For example, narrowing `unknown` to a
specific type is very common. But in _.isFunction, the intent is to
keep the original signature when narrowing a union:
```ts
declare const value: number | (() => void);
if (_.isFunction(value)) {
value; // $ExpectType () => void
}
```
Unfortunately functions that return void are assignable to functions
that return any *and vice versa*, so the new result is that `value` is
narrowed to isFunctions' return type:
```ts
declare const value: number | (() => void);
if (_.isFunction(value)) {
value; // $ExpectType (...args: any[]) => any
}
```
I've updated the tests to reflect that. This change doesn't apply to
functions that return types besides void, but the tests happen to use
such a function.
|
@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": 61480,
"author": "sandersn",
"headCommitOid": "7f72912e09d93521816f302a7f9b61c2c11433cc",
"mergeBaseOid": "d7b179c7a9a4aa4ff13f0608606ae10a94349014",
"lastPushDate": "2022-07-27T16:26:51.000Z",
"lastActivityDate": "2022-07-27T16:27:17.000Z",
"hasMergeConflict": false,
"isFirstContribution": false,
"tooManyFiles": false,
"hugeChange": false,
"popularityLevel": "Critical",
"pkgInfo": [
{
"name": "lodash",
"kind": "edit",
"files": [
{
"path": "types/lodash/lodash-tests.ts",
"kind": "test"
}
],
"owners": [
"bczengel",
"chrootsu",
"aj-r",
"e-cloud",
"thorn0",
"jtmthf",
"DomiR",
"WilliamChelman"
],
"addedOwners": [],
"deletedOwners": [],
"popularityLevel": "Critical"
}
],
"reviews": [],
"mainBotCommentID": 1196980310,
"ciResult": "fail",
"ciUrl": "https://github.com/DefinitelyTyped/DefinitelyTyped/commit/7f72912e09d93521816f302a7f9b61c2c11433cc/checks?check_suite_id=7547590249"
} |
|
🔔 @bczengel @chrootsu @aj-r @e-cloud @thorn0 @jtmthf @DomiR @WilliamChelman — please review this PR in the next few days. Be sure to explicitly select |
typescript-bot
moved this from Waiting for Code Reviews
to Needs Author Action
in New Pull Request Status Board
|
@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 failure is backbone's dependency on a version of knex that doesn't work with 4.8, but I have a PR out for that: knex/knex#5279. I'm going to merge this so that the lodash tests go back to passing, but an owner needs to think about whether |
Typescript 4.8 now prefers the asserted type in type predicate narrowing in the rare case where both types are assignable to each other. Previously it preferred the type of the provided parameter.
Usually this is desired, since type predicates are essentially type assertions. For example, narrowing
unknownto a specific type is very common. But in _.isFunction, the intent is to keep the original signature when narrowing a union:Unfortunately functions that return void are assignable to functions that return any and vice versa, so the new result is that
valueis narrowed to isFunctions' return type:I've updated the tests to reflect that. This change doesn't apply to functions that return types besides void, but the tests happen to use such a function.
I poked at
_.isFunctionto try to find a workaround but couldn't find one that worked.