inferFromUsage: use ChangeTracker and typeToTypeNode #22379
Conversation
| @@ -221,49 +216,41 @@ namespace ts.codefix { | |||
| } | |||
| } | |||
|
|
|||
| function getTypeAccessiblityWriter(checker: TypeChecker): EmitTextWriter { | |||
| let str = ""; | |||
| function isTypeAccessible(type: Type, enclosingDeclaration: Declaration, checker: TypeChecker): boolean { | |||
There was a problem hiding this comment.
@weswigham Is there a better way to do this now?
There was a problem hiding this comment.
typeToTypeNode itself should have an argument that lets you pass in visibility tracking hooks in the same way without needing to print the type.
There was a problem hiding this comment.
Had to add that parameter as an internal overload -- should it be public?
There was a problem hiding this comment.
I don't think we want to encourage using it? EmitTextWriter is internal, so it's not like the other method was public.
There was a problem hiding this comment.
OK, will leave it internal. (#22384)
ghost
force-pushed
the
inferFromUsage_textChanges
branch
from
8360377 to
316073b
Compare
ghost
force-pushed
the
inferFromUsage_textChanges
branch
from
316073b to
e58d270
Compare
src/services/textChanges.ts
Outdated
| @@ -343,6 +345,13 @@ namespace ts.textChanges { | |||
| this.replaceRange(sourceFile, { pos, end: pos }, createToken(modifier), { suffix: " " }); | |||
| } | |||
|
|
|||
| public insertTypeAnnotation(sourceFile: SourceFile, node: TypeAnnotatable, type: TypeNode): void { | |||
There was a problem hiding this comment.
so why not just replaceNode with a node with a type annotation?
There was a problem hiding this comment.
To avoid an issue like #22358 -- we should be adding new text but not reformatting existing text
There was a problem hiding this comment.
i see.. we should add a comment for that then..
ghost
mentioned this pull request
Now the only fix left not using
ChangeTrackerisfixJSDocTypes, which could probably be changed in a similar way.