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

Expose APIs for other VS Code extensions to use functionality from the Dart extension #5008

Open
yahu1031 opened this issue Feb 25, 2024 · 10 comments

Comments

@yahu1031
Copy link

Is your feature request related to a problem? Please describe.
Not exactly a problem, but extensions depending on dart LSP will make this feature request more profitable. Exposing required/specific data would be more helpful.

Describe the solution you'd like
Exposing analyzer.

Additional context
In my case, we are building the fluttergpt extension, which will utilize the dart LSP. This had become a blocker for us. Ref: CommandDash/commanddash#224

@DanTup
Copy link
Member

DanTup commented Feb 25, 2024

@yahu1031 can you add details of exactly what functionality you're using (code links would be helpful too) so I can understand exactly what would need to be provided if "raw" access to the analyzer was taken away?

A brief description of why you need it (eg. what functionality it powers) would be useful (not just which specific LSP requests you're calling).

If there are things you're not currently using, but are considering or might be helpful, please include details too.

Thanks!

@DanTup DanTup added the awaiting info Requires more information from the customer to progress label Feb 25, 2024
@DanTup DanTup changed the title Exposing analyzer. Expose APIs for other VS Code extensions to use the Dart LSP server Feb 25, 2024
@samyakkkk
Copy link

@DanTup yes, we are building an AI extension that provides extra abilities to Flutter developers on top of the current existing Dart extension.

We found it really promising that the internal APIs like the analyzer (that we currently use to get the Flutter outline), and other like devtools, pubGlobal (that we plan to use) are exposed making it easy for us to call these methods from our extension. Loosing access to those will require us to duplicate the same code on our end.

On a separate issue, we had a brief discussion with Jacob and others in the Flutter team. We wanted to figure out a way that we could get direct access to the Dart Server spun up by the VSCODE extension because a duplicate instance on our end is not feasible because of the memory issue. We run custom kind of analysis on the files for various purposes.

I could start a separate thread/issue for the dart server access extension issue, but in general the exposed internal APIs could come really handy to us in future and would love if they are kept exposed. (maybe in a safer way).

Thanks.

@DanTup
Copy link
Member

DanTup commented Feb 26, 2024

@samyakkkk the problem with the internal APIs is that they are not designed for consumption and have no versioning. They frequently change (particularly when things are refactored) and will break your extension (like this recent change).. this is not a problem for the tests they are intended for because those are versioned in the same repository.

I don't want to break any of your functionality, but it's not feasible to officially support all of the current private APIs (plus anything that is added in future to simplify for testing), the surface area is just too large and volatile (it's just internal classes exposed). If other extensions need access to things, we need to expose them as a supported API that we can commit to keeping stable.

For the LSP server specifically, the plan we've discussed before is to block access to state-modifying requests that would break the Dart extension (like textDocument/*). My expectation is that you are not using these APIs so I think your use of the LSP server will fit within the APIs that we'd like to make available. However, I can't confirm that without more details about what you're using.

If you're also using other (non-LSP) APIs (you mentioned Pub?), we also need to know which to see what would be required to provide a stable API over them.

Thanks!

@DanTup DanTup changed the title Expose APIs for other VS Code extensions to use the Dart LSP server Expose APIs for other VS Code extensions to use functionality from the Dart extension Feb 26, 2024
@samyakkkk
Copy link

samyakkkk commented Feb 26, 2024

@DanTup that makes sense. To clarify, we are only using the analyzer object at these two instances:

  1. To get the outline. Can we keep the fileTracker and flutterOutlineTreeProvider exposed for this?
  2. To get the definition of the textDocument. This has a special purpose (we parse back the semantic tokens to understand what objects in the current code are class/method etc). Does the new planned APIs provide support for this?. Any early notes that we can also have a look at?

For the other APIs, our use case would be to be able to see the simulators that are available, launch them from our extension and listen to the logger in both tests and app runs. I assume this needs access to: VsCodeTestController, FlutterDaemon and also to the logger to listen to the logs also get access to the VMService URL.

@DanTup I'd also like to ask you that what parts of this could also be done by us if we rewrite the code on our end? this will allow us to request API access only when we are about to start on the features and if due to some reason it is not possible, we can do it on our side even though it requires duplications.

What exactly is not possible to be duplicated on our end, like it would required analyzer object that we do not have?

Appreciate your helpful answers all along.

@DanTup
Copy link
Member

DanTup commented Feb 26, 2024

To get the outline. Can we keep the fileTracker and flutterOutlineTreeProvider exposed for this?

I'm not against keeping it (and other things) there for now, but we should make a formal/supported API for getting this. One option is to just ensure if we formally expose LSP, you can subscribe to the notification directly. The notification is already specified as part of the LSP server, but using analyzer.fileTracker.waitForOutline() is not the way you should access to it (because it's not guaranteed that interface won't change).

To get the definition of the textDocument. This has a special purpose (we parse back the semantic tokens to understand what objects in the current code are class/method etc). Does the new planned APIs provide support for this?. Any early notes that we can also have a look at?

If we expose access to LSP, you'd likely be able to call textDocument/definition and semantic tokens methods. There aren't any formal notes because nothing serious has started yet. Work has begun on supporting DevTools extensions and it seems that access to functionality of the LSP server would be useful there.

For the other APIs, our use case would be to be able to see the simulators that are available, launch them from our extension and listen to the logger in both tests and app runs. I assume this needs access to: VsCodeTestController, FlutterDaemon and also to the logger to listen to the logs also get access to the VMService URL.

Thanks, this is useful info too. Again, I don't think we should just expose the internals of these classes, but we should consider supported APIs where they would be useful for other tools to build on.

We have already started exposing an API for the VS Code extension that's used by the Flutter sidebar (see definitions here). It includes events for when devices and debug sessions change, but it's not officially exposed for anything besides the sidebar yet. Adding to this might be the best way to provide the non-LSP parts that you need (and could be a convenient place to add things like waitForOutline if that makes more sense than handling the events and keeping your own cache of them too).

@samyakkkk
Copy link

Understood the point you are making @DanTup . It seems promising to add the required support to the newly exposed VScode APIs.

I also read the notifications documentation but that may not be the solution for us, since our requirements are more of actively seeking information and triggering actions, than listening passively for events. However, I agree with the intent here and can open new issue(s) documenting with our requirements, for considering if they can be officially support.

Thanks again for keeping the currently exposed APIs intact. I apologise for the trouble on this. We didn't consider this case and became too much dependent on the internal APIs :)

@DanTup DanTup removed the awaiting info Requires more information from the customer to progress label Feb 26, 2024
@DanTup DanTup added this to the On Deck milestone Feb 26, 2024
@DanTup
Copy link
Member

DanTup commented Feb 26, 2024

Had a quick chat with @jacob314 about this earlier, who noted some of the APIs described above are not really VS Code-specific (for example asking for a list of simulators and launching them). These APIs would be better exposed in an editor-agnostic way instead of adding to the API linked above that was VS Code-specific.

The right route forward is probably to finish getting DTD up and running and then look to build new APIs there. Dart-Code will be able to provide an endpoint to communicate with DTD.

I also read the notifications documentation but that may not be the solution for us, since our requirements are more of actively seeking information and triggering actions, than listening passively for events.

Understood. It might be useful to write up some notes on exactly what the complete flow is here (what the user does, what information you ultimately need to collect). Some of the code above was issuing requests in a loop which might not be very efficient (particularly if it involves resolving files that aren't open/cached). If there are some common needs for data by different tools, it might be worth considering some custom requests that can provide data more efficiently.

For now there's probably not much for you to do besides keep notes here of what you're using/planning to use. If/when there are better things for you to use I'll update here. In the meantime, I'll try not to make changes that'll break you, but as noted above it may be hard to guarantee. If you do notice anything break you then don't hesitate to ping me (here or Discord etc.). I'd recommend remaining on the Pre-release version of the Dart extension because you might see any potential issues earlier.

@samyakkkk
Copy link

Fantastic to see that DTD is being worked on, definitely needed. Added one comment regarding custom ast analysis possibility on the current dart server.

I'll soon update our notes regarding the full flow and what APIs that we anticipate needing. For now, having the internal APIs exposed helps. Have also upgraded to pre-release to be on the lookout. thanks again!

@tomgilder
Copy link
Contributor

Hello, for Wings I'm using two private APIs, which I think should be very viable to support publicly.

They're just the SDK roots, in order to know where the currently-in-use Flutter and Dart installs are: workspaceContext.sdks.flutter and workspaceContext.sdks.dart.

@DanTup
Copy link
Member

DanTup commented Apr 16, 2024

@tomgilder thanks! Those should be easy to handle - although some things worth noting:

  • The user can change their SDK while the extension is running (this triggers a reload of the Dart extension internally) so probably we need to provide some kind of event/stream for this so you can re-read the SDK if required
  • When a Flutter project is open, the Dart/Flutter SDKs will be consistent (that is, the Dart SDK will be inside the Flutter SDK). However when a non-Flutter project is open, they may be different (because we're tracking the Flutter SDK only for if the user tries to execute Flutter commands like "Flutter: New Project" but may have used a preferred Dart SDK that isn't part of a Flutter SDK)

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