Skip to content

handle org-dartlang-app library uris in dwds #381

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

Closed
wants to merge 2 commits into from
Closed
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
5 changes: 5 additions & 0 deletions dwds/CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -1,3 +1,8 @@
## 0.3.3-dev

- Handle `org-dartlang-app` scheme library uris and convert them to match the
dart uris that we will see in the sourcemaps.

## 0.3.2

- Add support for `scope` in `evaluate` calls.
Expand Down
26 changes: 20 additions & 6 deletions dwds/lib/src/chrome_proxy_service.dart
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ import 'dart:async';
import 'dart:convert';
import 'dart:io';

import 'package:path/path.dart' as p;
import 'package:pedantic/pedantic.dart';
import 'package:pub_semver/pub_semver.dart' as semver;
import 'package:vm_service_lib/vm_service_lib.dart';
Expand Down Expand Up @@ -119,11 +120,21 @@ class ChromeProxyService implements VmServiceInterface {
..kind = EventKind.kResume
..isolate = isolateRef;

for (var library in await _getLibraryNames()) {
for (var uri in await _getLibraryUris()) {
// The `org-dartlang-app` scheme is used to reference files that are not
// under the `lib` directory (and thus can't be referenced using a
// `package` scheme).
//
// We want to make these match the uris in the source map, which are
// relative to a top level directory in the package (such as `web` or
// `test`), so we strip the scheme and skip the first path segment.
var sourceMapUri = uri.scheme == 'org-dartlang-app'
? p.url.joinAll(['/'].followedBy(uri.pathSegments.skip(1)))
: '$uri';
isolate.libraries.add(LibraryRef()
..id = library
..name = library
..uri = library);
..id = sourceMapUri
..name = sourceMapUri
..uri = sourceMapUri);
}

// TODO: Something more robust here, right now we rely on the 2nd to last
Expand Down Expand Up @@ -803,13 +814,16 @@ function($argsString) {
}

/// Runs an eval on the page to compute all the library names in the app.
Future<List<String>> _getLibraryNames() async {
Future<List<Uri>> _getLibraryUris() async {
Copy link
Member

Choose a reason for hiding this comment

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

[nit] https://dart.dev/guides/language/effective-dart/design#avoid-starting-a-method-name-with-get

Future<List<Uri>> get _libraryUris async {? Or maybe _transitiveLibraryUris?

var expression = "require('dart_sdk').dart.getLibraries();";
var librariesResult = await tabConnection.runtime.sendCommand(
'Runtime.evaluate',
params: {'expression': expression, 'returnByValue': true});
_handleErrorIfPresent(librariesResult, evalContents: expression);
return List.from(librariesResult.result['result']['value'] as List);
return (librariesResult.result['result']['value'] as List)
.cast<String>()
.map(Uri.parse)
.toList();
}

/// Listens for chrome console events and handles the ones we care about.
Expand Down
2 changes: 1 addition & 1 deletion dwds/pubspec.yaml
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
name: dwds
version: 0.3.2
version: 0.3.3-dev
author: Dart Team <[email protected]>
homepage: https://github.com/dart-lang/webdev/tree/master/dwds
description: >-
Expand Down
3 changes: 2 additions & 1 deletion dwds/test/chrome_proxy_service_test.dart
Original file line number Diff line number Diff line change
Expand Up @@ -242,7 +242,8 @@ void main() {
expect(result, const TypeMatcher<Isolate>());
var isolate = result as Isolate;
expect(isolate.name, contains(context.appUrl));
// TODO: library names change with kernel dart-lang/sdk#36736
// TODO(jakemac): Update to the full expected uri `/hello_world/main.dart`
// once the next dev sdk comes out.
expect(isolate.rootLib.uri, endsWith('main.dart'));

expect(
Expand Down
4 changes: 4 additions & 0 deletions webdev/pubspec.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -49,5 +49,9 @@ dev_dependencies:
uuid: ^2.0.0
webdriver: ^2.0.0

dependency_overrides:
Copy link
Member

Choose a reason for hiding this comment

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

Not required but I'm fine leaving it in if you want.

Copy link
Member

Choose a reason for hiding this comment

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

Let's wait for #384 to land before merging this

dwds:
path: ../dwds

executables:
webdev: