Skip to content

Example of hoisting devtools dependency #1303

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

Merged
merged 8 commits into from
Apr 19, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
14 changes: 9 additions & 5 deletions dwds/CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -1,11 +1,15 @@
## 10.0.2-dev
## 11.0.0-dev

- Support `vm_service` version `6.2.0`.
- Fix missing sdk libraries in `getObject()` calls.
- Fix incorrect `rootLib` returned by `ChromeProxyService`.
- Fix not working breakpoints in library part files.
- Fix data race in calculating locations for a module.

**Breaking changes:**
- `Dwds.start` no longer supports automatically injecting a devtools server. A `devtoolsLauncher`
callback must be provided to support launching devtools.

## 10.0.1

- Support `webkit_inspection_protocol` version `^1.0.0`.
Expand Down Expand Up @@ -33,14 +37,14 @@
## 9.0.0

- Fix an issue where relative worker paths provided to the `ExpressionCompilerService`
would cause a crash.
would cause a crash.
- Fix an issue where the injected client connection could be lost while the application
is paused.
- Support keep-alive for debug service connections.
- Depend on the latest `package:sse`.
- Filter out DDC temporary variables from the variable inspection view.
- Add `DwdsEvent`s around stepping and evaluation.
- Send an event to the Dart Debug Extension that contains VM service protocol URI.
- Send an event to the Dart Debug Extension that contains VM service protocol URI.
- Depend on `package:vm_service` version `6.1.0+1`.
- Update the `keepAlive` configs to prevent accidental reuse of a connection after stopping
a debug session.
Expand All @@ -52,7 +56,7 @@

## 8.0.3

- Fix an issue where failed hot restarts would hang indefinitely.
- Fix an issue where failed hot restarts would hang indefinitely.

## 8.0.2

Expand Down Expand Up @@ -201,7 +205,7 @@
- Update the `require_restarter` to rerun main after a hot restart to align with
the legacy strategy. We therefore no longer send a `RunRequest` after a hot
restart.
- Compute only the required top frame for a paused event.
- Compute only the required top frame for a paused event.
- Change `streamListen` to return an `RPCError` / error code `-32601` for streams
that are not handled.
- Populate information about async Dart frames.
Expand Down
2 changes: 1 addition & 1 deletion dwds/debug_extension/pubspec.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ dependencies:
pedantic: ^1.5.0
pub_semver: ^2.0.0
sse: ^3.8.1
web_socket_channel: ^1.0.0
web_socket_channel: ^2.0.0
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this wasn't version solving locally


dev_dependencies:
webdev: ^2.0.0
Expand Down
7 changes: 4 additions & 3 deletions dwds/lib/dwds.dart
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,7 @@ export 'src/readers/asset_reader.dart' show AssetReader;
export 'src/readers/frontend_server_asset_reader.dart'
show FrontendServerAssetReader;
export 'src/readers/proxy_server_asset_reader.dart' show ProxyServerAssetReader;
export 'src/servers/devtools.dart';
export 'src/services/chrome_proxy_service.dart' show ChromeDebugException;
export 'src/services/expression_compiler.dart'
show ExpressionCompilationResult, ExpressionCompiler, ModuleInfo;
Expand Down Expand Up @@ -104,18 +105,17 @@ class Dwds {
bool useSseForDebugProxy,
bool useSseForDebugBackend,
bool useSseForInjectedClient,
bool serveDevTools,
UrlEncoder urlEncoder,
bool spawnDds,
bool enableDevtoolsLaunch,
DevtoolsLauncher devtoolsLauncher,
}) async {
hostname ??= 'localhost';
enableDebugging ??= true;
enableDebugExtension ??= false;
useSseForDebugProxy ??= true;
useSseForDebugBackend ??= true;
useSseForInjectedClient ??= true;
serveDevTools ??= true;
enableDevtoolsLaunch ??= true;
spawnDds ??= true;
globalLoadStrategy = loadStrategy;
Expand Down Expand Up @@ -143,8 +143,9 @@ class Dwds {
if (urlEncoder != null) extensionUri = await urlEncoder(extensionUri);
}

var serveDevTools = devtoolsLauncher != null;
if (serveDevTools) {
devTools = await DevTools.start(hostname);
devTools = await devtoolsLauncher(hostname);
var uri =
Uri(scheme: 'http', host: devTools.hostname, port: devTools.port);
_logger.info('Serving DevTools at $uri\n');
Expand Down
112 changes: 63 additions & 49 deletions dwds/lib/src/injected/client.js

Large diffs are not rendered by default.

10 changes: 2 additions & 8 deletions dwds/lib/src/servers/devtools.dart
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@

import 'dart:io';

import 'package:devtools_server/devtools_server.dart';
typedef DevtoolsLauncher = Future<DevTools> Function(String hostname);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Won't this still require a dependency on DevTools given the type?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I believe this is just the DevTools wrapper you wrote below

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah my mistake. We'll want to model this as a breaking change so we need a pubspec and changelog update. But otherwise LGTM.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Will do, thanks!


/// A server for Dart Devtools.
class DevTools {
Expand All @@ -19,13 +19,7 @@ class DevTools {
/// All subsequent calls to [close] will return this future.
Future<void> _closed;

DevTools._(this.hostname, this.port, this._server);
DevTools(this.hostname, this.port, this._server);

Future<void> close() => _closed ??= _server.close();

static Future<DevTools> start(String hostname) async {
var server =
await serveDevTools(hostname: hostname, enableStdinCommands: false);
return DevTools._(server.address.host, server.port, server);
}
}
2 changes: 1 addition & 1 deletion dwds/lib/src/version.dart

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

8 changes: 4 additions & 4 deletions dwds/pubspec.yaml
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
name: dwds
# Every time this changes you need to run `pub run build_runner build`.
version: 10.0.2-dev
version: 11.0.0-dev
homepage: https://github.com/dart-lang/webdev/tree/master/dwds
description: >-
A service that proxies between the Chrome debug protocol and the Dart VM
Expand All @@ -15,9 +15,6 @@ dependencies:
built_value: '>=6.7.0 <9.0.0'
crypto: '>=2.0.6 < 4.0.0'
dds: ^1.4.1
# devtools_server indirectly depends on devtools so keep this around.
devtools: ^2.0.0
devtools_server: ^2.0.0
http: '>=0.12.0 < 0.14.0'
http_multi_server: ^3.0.0
logging: '>=0.11.3 < 2.0.0'
Expand Down Expand Up @@ -55,3 +52,6 @@ dev_dependencies:
test: ^1.6.0
uuid: '>=2.0.0 < 4.0.0'
webdriver: '>=2.0.0 < 4.0.0'
# devtools_server indirectly depends on devtools so keep this around.
devtools: ^2.0.0
devtools_server: ^2.0.0
36 changes: 21 additions & 15 deletions dwds/test/fixtures/server.dart
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ import 'package:http_multi_server/http_multi_server.dart';
import 'package:shelf/shelf.dart';
import 'package:shelf/shelf_io.dart' as shelf_io;
import 'package:webkit_inspection_protocol/webkit_inspection_protocol.dart';
import 'package:devtools_server/devtools_server.dart' as devtools_lancher;

Handler _interceptFavicon(Handler handler) {
return (request) async {
Expand Down Expand Up @@ -94,21 +95,26 @@ class TestServer {
});

var dwds = await Dwds.start(
assetReader: assetReader,
buildResults: filteredBuildResults,
chromeConnection: chromeConnection,
loadStrategy: strategy,
spawnDds: spawnDds,
serveDevTools: serveDevTools,
enableDebugExtension: enableDebugExtension,
enableDebugging: enableDebugging,
useSseForDebugProxy: useSse,
useSseForDebugBackend: useSse,
useSseForInjectedClient: useSse,
hostname: hostname,
urlEncoder: urlEncoder,
expressionCompiler: expressionCompiler,
);
assetReader: assetReader,
buildResults: filteredBuildResults,
chromeConnection: chromeConnection,
loadStrategy: strategy,
spawnDds: spawnDds,
enableDebugExtension: enableDebugExtension,
enableDebugging: enableDebugging,
useSseForDebugProxy: useSse,
useSseForDebugBackend: useSse,
useSseForInjectedClient: useSse,
hostname: hostname,
urlEncoder: urlEncoder,
expressionCompiler: expressionCompiler,
devtoolsLauncher: serveDevTools
? (hostname) async {
var server = await devtools_lancher.serveDevTools(
hostname: hostname, enableStdinCommands: false);
return DevTools(server.address.host, server.port, server);
}
: null);

var server = await HttpMultiServer.bind('localhost', port);
var cascade = Cascade();
Expand Down
1 change: 1 addition & 0 deletions frontend_server_common/pubspec.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -7,5 +7,6 @@ environment:

dependencies:
file: ^6.0.0
usage: ^4.0.0
dwds:
path: ../dwds
34 changes: 20 additions & 14 deletions webdev/lib/src/serve/webdev_server.dart
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ import 'package:http_multi_server/http_multi_server.dart';
import 'package:shelf/shelf.dart';
import 'package:shelf/shelf_io.dart' as shelf_io;
import 'package:shelf_proxy/shelf_proxy.dart';
import 'package:devtools_server/devtools_server.dart';

import '../command/configuration.dart';
import 'chrome.dart';
Expand Down Expand Up @@ -128,21 +129,26 @@ class WebDevServer {
options.configuration.verbose,
);
}

var shouldServeDevTools =
options.configuration.debug || options.configuration.debugExtension;
dwds = await Dwds.start(
hostname: options.configuration.hostname,
assetReader: assetReader,
buildResults: filteredBuildResults,
chromeConnection: () async =>
(await Chrome.connectedInstance).chromeConnection,
loadStrategy: loadStrategy,
serveDevTools:
options.configuration.debug || options.configuration.debugExtension,
enableDebugExtension: options.configuration.debugExtension,
enableDebugging: options.configuration.debug,
spawnDds: !options.configuration.disableDds,
expressionCompiler: ddcService,
);
hostname: options.configuration.hostname,
assetReader: assetReader,
buildResults: filteredBuildResults,
chromeConnection: () async =>
(await Chrome.connectedInstance).chromeConnection,
loadStrategy: loadStrategy,
enableDebugExtension: options.configuration.debugExtension,
enableDebugging: options.configuration.debug,
spawnDds: !options.configuration.disableDds,
expressionCompiler: ddcService,
devtoolsLauncher: shouldServeDevTools
? (String hostname) async {
var server = await serveDevTools(
hostname: hostname, enableStdinCommands: false);
return DevTools(server.address.host, server.port, server);
}
: null);
pipeline = pipeline.addMiddleware(dwds.middleware);
cascade = cascade.add(dwds.handler);
cascade = cascade.add(assetHandler);
Expand Down
2 changes: 1 addition & 1 deletion webdev/lib/src/version.dart

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

7 changes: 5 additions & 2 deletions webdev/pubspec.yaml
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
name: webdev
# Every time this changes you need to run `pub run build_runner build`.
version: 2.7.2
version: 2.7.3
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I believe this is a non breaking change for webdev, but I'm not sure if this is a minor or patch version bump

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Minor should be fine.

# We should not depend on a dev SDK before publishing.
# publish_to: none
homepage: https://github.com/dart-lang/webdev
Expand All @@ -18,7 +18,7 @@ dependencies:
browser_launcher: ^0.1.5
crypto: ^3.0.0
dds: ^1.4.1
dwds: ^10.0.0
dwds: ^11.0.0
http: ^0.13.0
http_multi_server: ^3.0.0
io: ^0.3.2+1
Expand All @@ -36,6 +36,9 @@ dependencies:
vm_service: '>=3.0.0 <7.0.0'
webkit_inspection_protocol: '>=0.4.0 <2.0.0'
yaml: ^3.0.0
# devtools_server indirectly depends on devtools so keep this around.
devtools: ^2.0.0
devtools_server: ^2.0.0

dev_dependencies:
build: ^2.0.0
Expand Down