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

Adding LSP as a standalone parser type #118

Open
dangmai opened this issue Aug 26, 2019 · 15 comments
Open

Adding LSP as a standalone parser type #118

dangmai opened this issue Aug 26, 2019 · 15 comments

Comments

@dangmai
Copy link
Owner

dangmai commented Aug 26, 2019

This is a continuation of our discussion on Twitter between me and @ntotten. The purpose is to add the ability for prettier-plugin-apex to interface directly with Salesforce LSP that is started in VSCode.

Here's my thought on how this would be implemented:

  • Currently there's an option called --apex-standalone-parser with 2 possible choices: none and built-in. I'd like to add another option lsp.
  • I'd need Salesforce to implement a custom endpoint on the LSP to get the AST from it. Preferably this will give me back the result in JSON format, like the implementation here: https://github.com/dangmai/apex-ast-serializer/blob/master/src/main/java/net/dangmai/serializer/Apex.java (a few things that apex-ast-serializer does: serializes the AST to JSON in a standard way for multiple data structures, mark circular references) If that's not possible, I will have to write custom code to parse their AST.
  • On Salesforce side, set the default for this option to lsp on their prettierrc documentation for VSCode integration, so that people who follow that documentation gets the benefits out of the box.

The 1.0.0 release is basically ready at this point, but if the previous steps can be done quickly then I'd be happy to wait for them to finish before releasing 1.0.0.

@dangmai
Copy link
Owner Author

dangmai commented Sep 15, 2019

Gently pinging @ntotten - do you know when you'd have resource on your side to work on this task yet? I want to push 1.0.0 sooner rather than later, and this is the only task left for it.

Here's how I'm envisioning it could work: Because prettier-plugin-apex relies on the AST being serialized in a particular way, I can publish a maven library for apex-ast-serializer that Salesforce can pull into your jorje in order to use in your custom LSP endpoints. Once that is done I will modify prettier-plugin-apex to support that endpoint.

Any thoughts?

@dangmai
Copy link
Owner Author

dangmai commented Sep 28, 2019

Since I haven't heard back about this issue, I will release 1.0.0 in the near future without this feature; it will be added later in a patch release.

@ntotten
Copy link

ntotten commented Jan 6, 2020

We still want to do this. Hopefully, we can prioritize soon.

@nawforce
Copy link

I am getting a few moans about performance due to devs not wanting to have to start the server. I was wondering if auto starting the server might be an option, it's a little awkward because Prettier is assuming parse() is synchronous so you have to busy wait in some form for the port to become available. On the plus side though it would work from command line, IntelliJ other language servers etc.

Is it worth me looking at creating a PR for this to add a new option something like 'built-in-with-autolaunch'?

@dangmai
Copy link
Owner Author

dangmai commented Feb 24, 2020

Hey @nawforce, I'm glad to hear that you're using them in your company, and would love a PR to make performance better! What do you have in mind for a solution? (i.e. where would you initiate the server start sequence/where would you stop the server?)

FYI I thought about this a while back, and opened an issue upstream here - Prettier currently does not offer lifecycle hooks for plugins, which (to me) would be the right place to handle this.

@dangmai
Copy link
Owner Author

dangmai commented Feb 24, 2020

In the mean time, one thing you can do for your team is to run the HTTP server in an internal server (let's say at http://apex.internal:2117), then version control your .prettierrc and use this setting:

{
  "apexStandaloneParser": "built-in",
  "apexStandaloneHost": "apex.internal"
}

This might or might not work in your particular circumstance (i.e. that server will not be reachable outside of your internal network), but just a thought experiment for a quick fix.

@dangmai
Copy link
Owner Author

dangmai commented Feb 24, 2020

One other potential quick fix I just thought of: If your team is using 1 IDE (e.g. VSCode), you can set the editor up so that it auto launches the HTTP Server when the project directory is opened, and you can version control that so that everyone on your team runs it - I actually set this project up that way: see here.

@nawforce
Copy link

nawforce commented Mar 4, 2020

Thanks for suggestions, think we are going to try and get a local server up but I will play with the auto-starting idea as that will give us another option just in case. Starting from VSCode is a pretty smart idea, if only I could convince everyone to use it...

@pozil
Copy link

pozil commented Jun 29, 2021

@dangmai how about introducing a hybrid mode with a standalone parser that automatically shuts down after a certain time of inactivity:

  1. first prettier call: plugin checks if standalone parser is running. If not, it start the the standalone parser server.
  2. second prettier call: plugin checks if standalone parser is running. If yes, it send content to the server and server inactivity timer is reset
  3. some time after last prettier call: server's inactivity timer expires and server auto shuts down

Is that option worth exploring?

@dangmai
Copy link
Owner Author

dangmai commented Jun 29, 2021

Unfortunately that's not possible because currently Prettier doesn't expose any lifecycle hook for us to spin up/shut down the server - see my issue upstream here: prettier/prettier#5317

@pozil
Copy link

pozil commented Jun 29, 2021

Ok I agree that it would be way cleaner to work with a lifecycle hook but while waiting for this Prettier feature, perhaps you could embed the logic to start the server somewhere in parse?
Later on, when the lifecycle hook comes up, this logic can be moved in the appropriate spot.

@pozil
Copy link

pozil commented Jun 29, 2021

Something like this (pseudo code):

function parse(sourceCode, _, options) {
  if (options.apexStandaloneParser === "hybrid") {
    if (!isServerRunning()) {
      await startTemporaryStandaloneServer();
    }
    serializedAst = parseTextWithHttp(
      sourceCode,
      options.apexStandaloneHost,
      options.apexStandalonePort,
      options.parser === "apex-anonymous",
    );
  }
  ...
}

@dangmai
Copy link
Owner Author

dangmai commented Jun 29, 2021

There's no await in Prettier, everything has to be synchronous unfortunately (see prettier/prettier#4459). If we go down this path, we have to spawn a process to wait for the server to be up, which defeats the point of the whole thing.

@dangmai
Copy link
Owner Author

dangmai commented Jun 29, 2021

In any case though, I'd prefer to have this issue to be tracking the possibility of including the parsing server inside LSP. Let's discuss about other possible improvements by opening new issues. I will say that I thought about the hybrid mode before, and it's not something I want to pursue in this library - it's a very leaky abstraction that I don't want to take on, and there are better tools out there that can be composed to achieve the same effect. For example, the workaround to auto start the server on VSCode open that I showed in an earlier comment.

@pozil
Copy link

pozil commented Jun 30, 2021

Fair enough, thanks for letting me know.

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

No branches or pull requests

4 participants