(chore) bumps typescript to 4.7 #1496
Conversation
| @@ -18,7 +18,7 @@ export class FindReferencesProviderImpl implements FindReferencesProvider { | |||
| const { lang, tsDoc } = await this.getLSAndTSDoc(document); | |||
| const fragment = tsDoc.getFragment(); | |||
|
|
|||
| const references = lang.getReferencesAtPosition( | |||
There was a problem hiding this comment.
Don't know what's the difference between these two methods. But the one we used before no longer returns isDefinition
There was a problem hiding this comment.
From a quick TS codebase search it seems that findReferences is like getReferencesAtPosition with an additional step afterwards, at least they both call getReferencedSymbolsForNode which does the actual work. Not sure what exactly changed that the definition is no longer part of it. Maybe they filter out the definition-position already now, which means we can keep using and just remove the check? Edit: When rewriting the store transformation and do additional logic in references/rename/etc instead, we need the definition to appear, so it would be bad if the definition is filtered out by TS now, and using findReferences would be better.
There was a problem hiding this comment.
Found a place where it does make a difference. If you use a go-to reference on the definition.
It would invoke includeDeclaration: true. Previously if there is only one reference it would take you to the reference. if we use the findReferences and remove the check it would not take you to the reference but show both reference and definition.
|
I'm ok with it no longer working, it doesn't appear in many places and I want to switch to the new transformation in about a month. |
|
I ported the string literal completion as a fallback before you commented XD. I think we can use this fallback and if someone finds any difference we can just tell them it won't matter soon. It's ready for merge now. Do we want to merge it now? Or do we want to wait a few days until vscode release a new version with 4.7? To see if people have problems with 4.7. |
Draft for now as it seems like the string literal prop completion is broken. From my debugging, it seems like typescript now looks at the first argument of the component class construtor instead of the
JSX.ElementAttributesProperty. Since we plan to switch to the new transformation, not sure if it's still worth the effort to ask the typescript team to support it.