-
Notifications
You must be signed in to change notification settings - Fork 12.8k
session.ts: Revert some emptyArray
back to undefined
#17781
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You're planning to port this to release-2.5
rather than reverting?
src/server/session.ts
Outdated
@@ -585,7 +585,7 @@ namespace ts.server { | |||
): ReadonlyArray<protocol.DiagnosticWithLinePosition> | ReadonlyArray<protocol.Diagnostic> { | |||
const { project, file } = this.getFileAndProject(args); | |||
if (isSemantic && isDeclarationFileInJSOnlyNonConfiguredProject(project, file)) { | |||
return emptyArray; | |||
return undefined; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
undefined [](start = 23, length = 9)
This appears to have been []
before your original change.
@@ -601,7 +601,7 @@ namespace ts.server { | |||
|
|||
const definitions = project.getLanguageService().getDefinitionAtPosition(file, position); | |||
if (!definitions) { | |||
return emptyArray; | |||
return undefined; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
undefined [](start = 23, length = 9)
This was probably fine as emptyArray
, but I have no problem with reverting it.
@@ -669,7 +669,7 @@ namespace ts.server { | |||
const occurrences = project.getLanguageService().getOccurrencesAtPosition(file, position); | |||
|
|||
if (!occurrences) { | |||
return emptyArray; | |||
return undefined; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
undefined [](start = 23, length = 9)
Was probably fine.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
So -- leave it, or change it like this PR does?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In master
, whichever is more consistent with the other requests. My impression is that it's undefined
. In release-2.5
, whichever it used to return (also undefined
?).
Reverts the changes in #17165 that changed return values from
undefined
toemptyArray
.Does not revert any type-only changes or changes from
[]
toemptyArray
.Ref: #17165 (comment)