Skip to content

Refactor wish list #1094

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

Open
nojaf opened this issue May 16, 2025 · 4 comments
Open

Refactor wish list #1094

nojaf opened this issue May 16, 2025 · 4 comments

Comments

@nojaf
Copy link
Contributor

nojaf commented May 16, 2025

As mentioned here, it would be great to refactor the extension and LSP server. Here are some ideas to consider; please feel free to raise other points or challenge any listed:

  • Use promises in the LSP server, as many endpoints spin a binary or read files.
  • Refactor incrementalCompilation.ts and implement an interface to handle the differences between bsb and rewatch, as those checks are scattered throughout the code.
  • Utilize semver for more version checks.
  • Enable more logging in the output window; see Initial SendLogNotification #1066. Wink @zth.
  • Employ a formatter (e.g., prettier/biome), as our lack of it is quite annoying.
  • Remove chokidar from the LSP server and use the [DidChangeWatchedFiles Notification] instead.
  • Bump the TypeScript version, as we likely have an outdated one.
  • Consider removing ESBuild; do we still need to bundle?
  • Should we implement the mono-repo structure properly? Currently, we have three package.json files, and the top one lacks a workspaces entry.
  • Use yarn as we do in rescript compiler?
  • Update CONTRIBUTING.md, as it may no longer be accurate.

Let me know your thoughts, @zth, @cometkim, @mediremi!

@fhammerschmidt
Copy link
Member

fhammerschmidt commented May 16, 2025

At some point the server part should be in OCaml (#269).
Then we only have a small client left where we could get rid of the typescript dependency entirely. The client might not be too much work to rewrite in ReScript instead?

@nojaf
Copy link
Contributor Author

nojaf commented May 16, 2025

At some point the server part should be in OCaml (#269).

True, but all the above are feasible. I started working on an OCaml LSP some time ago: https://github.com/nojaf/rescript/tree/lsp. It is a lot of work, and I became a bit demotivated upon learning that the analysis-tool currently relies on being run as a single execution. During the retreat, there was also mention of possibly doing this in Rust. I'm not sure which option is the lesser evil, though.

@nojaf
Copy link
Contributor Author

nojaf commented May 16, 2025

The client might not be too much work to rewrite in ReScript instead?

Yeah, that could be fun to have in the end.

@mediremi
Copy link
Contributor

We probably want to switch from TextDocumentSyncKind.Full to TextDocumentSyncKind.Incremental (see "Incremental Text Document Synchronization" from the language server extension guide) and use VS Code's TextDocuments documents manager instead of rolling our own file content cache as we currently do.

import {
  createConnection,
  TextDocumentSyncKind
} from 'vscode-languageserver/node';
import {
  TextDocuments
} from 'vscode-languageserver';
import { TextDocument } from 'vscode-languageserver-textdocument';

const connection = createConnection();
const documents = new TextDocuments(TextDocument);
documents.listen(connection);

connection.onInitialize(() => ({
  capabilities: {
    textDocumentSync: TextDocumentSyncKind.Incremental
  }
}));

connection.listen();

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants