Skip to content

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

Merged
2 commits merged into from
Aug 14, 2017

Conversation

ghost
Copy link

@ghost ghost commented Aug 14, 2017

Reverts the changes in #17165 that changed return values from undefined to emptyArray.
Does not revert any type-only changes or changes from [] to emptyArray.
Ref: #17165 (comment)

Copy link
Member

@amcasey amcasey left a 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?

@@ -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;
Copy link
Member

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;
Copy link
Member

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;
Copy link
Member

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.

Copy link
Author

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?

Copy link
Member

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?).

@ghost ghost changed the base branch from master to AddIsGlobalCompletionRestrictions August 14, 2017 20:03
@ghost ghost changed the base branch from AddIsGlobalCompletionRestrictions to release-2.5 August 14, 2017 20:03
@ghost ghost merged commit eeb40fe into release-2.5 Aug 14, 2017
@ghost ghost deleted the session_undefined_array branch August 14, 2017 21:59
@microsoft microsoft locked and limited conversation to collaborators Jun 14, 2018
This pull request was closed.
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants