Repository navigation
Add logic for VS code to promote other VS code extensions #4705
Description
As part of the broader effort to support dart ecosystem extensions, we've designed a mechanism for a pub package to provide extension information as part of its package. For DevTools, this is the precompiled flutter web app that DevTools will embed. For VS Code extensions, the pub package can include a config.yaml file that tells the Dart-Code promotion which extension should be promoted:
my_package/
lib/
...
extension/
vs_code/
# contains meta data that VS code needs to point to another
# extension on the VS code marketplace
config.yaml
package:extension_discovery has logic to detect available extensions for a given .dart_tool/package_config.json file. Since this package is written in Dart, it cannot be used by the Dart-Code extension directly, so it will need to be called into from the Dart-Code extension by proxy of some other Dart service. Eventually, this would be a great fit for the Dart Tooling Daemon, but since this does not exist yet, we can use the DevTools server in the interim.
We can add an API to DevTools server that the Dart-Code extension can call, and the devtools_server will call findExtensions('vs_code', <path/to/package_config.json>) from package:extension_discovery on the Dart-Code extension's behalf.
Open Question
The element we still need to figure out is how to handle a monorepo workspace, where there are multiple package_config.json files available.
Related: Flutter DevTools Extensions
Activity
DanTup commentedon Sep 4, 2023
@anderdobo @mosuem FYI - I wonder whether there's value in trying to use this for arb-editor (eg. have the Flutter/intl packages provide the recommendation)? It might need something extra (only show the recommendation if an arb file is opened?), but it might work out better than special handling for one extension?
mosuem commentedon Sep 5, 2023
To recap an offline discussion: We will go with implementing the suggestion the extension itself for now, see #4719. We will possibly migrate to using this new feature. At the moment, the lack of a file filter ("only show the recommendation on opening
.arbfiles") would result in too much noise for Flutter users.DanTup commentedon Sep 5, 2023
We could have the client just call a single API for each project it discovers in the workspace. A single API that takes a set of projects might be slightly more efficient, but I suspect internally it would just do the same anyway so it may be better to keep the API simple.
One slight tweak I'd make though (at least for the API called by the IDE) is to accept a package folder rather than the
package_config.jsondirectly. Dart-Code tries to avoid knowing about implementation details like.dart_tool/package_config.json(although it's unlikely to happen again, this file has moved in the past from.packages->.dart_tool/package_config.json) so I'd prefer to keep that in the Dart world.DanTup commentedon Nov 20, 2023
I made a start on this at #4847 (and some SDK work in the DevTools server that needs tests before uploading).
@kenzieschmoll @anderdobo Jacob asked me today whether this includes transitive dependencies or only direct dependencies. It's using package:extension_discovery which I think reads the
.dart_tool/package_config.jsonand might therefore include transitive dependencies -although I'm not sure whether that's the behaviour we want?kenzieschmoll commentedon Nov 20, 2023
Right now,
package:extension_discoveryincludes transitive dependencies. @jonasfj if I remember correctly, there is not currently a way to distinguish between transitive and direct deps based on thepackage_config.jsonfile, right?If we did only want to include direct dependencies, I think this would be easy to do by modifying the
extension_discoverypackage so that it has a parameterincludeTransitiveDependencies, and when false, it could only return extensions whose parent package has an entry in thepubspec.yamlfile at the project root that is passed in to thefindExtensionsmethod.DanTup commentedon Dec 4, 2023
@anderdobo @jacob314 are you happy with how this displays in my current PR?
In particular, we need to be sure the user is aware they are installing third-party code that is not owned/reviewed by the Flutter team.
@kenzieschmoll @jacob314
dependenciesor alsodev_dependencies?)kenzieschmoll commentedon Dec 4, 2023
serverpodis a good first candidate. What do you need for the whitelist? just the package name? @vlidholt - can you confirm thatserverpodis the correct serverpod package that you would like to promote your VS code extension from?Since we are using a whitelist for now, I think that including transitive deps is okay. I do think we should update
extension_discoverypackage to take a parameterincludeTransitivein thefindExtensionsmethod though. (CC @jonasfj). We do want to include dev dependencies, IMO. For example, someone may provide a package solely for developer tooling (this package could contain a devtools extension and may promote a paired vs code extension too).DanTup commentedon Dec 4, 2023
I guess we could do it either by package name or extension identifier. There's probably not a lot of difference. If we have the right package name(s) I'll use them.
kenzieschmoll commentedon Dec 4, 2023
Extension identifier might be better since the package could arbitrarily change the VS code extension it is asking to promote, and then we really don't have control over the VS code extension whitelist.
jonasfj commentedon Dec 4, 2023
I think we can let findExtensions return a field indicating whether it's a direct or transitive dependency.
Please file an issue on repository, then let's discuss that separately.
Correct: the information is not in package_config.json, so we'll need to read pubspec.yaml
kenzieschmoll commentedon Dec 4, 2023
Filed dart-lang/tools#214 to track adding transitive dependency distinction to
extension_discoveryanderdobo commentedon Dec 4, 2023
@DanTup the notification and language LGTM. Is this ready to be used to promote ARB Editor? If so, we should.
DanTup commentedon Dec 4, 2023
I think we'll have a different codepath for the ARB editor because it won't be driven by a package (but rather opening arb files), but I do plan to look at that shortly too (there's an issue at #4719 tracking that).
jacob314 commentedon Dec 6, 2023
LGTM as long as the initial whitelisted packages aren't ones where the transitive dependency case is a concern. I'm not concerned about Serverpod as odds of it only being a transitive dependency seem very low (why are your dependencies using serverpod if you aren't running serverpod?).
On the other hand, you really wouldn't want to show Freezed (e.g. you depend on a package that is using Freezed for its internal serialization but you have no idea what it is) unless they were direct dependencies because none or your app is using the functionality.
[dds/devtools_server] Add support for finding VS Code extensions defi…