Skip to content
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

feat: extend typescript help signatures with mongodb items VSCODE-403 #509

Draft
wants to merge 16 commits into
base: main
Choose a base branch
from

Conversation

alenakhineika
Copy link
Contributor

@alenakhineika alenakhineika commented Apr 5, 2023

Description

This PR is not necessarily going to be merged in the existing format. Treat it as an investigation/collaboration resource.

The JS contextual signature help for find thinks that any find keyword is the Array.find() method. This is confusing for users of the extension, because MongoDB has its own usage and syntax the find method.

VSCode does not allow extensions to change the results of other extensions, therefor to overcome this obstacle we use the TypeScript Language Service API to extend default JavaScript Method Signatures in playgrounds.

If the cursor is positioned in the query node we show the query method help:

Collection.find(query, projection, options) : Cursor
---
Selects documents in a collection or view.

And similarly for the aggregate method, we show the aggregate method help:

Collection.aggregate(pipeline, options) : Cursor
---
Calculates aggregate values for the data in a collection or a view.

If the current node is neither find nor aggregation, we return the TypeScript language server help signatures.

Checklist

Motivation and Context

  • Bugfix
  • New feature
  • Dependency update
  • Misc

Types of changes

  • Backport Needed
  • Patch (non-breaking change which fixes an issue)
  • Minor (non-breaking change which adds functionality)
  • Major (fix or feature that would cause existing functionality to change)

@alenakhineika alenakhineika marked this pull request as ready for review April 6, 2023 13:37
src/language/tsLanguageService.ts Outdated Show resolved Hide resolved
src/language/tsLanguageService.ts Outdated Show resolved Hide resolved
src/language/tsLanguageService.ts Outdated Show resolved Hide resolved
src/language/tsLanguageService.ts Outdated Show resolved Hide resolved
src/language/tsLanguageService.ts Outdated Show resolved Hide resolved
src/language/visitor.ts Outdated Show resolved Hide resolved
src/language/tsLanguageService.ts Outdated Show resolved Hide resolved
src/language/server.ts Show resolved Hide resolved
src/language/tsLanguageService.ts Outdated Show resolved Hide resolved
src/language/tsLanguageService.ts Outdated Show resolved Hide resolved
src/language/tsLanguageService.ts Outdated Show resolved Hide resolved
Comment on lines 141 to 145
// Return a new instance of the language service.
getLanguageService(jsDocument: TextDocument): ts.LanguageService {
currentTextDocument = jsDocument;
return jsLanguageService;
},
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The comment above this function and its name are a bit misleading – the function always returns the same instance, not a new one. Would it make more sense to either a) create a new language service instance each time or b) split this function into two functions, getLanguageService(): ts.LanguageService and setCurrentTextDocument(jsDocument: TextDocument): void?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The getLanguageService function is a part of ts.LanguageServiceHost type, which we can not extend with setCurrentTextDocument. Rephrased the comment to reduce confusion.

src/language/tsLanguageService.ts Outdated Show resolved Hide resolved
src/language/tsLanguageService.ts Outdated Show resolved Hide resolved
src/language/tsLanguageService.ts Show resolved Hide resolved
};

// Create the language service files.
const jsLanguageService = ts.createLanguageService(host);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[Question only:] Correct me if I’m wrong, but the general structure here is that host serves as our way to connect the TS logic to our VSCode documents, but other than that, the LanguageService instance is unaware of the fact that it’s running against a VSCode environment/as part of a VSCode extension. Is that right? Could we potentially use this, say, inside of mongosh to provide autocompletions there?

Copy link
Contributor Author

@alenakhineika alenakhineika Apr 12, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes. I just tried to use it in tsLanguageService for completions:

doComplete(
  document: TextDocument,
  position: Position
): CompletionItem[] {
  const jsDocument = TextDocument.create(document.uri, 'javascript', document.version, document.getText());
  const jsLanguageService = this._host.getLanguageService(jsDocument);
  const offset = jsDocument.offsetAt(position);
  const jsCompletion = jsLanguageService.getCompletionsAtPosition(
    jsDocument.uri,
    offset,
    { includeExternalModuleExports: false, includeInsertTextCompletions: false }
  );
...
}

and in global.d.ts:

declare global {
  enum Stages {
    match = '$match',
  }

  let db: {
    getCollection(coll: string): {
      find(query?: Document, projection?: Document, options?: FindOptions): Promise<FindCursor>;
      aggregate(pipeline: [{ [key in Stages]: Document }], options: Document & {
        explain?: never;
      }): Promise<AggregationCursor>;
      aggregate(pipeline: [{ [key in Stages]: Document }], options: Document & {
        explain: ExplainVerbosityLike;
      }): Promise<Document>;
      aggregate(...stages: [{ [key in Stages]: Document }]): Promise<AggregationCursor>;
    };
  };
}

And this will give us completions.

Screenshot 2023-04-12 at 13 53 36

So as your comment below states the next step here is to figure out how to create a .d.ts file that would include everything we need, so we could reuse it across all products.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

But all of this does not solve the Int8Array.find problem anyway :) it continues to appear even knowing the context.


declare global {
let use: (dbName: string) => void;
}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I’m guessing the next step here is to figure out how to create a .d.ts file for mongosh environments? Probably not something for this PR, though 🙂

@alenakhineika alenakhineika mentioned this pull request Apr 14, 2023
11 tasks
@alenakhineika alenakhineika marked this pull request as draft April 14, 2023 15:30
…escript-help-signatures

# Conflicts:
#	package-lock.json
#	package.json
#	src/language/server.ts
#	src/language/visitor.ts
…escript-help-signatures

# Conflicts:
#	src/language/server.ts
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants