search

dart-lang / pub

The pub command line tool
https://dart.dev/tools/pub/cmd
BSD 3-Clause "New" or "Revised" License
1.04k stars 229 forks source link

remove the dep on package:frontend_server_client? #3401

Open devoncarew opened 2 years ago

devoncarew commented 2 years ago

We bring the webdev repository into the Dart sdk repository as part of our DEPS file - https://github.com/dart-lang/sdk/blob/main/DEPS#L448. It looks like the only thing used from webdev is package:frontend_server_client, and the only sdk consumer of that is the pub client code.

From looking at the pub code, it looks like that's only used in one place here - https://github.com/dart-lang/pub/blob/master/lib/src/dart.dart#L178. I'm wondering if there's a way to replace the use of package:frontend_server_client here - perhaps delegating out to one of the dart compile sub-commands?

sigurdm commented 2 years ago

cc @jakemac53 who made the frontend_server_client integration.

My main worry is the performance impact. We use frontend_server_client to enable incremental compilation - not sure that is available with dart compile - we used to invoke dart --snapshot to compile, but I believe talking to the frontend server is much faster.

We could also just vendor (inline) package:frontent_server_client into pub if the size of the webdev repo is the concern.

devoncarew commented 2 years ago

This dependency - pub on frontend_server_client, and then the sdk repo on the webdev repo - isn't a super pressing concern; I'm just looking to trim deps generally.

If dart compile is plug-in compatible from a use case perspective - or can be made so - than this is probably worth investigating.

jakemac53 commented 2 years ago

Fwiw, mostly what this package does is act as a wrapper around the CLI of frontend_server (as well as knowing how to launch it). Moving the same functionality to dart compile likely wouldn't save us much unless we exposed a much restricted set of the functionality.

See also https://github.com/dart-lang/sdk/issues/47372, all this functionality would be ideally be moved into the SDK? Granted that doesn't help to remove the dependency.