Control flow analysis for dependent parameters #47190
Conversation
typescript-bot
added
Author: Team
For Uncommitted Bug
|
@typescript-bot perf test faster |
|
Heya @ahejlsberg, I've started to run the parallelized Definitely Typed test suite on this PR at 35f7909. You can monitor the build here. |
|
Heya @ahejlsberg, I've started to run the abridged perf test suite on this PR at 35f7909. You can monitor the build here. Update: The results are in! |
|
Heya @ahejlsberg, I've started to run the inline community code test suite on this PR at 35f7909. You can monitor the build here. Update: The results are in! |
|
Heya @ahejlsberg, I've started to run the extended test suite on this PR at 35f7909. You can monitor the build here. |
ahejlsberg
requested review from
DanielRosenwasser,
RyanCavanaugh and
weswigham
There was a problem hiding this comment.
A test showing how this behavior is disabled when one of the parameters is reassigned would be nice.
With this, would we revisit giving overload implementation parameters a contextual type equal to the union of the overload parameter lists (under noImplicitAny), so overloads can make use of this without kinda contorting your function definition style?
|
@ahejlsberg Here they are:Comparison Report - main..47190
System
Hosts
Scenarios
Developer Information: |
|||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
@ahejlsberg |
|
I was wondering if this could help us with some XState types and I've created a more complex test case that resembles how we would like to utilize this feature (which is awesome!) but unfortunately, it turns out that this doesn't work for us. I believe there is a potential for refining the Is this something that could be addressed in this PR? Is making this work aligning with your goals for this feature? Should this be tracked separately? EDIT:// after looking more into this - this is actually out of the scope of this PR since simpler situations (with regular object/tuple structures) don't work either: It's because Just as a note - with a dedicated structure it's at least possible to narrow this situation down with custom type predicated (like here) but it would be awesome if this would just work when using language features. And, of course, this approach (custom type predicates) won't work with dependent parameters at all. |
|
@Andarist The issue in your example is that the discriminant is stored in a nested object instead of directly in a property. In other words, this doesn't work: type XStateActionArgs =
| [event: { type: "LOG_OUT" }, context: { user: { name: string } }]
| [event: { type: "LOG_IN" }, context: { user: null }];whereas this does: type XStateActionArgs =
| [event: "LOG_OUT", context: { user: { name: string } }]
| [event: "LOG_IN", context: { user: null }];Effectively, discriminants only affect the directly containing type. This is true for discriminated unions in general, and isn't specific to this PR. |
|
I'm not sure if this is a bug, or out of scope but it seems to me like something like this should work const fn: (...args: [number, number] | [string, string]) => void = (a, b) => {
if (typeof a === "number") {
console.log(a + b); // 'b' still 'string | number'
}
}What could be the reason for this to fail? |
I came out the same issue. Did you resolve it? @KamenKolev |
|
I'm not 100% sure but I think that this only works for discriminated unions and a discriminated union mandates that you are using a literal member~. |
|
Hmm, is there a reason this doesn't work for methods? type Foo = {
method(...args:
[type: "str", cb: (e: string) => void] |
[type: "num", cb: (e: number) => void]
): void;
}
// this fails for some reason, as a method
let fooBad: Foo = {
method(type, cb) {
if (type == 'num') {
cb(123) // error!
} else {
cb("abc") // error!
}
}
};
// suddenly works for arrow function
let fooOkay1: Foo = {
method: (type, cb) => {
if (type == 'num') {
cb(123)
} else {
cb("abc")
}
}
};
// also works for manual destructuring of rest args
let fooOkay2: Foo = {
method(...args) {
const [type, cb] = args;
if (type == 'num') {
cb(123)
} else {
cb("abc")
}
}
};(note to self, follow up on this SO question) I guess if I don't hear back here I can open a new issue |
|
For the record - I'm in the process of debugging this issue ☝️ . I'm not sure if I will be able to actually fix it - but I'm trying 😉 I already have debugged this to the very exact code line where the behavior is diverging for those cases. EDIT:// I think I have a fix locally - gonna prepare a PR today or tomorrow morning. |
|
We might need to open a new issue for this and then see if the TS team would even accept a PR for it. |
|
The PR is already up: #48110 We can go through a ceremony of opening an issue and attaching it to this PR but from my PoV that would just be redundant bureaucracy |
|
I would assume this PR would have fixed this kind of issue, is this a separate issue? |
|
I think this issue is something entirely else. I'm also not sure if this is an issue if we consider how TS is designed. You are operating here on However, it seems that perhaps this TS playground could actually work 🤔 Either way, I would classify this as a totally separate issue - cause knowing what this PR was about this is something entirely else. Take my words with a grain of salt - I'm just a bystander here 😅 |
|
Is there a reason this doesn't work if the tuples in the union are of different lengths? type Foo = (...args: [type: "one"] | [type: "two", x: string]) => void;
const foo: Foo = (type, x) => { // error!
if (type !== "one") x.toUpperCase();
}Or potentially of different lengths? type Foo = (...args: [type: "one", x?: number] | [type: "two", x: string]) => void;
const foo: Foo = (type, x) => { // error!
if (type !== "one") x.toUpperCase();
}Shall I file this as a new issue or is there something I'm missing? |
This PR builds on the work in #46266 to support control flow analysis for dependent parameters contextually typed by a rest parameter with a discriminated union type. Some examples:
Fixes #46658.