Avoid convertExport when there's a non-identifier or a bogus one
#44106
Conversation
typescript-bot
added
Author: Team
For Uncommitted Bug
elibarzilay
requested review from
andrewbranch and
sandersn
and removed request for
andrewbranch
|
@jessetrinity: ping |
There was a problem hiding this comment.
Couple of additional questions. I personally think returning emptyArray is OK for a bug fix, but I agree that we should get @jessetrinity's opinion.
| /////*[| |]*/ /** x */ export default x; | ||
|
|
||
| goTo.eachRange(r => { | ||
| goTo.selectRange(r); |
There was a problem hiding this comment.
interesting, do you have to select each range? I thought having the cursor there would be enough.
There was a problem hiding this comment.
But there are three ranges in three files... In any case, existing cases use markers, which I didn't want to mix with a comment.
There was a problem hiding this comment.
The cursor there alone is not supposed to be enough unless invoked by the refactor command, because I thought it would be too annoying. See https://github.com/microsoft/TypeScript/blob/master/tests/cases/fourslash/refactorConvertExportForTriggerReason.ts
There was a problem hiding this comment.
OK. Some refactors are offered in cases other than explicit invoking of the refactor command, right? I always forget about that.
|
@jessetrinity: I made it an error now. I think that there is one minor difference to the previous code where it returns this error instead of |
Fixes #44105