Refine Extract Local#20729
Conversation
Still insert a blank line after extracted properties.
Expose `isContextSensitive` from the checker and omit type annotations for expressions for which it returns false.
sandersn
left a comment
There was a problem hiding this comment.
Looks good, although I wonder whether there are many people who always want the type annotation. I think this is common in C#, but I don't have a good guess for Typescript.
|
@sandersn, yes, it should probably be controlled by a setting. |
|
Note that this change will produce too few type annotations until #20727 is fixed. |
weswigham
left a comment
There was a problem hiding this comment.
Eh, I'm a little wary of using isContextSensitive as is, but it's likely a decent heuristic for now. Just to check if we've covered the other things contextual types are used for correctly (and to verify that this does what you'd like in more cases), can you add a test extracting the lambda from:
const myObj: { member(x: number, y: string): void } = {
member: (x, y) => x + y,
}(or point me at a similar one)? (I expect the extracted constant to have an annotation, in this case)
|
@weswigham Like this? // ==ORIGINAL==
const myObj: { member(x: number, y: string): void } = {
member: /*[#|*/(x, y) => x + y/*|]*/,
}
// ==SCOPE::Extract to constant in enclosing scope==
const newLocal: (x: number, y: string) => void = (x, y) => x + y;
const myObj: { member(x: number, y: string): void } = {
member: /*RENAME*/newLocal,
} |
|
Note to anyone reverting this change: the blank line portion (unfortunately, spread across three commits) is essentially a separate change and can be retained even if the context-sensitive check turns out to be infeasible. |
Fixes #19839.